Skip to content

fix(plugin): guard mig smi output parsing against malformed lines #2246

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.

Sign up for GitHub

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

Closed
AdityaShome wants to merge 5 commits into from
Closed

fix(plugin): guard mig smi output parsing against malformed lines #2246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d1731a9
bounds-check MIG UUID parsing in nvidia-smi fallback
AdityaShome Jul 31, 2026
83cb3b3
add regression tests for MIG UUID parsing bounds-check
AdityaShome Jul 31, 2026
6292517
don't fatal-exit on bad index, validate UUID before returning
AdityaShome Jul 31, 2026
af6bbd1
skip appending empty MIG UUID in GetContainerDeviceStrArray
AdityaShome Aug 1, 2026
89319cd
return an error instead of skipping unresolved MIG UUIDs
AdityaShome Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/device-plugin/nvidiadevice/nvinternal/plugin/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,12 @@ func (plugin *NvidiaDevicePlugin) Allocate(ctx context.Context, reqs *kubeletdev
}
devreq = alignedDevreq
}
response, err := plugin.getAllocateResponse(plugin.GetContainerDeviceStrArray(devreq))
containerDevices, err := plugin.GetContainerDeviceStrArray(devreq)
if err != nil {
PodAllocationFailed(nodename, current, NodeLockNvidia)
return nil, fmt.Errorf("failed to resolve container devices: %w", err)
}
response, err := plugin.getAllocateResponse(containerDevices)
if err != nil {
return nil, fmt.Errorf("failed to get allocate response: %v", err)
}
Expand Down
41 changes: 29 additions & 12 deletions pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,24 +161,34 @@ func GetMigUUIDFromSmiOutput(output string, uuid string, idx int) string {
continue
}
klog.Infoln("inspecting", val)
num := strings.Split(val, "Device")[1]
num = strings.Split(num, ":")[0]
deviceParts := strings.Split(val, "Device")

@mesutoezdil mesutoezdil Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getmiguuidfromsmioutput and getmiguuidfromindex are gone in master now. #2378 replaced this whole nvidia-smi fallback with nvml miginstancemanager. pls check if the bug still exists in new code.

@AdityaShome AdityaShome Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great work on the new MIG design overall, while going through the code I found two smaller issues:

  1. gpuUUIDToIndex and nvmlBusyGPUs (mig_startup.go) call nvml.Init() with no matching nvml.Shutdown() unlike every other NVML entry point in the codebase. gpuUUIDToIndex runs on every Allocate() and every 5s via the reconciler, so this leaks indefinitely on nodes with active MIG allocations.
  2. migmgr.go's MigInstanceManager never calls nvml.Init() itself it silently relies on nvmlBusyGPUs() having already leaked one at startup. Fixing 1 by adding a proper Shutdown() would break this.

Planning two follow-up PRs for these.

if len(deviceParts) < 2 {
continue
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
num := strings.Split(deviceParts[1], ":")[0]
num = strings.TrimSpace(num)
index, err := strconv.Atoi(num)
if err != nil {
klog.Fatal("atoi failed num=", num)
klog.Errorf("failed to parse device index from smi output line %q: %v", val, err)
continue
}
if index == idx {
outputStr := strings.Split(val, ":")[2]
outputStr = strings.TrimSpace(outputStr)
colonParts := strings.Split(val, ":")
if len(colonParts) < 3 || !strings.Contains(colonParts[1], "UUID") {
continue
}
outputStr := strings.TrimSpace(colonParts[2])
Comment thread
coderabbitai[bot] marked this conversation as resolved.
outputStr = strings.TrimRight(outputStr, ")")
if !strings.HasPrefix(outputStr, "MIG-") {
continue
}
return outputStr
Comment thread
mesutoezdil marked this conversation as resolved.
}
}
return ""
}

func GetMigUUIDFromIndex(uuid string, idx int) string {
func GetMigUUIDFromIndex(uuid string, idx int) (string, error) {
defer nvml.Shutdown()
if nvret := nvml.Init(); nvret != nvml.SUCCESS {
klog.Errorln("nvml Init err: ", nvret)
Expand All @@ -202,15 +212,18 @@ func GetMigUUIDFromIndex(uuid string, idx int) string {
klog.Fatalf("nvidia-smi -L failed with %s\n", err)
}
outStr := stdout.String()
uuid := GetMigUUIDFromSmiOutput(outStr, originuuid, idx)
return uuid
migUUID := GetMigUUIDFromSmiOutput(outStr, originuuid, idx)
if migUUID == "" {
return "", fmt.Errorf("failed to resolve MIG UUID for device %s at index %d via nvidia-smi fallback", originuuid, idx)
}
return migUUID, nil
}
res, ret := migdev.GetUUID()
if ret != nvml.SUCCESS {
klog.Error(`nvml get mig uuid error ret=`, ret)
panic(0)
}
return res
return res, nil
}

func GetMigGpuInstanceIdFromIndex(uuid string, idx int) (int, error) {
Expand Down Expand Up @@ -457,7 +470,7 @@ func deepCopyMigConfig(src nvidia.MigConfigSpec) nvidia.MigConfigSpec {
return dst
}

func (nv *NvidiaDevicePlugin) GetContainerDeviceStrArray(c device.ContainerDevices) []string {
func (nv *NvidiaDevicePlugin) GetContainerDeviceStrArray(c device.ContainerDevices) ([]string, error) {
tmp := []string{}
needsreset := false
position := 0
Expand Down Expand Up @@ -500,11 +513,15 @@ func (nv *NvidiaDevicePlugin) GetContainerDeviceStrArray(c device.ContainerDevic
}
}
}
tmp = append(tmp, GetMigUUIDFromIndex(val.UUID, position))
migUUID, err := GetMigUUIDFromIndex(val.UUID, position)
if err != nil {
return nil, fmt.Errorf("failed to resolve MIG UUID for %s at position %d: %w", val.UUID, position, err)
}
tmp = append(tmp, migUUID)
}
}
klog.V(3).Infoln("mig current=", nv.migCurrent, ":", needsreset, "position=", position, "uuid lists", tmp)
return tmp
return tmp, nil
}

var podAllocationTrySuccess = func(nodeName string, devName string, lockName string, pod *corev1.Pod) {
Expand Down
60 changes: 60 additions & 0 deletions pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,3 +846,63 @@ func TestWriteMigConfig_RemovesStaleFileOnFailure(t *testing.T) {
t.Errorf("expected stale config to be removed, stat err: %v", err)
}
}

func TestGetMigUUIDFromSmiOutput_HappyPath(t *testing.T) {
uuid := "GPU-b8f3c1a2-0000-0000-0000-000000000000"
output := "GPU 0: NVIDIA A100-SXM4-40GB (UUID: " + uuid + ")\n" +
" MIG 3g.20gb Device 0: (UUID: MIG-11111111-1111-1111-1111-111111111111)\n" +
" MIG 3g.20gb Device 1: (UUID: MIG-22222222-2222-2222-2222-222222222222)\n" +
"GPU 1: NVIDIA A100-SXM4-40GB (UUID: GPU-c9f4d2b3-0000-0000-0000-000000000000)\n"

got := GetMigUUIDFromSmiOutput(output, uuid, 1)
want := "MIG-22222222-2222-2222-2222-222222222222"
if got != want {
t.Errorf("expected %q, got %q", want, got)
}
}

func TestGetMigUUIDFromSmiOutput_MalformedLineWithoutDeviceKeyword(t *testing.T) {
uuid := "GPU-b8f3c1a2-0000-0000-0000-000000000000"
output := "GPU 0: NVIDIA A100-SXM4-40GB (UUID: " + uuid + ")\n" +
" MIG-instance-not-yet-configured\n"

got := GetMigUUIDFromSmiOutput(output, uuid, 0)
if got != "" {
t.Errorf("expected empty string for malformed line, got %q", got)
}
}

func TestGetMigUUIDFromSmiOutput_MalformedLineTooFewColons(t *testing.T) {
uuid := "GPU-b8f3c1a2-0000-0000-0000-000000000000"
output := "GPU 0: NVIDIA A100-SXM4-40GB (UUID: " + uuid + ")\n" +
" MIG 3g.20gb Device 0: no-uuid-here\n"

got := GetMigUUIDFromSmiOutput(output, uuid, 0)
if got != "" {
t.Errorf("expected empty string for malformed line, got %q", got)
}
}

func TestGetMigUUIDFromSmiOutput_NonNumericDeviceIndex(t *testing.T) {
uuid := "GPU-b8f3c1a2-0000-0000-0000-000000000000"
output := "GPU 0: NVIDIA A100-SXM4-40GB (UUID: " + uuid + ")\n" +
" MIG 3g.20gb Device x: (UUID: MIG-11111111-1111-1111-1111-111111111111)\n" +
" MIG 3g.20gb Device 0: (UUID: MIG-22222222-2222-2222-2222-222222222222)\n"

got := GetMigUUIDFromSmiOutput(output, uuid, 0)
want := "MIG-22222222-2222-2222-2222-222222222222"
if got != want {
t.Errorf("expected non-numeric device index line to be skipped and %q returned, got %q", want, got)
}
}

func TestGetMigUUIDFromSmiOutput_MatchingIndexWithoutUUIDField(t *testing.T) {
uuid := "GPU-b8f3c1a2-0000-0000-0000-000000000000"
output := "GPU 0: NVIDIA A100-SXM4-40GB (UUID: " + uuid + ")\n" +
" MIG 3g.20gb Device 0: status: not-a-uuid\n"

got := GetMigUUIDFromSmiOutput(output, uuid, 0)
if got != "" {
t.Errorf("expected empty string when the UUID field is missing, got %q", got)
}
}
Loading