fix(device-plugin): validate slice bounds in GetMigUUIDFromSmiOutput by Nitish08-08 · Pull Request #2430 · Project-HAMi/HAMi

Nitish08-08 · 2026-08-07T00:31:34Z

GetMigUUIDFromSmiOutput indexes directly into strings.Split() results without checking length. Malformed nvidia-smi output missing the 'Device' delimiter or sufficient colon-separated fields causes an index out of range panic.

Add length validation before each slice access and replace klog.Fatal with Warningf+continue so a single malformed line does not crash the device-plugin.

Also add unit tests covering valid output, empty input, missing delimiters, non-numeric indices, and uuid mismatches.

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Summary by CodeRabbit

Bug Fixes
- Improved handling of malformed NVIDIA MIG device output.
- Continues scanning when individual device entries contain invalid or incomplete data.
- Prevents parsing errors from causing incorrect failures when retrieving MIG UUIDs.
Tests
- Added coverage for valid extraction, instance selection, mismatched GPU UUIDs, malformed output, invalid device indices, and missing instances.

hami-robot · 2026-08-07T00:31:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Nitish08-08
Once this PR has been reviewed and has the lgtm label, please assign wawa0210 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

coderabbitai · 2026-08-07T00:31:52Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 92b7dd7a-3f55-42b7-855d-2abaee1f7b2a

📥 Commits

Reviewing files that changed from the base of the PR and between 47b8181 and 6d12a89.

📒 Files selected for processing (2)

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go
pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go
pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go

📝 Walkthrough

Walkthrough

The MIG UUID parser now validates SMI output before indexing fields. Malformed records produce warnings and are skipped. Table-driven tests cover valid extraction, mismatched UUIDs, invalid indices, malformed lines, and missing instances.

Changes

MIG parser hardening

Layer / File(s)	Summary
Defensive MIG parsing and validation `pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go`, `pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go`	`GetMigUUIDFromSmiOutput` validates delimiters, fields, and device indices. Malformed records are skipped with warnings. Table-driven tests cover valid extraction and empty-result cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Project-HAMi/HAMi#2234: Changes related MIG UUID lookup utilities.
Project-HAMi/HAMi#2246: Changes the same parser and its malformed-output tests.
Project-HAMi/HAMi#2360: Covers malformed MIG output and invalid device indices.

Suggested reviewers: archlitchi, mesutoezdil

Poem

A rabbit checks each MIG line,
Skips broken fields in due time.
UUIDs safely hop along,
Warnings mark where inputs went wrong.
Tests keep watch beside the gate.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the defensive slice-bound validation added to GetMigUUIDFromSmiOutput.
Linked Issues check	✅ Passed	The changes validate split results, skip malformed nvidia-smi lines, avoid panics, and add tests required by issue `#2359`.
Out of Scope Changes check	✅ Passed	The changes are limited to defensive parsing updates and focused unit tests for GetMigUUIDFromSmiOutput.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go`:
- Around line 993-998: Update the “MIG line without enough colons” fixture to
use a numeric device index while omitting the “UUID:” field, so parsing reaches
UUID-field validation and still expects no match. Keep the matching GPU UUID and
index unchanged, and ensure the test rejects truncated records accepted by the
parser.

In `@pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go`:
- Around line 178-183: Update the MIG output parsing around colonParts in
pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go:178-183 to extract and
validate the actual UUID field after “UUID:” (or require four colon-separated
fields and select the UUID field), rather than using the profile field. Update
the malformed fixture in
pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go:993-998 to use a
numeric device index with the UUID field missing, ensuring the test reaches UUID
validation.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 00257a19-255f-437e-b141-bc3b02a4681c

📥 Commits

Reviewing files that changed from the base of the PR and between 87d9795 and 5e83445.

📒 Files selected for processing (2)

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go
pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go

coderabbitai · 2026-08-07T00:33:12Z

+			colonParts := strings.Split(val, ":")
+			if len(colonParts) < 3 {
+				klog.Warningf("unexpected MIG output format, missing colon fields: %q", val)
+				continue
+			}
+			outputStr := colonParts[2]


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Parse and test the actual UUID field.

The parser selects the MIG profile field instead of the UUID field. The malformed fixture also exits during device-index parsing, so it cannot detect this defect.

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go#L178-L183: extract the value after "UUID:", or validate four colon fields and select the UUID field.

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go#L993-L998: use a numeric device index with a missing UUID field so the test reaches the UUID-field validation.

📍 Affects 2 files

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go#L178-L183 (this comment)

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go#L993-L998

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go` around lines 178 - 183, Update the MIG output parsing around colonParts in pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go:178-183 to extract and validate the actual UUID field after “UUID:” (or require four colon-separated fields and select the UUID field), rather than using the profile field. Update the malformed fixture in pkg/device-plugin/nvidiadevice/nvinternal/plugin/util_test.go:993-998 to use a numeric device index with the UUID field missing, ensuring the test reaches UUID validation.

Closes Project-HAMi#2359 GetMigUUIDFromSmiOutput indexes directly into strings.Split() results without checking length. Malformed nvidia-smi output missing the 'Device' delimiter or sufficient colon-separated fields causes an index out of range panic. Add length validation before each slice access and replace klog.Fatal with Warningf+continue so a single malformed line does not crash the device-plugin. Also add unit tests covering valid output, empty input, missing delimiters, non-numeric indices, and uuid mismatches. Signed-off-by: Nitish08-08 <nitishkumar805511@gmail.com> Signed-off-by: Nitish <nitishsahani0708@gmail.com>

coderabbitai · 2026-08-07T05:12:31Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

archlitchi · 2026-08-07T06:00:16Z

Already fixed in #2246

hami-robot Bot requested a review from archlitchi August 7, 2026 00:31

hami-robot Bot added the dco-signoff: yes label Aug 7, 2026

hami-robot Bot requested a review from mesutoezdil August 7, 2026 00:31

github-actions Bot added the kind/bug Something isn't working label Aug 7, 2026

hami-robot Bot added the size/L label Aug 7, 2026

coderabbitai Bot reviewed Aug 7, 2026

View reviewed changes

Nitish08-08 force-pushed the fix/issue-2359-mig-panic branch from 5e83445 to 6d12a89 Compare August 7, 2026 05:12

This was referenced Aug 7, 2026

fix(device-plugin): replace panic with graceful error handling in NVM… #2437

Closed

Fix/issue 2359 and 2419 #2438

Closed

archlitchi closed this Aug 7, 2026

This was referenced Aug 7, 2026

fix(device-plugin): replace panic with graceful error handling in Get… #2453

Closed

[BUG] vGPUmonitor drops or corrupts metrics due to hardcoded UUID length assumptions #2561

Closed

Navigation Menu

Uh oh!

fix(device-plugin): validate slice bounds in GetMigUUIDFromSmiOutput - #2430

fix(device-plugin): validate slice bounds in GetMigUUIDFromSmiOutput#2430
Nitish08-08 wants to merge 1 commit into
Project-HAMi:masterfrom
Nitish08-08:fix/issue-2359-mig-panic

Nitish08-08 commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

hami-robot Bot commented Aug 7, 2026

Uh oh!

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot Aug 7, 2026

Uh oh!

coderabbitai Bot commented Aug 7, 2026

Uh oh!

archlitchi commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Nitish08-08 commented Aug 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

hami-robot Bot commented Aug 7, 2026

Uh oh!

coderabbitai Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 7, 2026

Uh oh!

archlitchi commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Nitish08-08 commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading