Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libnvmf: avoid resource leak #1239

Merged
merged 4 commits into from
May 21, 2024
Merged

Conversation

khorben
Copy link
Contributor

@khorben khorben commented May 16, 2024

In nvmf_host_fetch_discovery_log_page(), the log variable may have been allocated on the heap during a first loop cycle, and should be free()'d again before exiting upon errors.

Reported by: Coverity
CID: 1545034
Sponsored by: The FreeBSD Foundation

@khorben
Copy link
Contributor Author

khorben commented May 16, 2024

Cc @bsdjhb

Copy link
Member

@bsdimp bsdimp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.. borderline worth it because this usually is adjacent to an exitvpath, but it is in a library.

@bsdimp bsdimp added the ready label May 16, 2024
@bsdimp
Copy link
Member

bsdimp commented May 16, 2024

Tagged with 'ready' but I'll give it until tomorrow for @bsdjhb to take. peek

@bsdjhb
Copy link
Member

bsdjhb commented May 16, 2024

The other error cases in the loop after the realloc all call free, so this is more consistent.

@bsdjhb
Copy link
Member

bsdjhb commented May 16, 2024

Would maybe s/first loop cycle/previous loop iteration/ in the log message. "again" might also be confusing as you aren't calling free multiple times, so I would perhaps omit it from the log message.

@khorben khorben force-pushed the khorben/cid-1545034 branch 2 times, most recently from a009674 to fe3edfa Compare May 16, 2024 17:56
@khorben
Copy link
Contributor Author

khorben commented May 16, 2024

Would maybe s/first loop cycle/previous loop iteration/ in the log message. "again" might also be confusing as you aren't calling free multiple times, so I would perhaps omit it from the log message.

The log variable is always allocated in the first loop cycle initially (if there is more than one).
I have removed "again" from the log message.
Thanks!

bsdimp pushed a commit to concussious/freebsd-src that referenced this pull request May 21, 2024
In nvmf_host_fetch_discovery_log_page(), the log variable may have been
allocated on the heap during the first loop cycle, and should be
free()'d before exiting upon errors.

Reported by:	Coverity
CID:		1545034
Sponsored by:	The FreeBSD Foundation

Reviewed by: imp,jhb
Pull Request: freebsd#1239
concussious and others added 4 commits May 21, 2024 17:41
MFC after: 3 days
Fixes: 286c490 (add -noauto), 3914ddf (import autofs)
Pull Request: freebsd#1243

Reviewed by: imp
Pull Request: freebsd#1243
Reviewed by: imp,jhb
Pull Request: freebsd#1242
In nvmf_host_fetch_discovery_log_page(), the log variable may have been
allocated on the heap during the first loop cycle, and should be
free()'d before exiting upon errors.

Reported by:	Coverity
CID:		1545034
Sponsored by:	The FreeBSD Foundation

Reviewed by: imp,jhb
Pull Request: freebsd#1239
@freebsd-git freebsd-git merged commit 408572a into freebsd:main May 21, 2024
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants