Repository navigation
fix(amdsmi): initialize Device metrics fields to avoid uninitialized reads - #8182
Merged
Merged
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets valgrind-reported issues in AMDSMI APIs by eliminating uninitialized reads and improving defensive initialization of output buffers/structures, aligning behavior with the PR’s stated goal of preventing memleaks/uninitialized-use patterns.
Changes:
- Add null-output-parameter validation for
amdsmi_topo_get_p2p_status(). - Zero-initialize output structs/bitmasks and local char buffers used for partition capability queries; trim whitespace in returned capability strings.
- Fix a debug log to report the correct status value for the
rsmi_dev_memory_partition_capabilities_getcall; initialize additionalDevicemembers in the constructor initializer list.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| projects/amdsmi/src/amd_smi/amd_smi.cc | Adds null checks and broad zero-initialization to prevent uninitialized reads in topology and partition APIs; corrects status used in debug logging. |
| projects/amdsmi/rocm_smi/src/rocm_smi_device.cc | Extends Device constructor member initialization to avoid uninitialized member state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SakaSitharammurthy
changed the title
…mi_get_gpu_memory_partition_config uninitialized read
marifamd
force-pushed
the
users/ssaka/fix_uninitialized_read
branch
from
July 15, 2026 21:23
ccc32bf to
5cdbff1
Compare
marifamd
requested changes
Jul 20, 2026
… tests Initialize remaining Device POD members (bdfid_ to UINT64_MAX to preserve the sentinel path selection in DiscoverAmdGpuKfdNodes), zero-init supported_nps_configs and the resource-profile local, and collapse the redundant nps_caps union writes to value-initialization. Add a null check to amdsmi_topo_get_link_type mirroring amdsmi_topo_get_p2p_status, and cover the p2p null-argument path with a C++ assertion and a Python binding test. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
gabrpham
requested changes
Jul 20, 2026
gabrpham
approved these changes
Aug 28, 2026
gabrpham
left a comment
gabrpham
left a comment
Contributor
There was a problem hiding this comment.
Potential for out of bounds error has been addressed. Approved.
marifamd
approved these changes
Aug 28, 2026
Contributor
TheRock Submodule Bump ActivityNewest first
|
corey-derochie-amd
pushed a commit
that referenced
this pull request
Oct 8, 2026
…reads (#8182) ## Motivation Due to uninitialized read of few variables in this api's "amdsmi_topo_get_p2p_status() and amdsmi_get_gpu_memory_partition_config()" creates memleak. ## JIRA ID ISSUE ID : ROCm/amdsmi#129 ## Test Result ->After running valgrind tool on this example "amd_smi_partition_example.cc" and observed no memory leaks. --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
corey-derochie-amd
pushed a commit
that referenced
this pull request
Oct 9, 2026
…reads (#8182) ## Motivation Due to uninitialized read of few variables in this api's "amdsmi_topo_get_p2p_status() and amdsmi_get_gpu_memory_partition_config()" creates memleak. ## JIRA ID ISSUE ID : ROCm/amdsmi#129 ## Test Result ->After running valgrind tool on this example "amd_smi_partition_example.cc" and observed no memory leaks. --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
corey-derochie-amd
pushed a commit
that referenced
this pull request
Oct 14, 2026
…reads (#8182) ## Motivation Due to uninitialized read of few variables in this api's "amdsmi_topo_get_p2p_status() and amdsmi_get_gpu_memory_partition_config()" creates memleak. ## JIRA ID ISSUE ID : ROCm/amdsmi#129 ## Test Result ->After running valgrind tool on this example "amd_smi_partition_example.cc" and observed no memory leaks. --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
corey-derochie-amd
pushed a commit
that referenced
this pull request
Oct 14, 2026
…reads (#8182) ## Motivation Due to uninitialized read of few variables in this api's "amdsmi_topo_get_p2p_status() and amdsmi_get_gpu_memory_partition_config()" creates memleak. ## JIRA ID ISSUE ID : ROCm/amdsmi#129 ## Test Result ->After running valgrind tool on this example "amd_smi_partition_example.cc" and observed no memory leaks. --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
corey-derochie-amd
pushed a commit
that referenced
this pull request
Oct 15, 2026
…reads (#8182) ## Motivation Due to uninitialized read of few variables in this api's "amdsmi_topo_get_p2p_status() and amdsmi_get_gpu_memory_partition_config()" creates memleak. ## JIRA ID ISSUE ID : ROCm/amdsmi#129 ## Test Result ->After running valgrind tool on this example "amd_smi_partition_example.cc" and observed no memory leaks. --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
corey-derochie-amd
pushed a commit
that referenced
this pull request
Oct 15, 2026
…reads (#8182) ## Motivation Due to uninitialized read of few variables in this api's "amdsmi_topo_get_p2p_status() and amdsmi_get_gpu_memory_partition_config()" creates memleak. ## JIRA ID ISSUE ID : ROCm/amdsmi#129 ## Test Result ->After running valgrind tool on this example "amd_smi_partition_example.cc" and observed no memory leaks. --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
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.
Motivation
Due to uninitialized read of few variables in this api's "amdsmi_topo_get_p2p_status () and amdsmi_get_gpu_memory_partition_config ()" creates memleak.
JIRA ID
ISSUE ID : ROCm/amdsmi#129
Test Result
->After running valgrind tool on this example "amd_smi_partition_example.cc" and observed no memory leaks.