Fix udev enumerator leaks and a wrong null-check in device discovery#477
Open
rocker-zhang wants to merge 1 commit into
Open
Fix udev enumerator leaks and a wrong null-check in device discovery#477rocker-zhang wants to merge 1 commit into
rocker-zhang wants to merge 1 commit into
Conversation
nvtop_device_get_hwmon() and the intel/v3d gpuinfo_*_get_device_handles() functions return early when a udev enumerator API call fails, but those early returns never release the enumerator, leaking the udev handle and the udev_enumerate object. In nvtop_device_get_hwmon() the no-hwmon path is reached for every device without an hwmon node, so it leaks on a normal startup, not just on error. Route the early exits through a single nvtop_enumerator_unref() using the goto-cleanup pattern already used in nvtop_enumerator_new(). nvtop_enumerator_new() also tested the address of the caller's pointer, which is never NULL, instead of the calloc() result. An allocation failure would therefore fall through and dereference a NULL pointer instead of returning -1; check *enumerator instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While reading the udev device-discovery code I noticed the enumerator is
leaked on a few early-return paths, in the same spirit as the recent cleanup
fixes (#467, #468).
Three functions create an enumerator with
nvtop_enumerator_new()and thenreturn early when a match/alloc step fails, without calling
nvtop_enumerator_unref():nvtop_device_get_hwmon()and the intel/v3dgpuinfo_*_get_device_handles(). The leaked enumerator owns a udev handle anda udev_enumerate object. The no-hwmon branch in
nvtop_device_get_hwmon()runsfor every device that has no hwmon node, so this one leaks on a normal run
rather than only on failure. I routed the early exits through a single unref
with the goto-cleanup pattern already used in
nvtop_enumerator_new(), leavingthe success path unchanged.
The same commit also fixes
nvtop_enumerator_new(): it checkedenumerator(the address of the caller's variable, never NULL) instead of
*enumerator(the calloc result), so an out-of-memory calloc would dereference NULL instead
of returning -1.
Validated on x86_64 (all backends + libdrm) and arm64 (NVIDIA + v3d): clean
build with
-Wall -Wextra, the gtest suite passes, and--snapshoton a liveGPU still enumerates devices and reports stats. I also confirmed the leak with
LeakSanitizer on both arches: a build that skips the unref on the early-return
path leaks the enumerator (the udev handle and udev_enumerate object), and the
fix brings it to zero. clang's static analyzer flags the leak in
nvtop_device_get_hwmon()before the change and is clean after.