fix(deviceplugin): add error handling and symlink guard in Allocate - #2655
fix(deviceplugin): add error handling and symlink guard in Allocate#2655SanyamRana wants to merge 1 commit into
Conversation
The Allocate function in the NVIDIA device plugin created two host directories (per-container vGPU cache and /tmp/vgpulock) with four bare os.MkdirAll / os.Chmod calls whose return values were silently discarded. This caused two problems: 1. Missing error handling: if MkdirAll failed (disk full, permission denied on the parent, read-only filesystem), the plugin continued to append volume mounts that would silently fail inside the container. The allocation appeared to succeed but the pod runtime was broken. 2. Local Privilege Escalation via symlink: the plugin runs as root. /tmp is world-writable. A malicious local user could race to place a symlink at /tmp/vgpulock pointing at a sensitive file such as /etc/shadow. os.Chmod follows symlinks, so the plugin would silently change /etc/shadow to mode 0777, allowing any user on the node to read or write it. Fix: wrap every MkdirAll and Chmod call with an error check that calls PodAllocationFailed and returns the error immediately. Before each Chmod, use os.Lstat (which does NOT follow symlinks) to verify the path is a real directory; refuse to chmod if a symlink is found. Signed-off-by: Sanyam Rana <sanyamrana26@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SanyamRana The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesvGPU allocation safety
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
The Allocate function in the NVIDIA device plugin created two host directories (per-container vGPU cache and /tmp/vgpulock) with four bare os.MkdirAll / os.Chmod calls whose return values were silently discarded.
This caused two problems:
Missing error handling: if MkdirAll failed (disk full, permission denied on the parent, read-only filesystem), the plugin continued to append volume mounts that would silently fail inside the container. The allocation appeared to succeed but the pod runtime was broken.
Local Privilege Escalation via symlink: the plugin runs as root. /tmp is world-writable. A malicious local user could race to place a symlink at /tmp/vgpulock pointing at a sensitive file such as /etc/shadow. os.Chmod follows symlinks, so the plugin would silently change /etc/shadow to mode 0777, allowing any user on the node to read or write it.
Fix: wrap every MkdirAll and Chmod call with an error check that calls PodAllocationFailed and returns the error immediately. Before each Chmod, use os.Lstat (which does NOT follow symlinks) to verify the path is a real directory; refuse to chmod if a symlink is found.
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