gimamdsmi: fix aga_obj_key_t::str() UUID truncation from sizeof(char*) - #81
Merged
Merged
Conversation
str() used sizeof(buf) as the snprintf bound while buf is a char*, so the bound was 8 (pointer size), not the buffer length. For the MAC-tail loop (off>=24) the expression sizeof(buf)-off underflows to a size_t far above INT_MAX, which glibc snprintf rejects, so the final 12-hex UUID group is never written. str() returns a truncated 25-char UUID. This is normally harmless (str() is used for logging) but under AGA_SMI_LAZY_INIT=1 the per-request AGA_SMI_SESSION_GUARD re-resolves the GPU handle via amdsmi_get_processor_handle_from_uuid(gpu_key->str()). The GIM amdsmi lib's is_uuid_valid() rejects the truncated string with AMDSMI_STATUS_INVAL, so the guard bails before filling any spec and before the virtualization-mode read. Net effect on a GIM/SR-IOV host: deployment_mode reported as baremetal and all GPU metrics dropped. Use OBJ_MAX_KEY_STR_LEN + 1 as the bound. Validated on leto MI210 GIM host with AGA_SMI_LAZY_INIT=1: resolve failures 14->0, full 36-char UUID emitted, deployment_mode=hypervisor, full metric set restored.
bhatturu
force-pushed
the
fix/gim-uuid-str-truncation
branch
from
July 23, 2026 03:39
52d55b8 to
536930e
Compare
sarat-k
pushed a commit
that referenced
this pull request
Jul 29, 2026
* gimamdsmi: fix aga_obj_key_t::str() UUID truncation from sizeof(char*) (#81) str() used sizeof(buf) as the snprintf bound while buf is a char*, so the bound was 8 (pointer size), not the buffer length. For the MAC-tail loop (off>=24) the expression sizeof(buf)-off underflows to a size_t far above INT_MAX, which glibc snprintf rejects, so the final 12-hex UUID group is never written. str() returns a truncated 25-char UUID. This is normally harmless (str() is used for logging) but under AGA_SMI_LAZY_INIT=1 the per-request AGA_SMI_SESSION_GUARD re-resolves the GPU handle via amdsmi_get_processor_handle_from_uuid(gpu_key->str()). The GIM amdsmi lib's is_uuid_valid() rejects the truncated string with AMDSMI_STATUS_INVAL, so the guard bails before filling any spec and before the virtualization-mode read. Net effect on a GIM/SR-IOV host: deployment_mode reported as baremetal and all GPU metrics dropped. Use OBJ_MAX_KEY_STR_LEN + 1 as the bound. Validated on leto MI210 GIM host with AGA_SMI_LAZY_INIT=1: resolve failures 14->0, full 36-char UUID emitted, deployment_mode=hypervisor, full metric set restored. * sec-fix: remediate gpuctl HIGH/UNKNOWN CVEs via dependency bumps (#82) Scans of the gpuctl Go binary flagged several vulnerabilities. Fixed by updating the replace pins in go.mod and the Makefile mod: target, then regenerating go.sum/vendor via `make mod`: - google.golang.org/grpc v1.79.3 -> v1.82.1 (GHSA-hrxh-6v49-42gf, HIGH) - golang.org/x/net v0.53.0 -> v0.56.0 (CVE-2026-25681, CVE-2026-27136, CVE-2026-39821 HIGH; CVE-2026-25680, CVE-2026-42502, CVE-2026-42506 MEDIUM; CVE-2026-46600 UNKNOWN) - golang.org/x/text v0.38.0 -> v0.39.0 (CVE-2026-56852, UNKNOWN) The grpc bump pulled x/net up to v0.53.0, which no longer matched the prior x/net@v0.48.0 replace; the pins are retargeted to the current base versions. trivy scan of the rebuilt gpuctl binary reports zero findings across all severities. No Go toolchain bump needed (no stdlib HIGH/CRITICAL). Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Bhanu Kiran Atturu <bhatturu@amd.com> Co-authored-by: Claude <noreply@anthropic.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
3 participants
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.
str() used sizeof(buf) as the snprintf bound while buf is a char*, so the bound was 8 (pointer size), not the buffer length. For the MAC-tail loop (off>=24) the expression sizeof(buf)-off underflows to a size_t far above INT_MAX, which glibc snprintf rejects, so the final 12-hex UUID group is never written. str() returns a truncated 25-char UUID.
This is normally harmless (str() is used for logging) but under AGA_SMI_LAZY_INIT=1 the per-request AGA_SMI_SESSION_GUARD re-resolves the GPU handle via amdsmi_get_processor_handle_from_uuid(gpu_key->str()). The GIM amdsmi lib's is_uuid_valid() rejects the truncated string with AMDSMI_STATUS_INVAL, so the guard bails before filling any spec and before the virtualization-mode read. Net effect on a GIM/SR-IOV host: deployment_mode reported as baremetal and all GPU metrics dropped.
Use OBJ_MAX_KEY_STR_LEN + 1 as the bound. Validated on leto MI210 GIM host with AGA_SMI_LAZY_INIT=1: resolve failures 14->0, full 36-char UUID emitted, deployment_mode=hypervisor, full metric set restored.
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist