[AMD-SMI] Clean up SMI - #3655
Merged
Merged
Conversation
dmitrii-galantsev
changed the title
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
from
March 2, 2026 19:03
74b4653 to
43d1d4a
Compare
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
2 times, most recently
from
March 2, 2026 19:54
20f402b to
870e661
Compare
marifamd
force-pushed
the
dgalants/smi-cleanup
branch
from
March 4, 2026 23:23
769016d to
66b3957
Compare
dmitrii-galantsev
marked this pull request as ready for review
March 5, 2026 23:01
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
2 times, most recently
from
March 5, 2026 23:05
d02dbc8 to
ad3b705
Compare
marifamd
force-pushed
the
dgalants/smi-cleanup
branch
from
March 6, 2026 07:00
ad3b705 to
0736d81
Compare
gabrpham
approved these changes
Mar 7, 2026
gabrpham
force-pushed
the
dgalants/smi-cleanup
branch
from
March 7, 2026 00:35
05018b6 to
ba6a894
Compare
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
2 times, most recently
from
March 17, 2026 17:04
52ad70c to
40cfd15
Compare
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
from
April 15, 2026 03:03
6af9770 to
dd714f8
Compare
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
from
April 15, 2026 03:05
dd714f8 to
291b649
Compare
The C API was renamed from amdsmi_get_cpusocket_handles to amdsmi_get_cpu_handles in commit 4260c0a, but the Python layer kept the old name. This left a stale function name throughout the Python interface and CLI that didn't match the C API. Rename the Python function to match the C API and update all call sites. Also remove the redundant dict-wrapping amdsmi_get_cpu_handles function that shadowed the renamed implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…udit Fixes 10 bugs identified in the code hygiene audit (issue #3634): - py-interface: sys.getsizeof → ctypes.sizeof for correct C type sizes - py-interface: PAGE_FAULT_START mapped to wrong enum (copy-paste bug) - amd_smi_no_drm_nic.cc: sizeof(array-1) → sizeof(array) - 1 in snprintf - amd_smi_system.cc: OOB write via sockets[0] on empty vector → push_back - amd_smi_utils.cc: num_pages = 0 assigned to pointer, not *num_pages - rocm_smi_device.cc: isRestartInProgress checked stale input instead of local isSystemAMDGPUModuleLive - rocm_smi_utils.cc: dangling pointer from local string c_str() → literal - rocm_smi_utils.cc: pclose(NULL) UB when popen fails → add NULL guard - amd_smi_lspci_commands.cc: multi-char literal ']----' → string "]----" - rocm_smi_kfd.cc: stoi truncates uint64_t GPU IDs → stoull Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes 5 more issues from the audit (issue #3634): - amd_smi_lspci_commands.cc: Validate BDF and search_key before passing to popen() to prevent shell injection (C-1) - amd_smi_system.cc: Replace popen("readlink ...") with POSIX readlink() syscall to eliminate command injection vector (C-2) - smi_nic.cpp: Allocate proper buffer for ethtool_perm_addr flexible array member to prevent stack buffer overflow (H-6) - amd_smi_utils.cc: Add width specifiers to sscanf %s formats to prevent buffer overflows on char[10] stack buffers (H-5) - amd_smi_cper.cc: Close file descriptor on short read error path to prevent fd leak (M-7) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e hygiene audit Fixes bugs of the same classes addressed in commit 562d704b3b, found in code added since that commit: - rocm_smi_kfd.cc: stoi → stoull for GPU ID stored in uint64_t (same bug as previously fixed at line 435) - amd_smi_utils.cc: Guard pclose(NULL) UB in shared_ptr<FILE> deleter - rocm_smi.cc: Fix refcount/fd leak on 3 error paths in rsmi_event_notification_init() — refcount was incremented but never decremented on ioctl failure, invalid anon_fd, or fdopen failure - rocm_smi.cc: Add %255s/%255[^\n] width specifiers to sscanf calls - Replace &vec[0] with vec.data() across production code, tests, and examples to eliminate UB when vectors are empty (31 locations) - amd_smi.cc: Convert non-standard VLA to std::vector Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
… audit Fixes bugs of the same classes addressed in commit 43dbf30eb3, found in code added since that commit: - fdinfo.cc: Add %12s width specifier to sscanf on 13-byte char buffer to prevent stack buffer overflow from malformed /proc fdinfo entries - rocm_smi_binary_parser.cc: Close file handle on calloc failure, and close file handle + free buffer on unsupported metrics version, to prevent fd and memory leaks on error paths - rocm_smi_monitor.cc: Close DIR* handle on regex exception path to prevent directory handle leak Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
from
April 15, 2026 17:34
291b649 to
966c790
Compare
Remove search_key from the shell command entirely. Previously,
get_lspci_device_data() interpolated user-influenced search_key
into a shell pipeline ("lspci ... | grep -i 'KEY'"), requiring
fragile character sanitization. Now only the regex-validated BDF
touches the shell; filtering is done in C++ via std::string::find().
Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
dmitrii-galantsev
force-pushed
the
dgalants/smi-cleanup
branch
from
April 15, 2026 17:36
966c790 to
6c0969e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marifamd
approved these changes
Apr 17, 2026
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
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.
Clean up SMI