fix(deployer): stop granting privilege to plugin-allocated devices - #15
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces logic to drop container privileges when devices are allocated by a device plugin, preventing workloads from bypassing slicing limits. It adds helper functions _is_device_plugin_resource and _resolve_privileged in the Kubernetes deployer, along with corresponding unit tests. Feedback was provided regarding a potential TypeError if envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_RUNTIME_VISIBLE_DEVICES is None, suggesting a fallback empty dictionary.
| if _is_device_plugin_resource(r_k) or ( | ||
| kdp | ||
| and ( | ||
| r_k | ||
| in envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_RUNTIME_VISIBLE_DEVICES | ||
| or r_k == envs.GPUSTACK_RUNTIME_DEPLOY_AUTOMAP_RESOURCE_KEY | ||
| ) | ||
| ): |
There was a problem hiding this comment.
The envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_RUNTIME_VISIBLE_DEVICES environment variable map can potentially be None according to its type definition in envs.py. If it is None, performing an in membership check will raise a TypeError.\n\nTo prevent potential runtime crashes, please use a fallback empty dictionary or {} when performing the membership check, similar to how envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_CDI is handled in _is_device_plugin_resource.
| if _is_device_plugin_resource(r_k) or ( | |
| kdp | |
| and ( | |
| r_k | |
| in envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_RUNTIME_VISIBLE_DEVICES | |
| or r_k == envs.GPUSTACK_RUNTIME_DEPLOY_AUTOMAP_RESOURCE_KEY | |
| ) | |
| ): | |
| if _is_device_plugin_resource(r_k) or ( | |
| kdp | |
| and ( | |
| r_k | |
| in (envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_RUNTIME_VISIBLE_DEVICES or {}) | |
| or r_k == envs.GPUSTACK_RUNTIME_DEPLOY_AUTOMAP_RESOURCE_KEY | |
| ) | |
| ): |
There was a problem hiding this comment.
Not applicable — this map can never be None at runtime.
envs.__getattr__ always evaluates the lambda in variables, and the lambda is to_dict(getenv(...)); to_dict short-circuits to {} on an empty value:
>>> from gpustack_runtime import envs
>>> type(envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_RUNTIME_VISIBLE_DEVICES)
<class 'dict'>
>>> from gpustack_runtime.envs import to_dict
>>> to_dict('')
{}The | None in the module-level annotation is a placeholder for the lazy-attribute declaration, not a reachable value. Six existing call sites read this same map unguarded — deployer/__types__.py:1391, deployer/kuberentes.py:1137 and :1141, deployer/docker.py:1004, deployer/podman.py:1003, deployer/cdi/__types__.py:680 — so a guard here would be the only one of its kind.
You did point at a real inconsistency, though: I had or {} on the CDI map two functions up and not here. Resolved in the other direction — the stray or {} is gone, so both reads now match the idiom used everywhere else in the file. Leaving this thread open since it is the opposite of the suggested change.
There was a problem hiding this comment.
Pull request overview
This PR tightens Kubernetes deployment security by preventing privileged containers from bypassing device-plugin device isolation (e.g., MIG, slicing/partitioning) and unintentionally gaining access to all host accelerators.
Changes:
- Introduces privilege-resolution logic that drops
privilegedwhen devices are provided by a device plugin (and for mapped device requests under the KDP injection policy). - Wires the new privilege resolution into Kubernetes Pod security context creation.
- Adds a parametrized test suite covering key privilege/resource-policy combinations and boundary cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| gpustack_runtime/deployer/kuberentes.py | Adds device-plugin detection and _resolve_privileged(); uses it when building Kubernetes container security contexts. |
| tests/gpustack_runtime/deployer/test_privileged.py | Adds parametrized unit tests for the new privilege-resolution behavior across env/KDP policies and resource shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return any( | ||
| resource_key == cdi or resource_key.startswith(f"{cdi}.") | ||
| for cdi in (envs.GPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_CDI or {}).values() | ||
| ) |
There was a problem hiding this comment.
The stated failure mode does not occur. With an empty CDI kind, resource_key.startswith(f"{cdi}.") is resource_key.startswith(".") — that requires the key to begin with a dot, not to contain one:
>>> "nvidia.com/gpu.sliced".startswith(".")
False
>>> "nvidia.com/gpu.sliced" == ""
FalseA Kubernetes resource name is a qualified name and cannot start with ., so an empty CDI kind matches nothing rather than everything. cpu / memory are skipped before this call anyway.
An empty value is reachable in the map (to_dict("nvidia.com/devices=") yields {"nvidia.com/devices": ""}), so the premise is right — but the consequence is a misconfigured entry that matches no key, which is the misconfiguration's own failure, not a misclassification here. Skipping falsy kinds would be a no-op guard, so I have left the code as is.
A privileged container receives all device nodes of the host, so it enumerates and can use every accelerator on the node, whatever the device plugin allocated to it. On a multi-card node that undoes both slicing modes: a workload holding one MIG device still sees the untouched cards beside it, and a soft-slicing memory limit lands on whichever device comes first instead of the allocated one. Drop privilege whenever the devices come from a device plugin -- any device plugin resource family, and every mapped device request under the KDP injection policy. Device requests the deployer resolves itself through visible-devices envs keep their privilege, as does a container that asks for no device at all. Signed-off-by: thxCode <thxcode0824@gmail.com>
0ead756 to
d4795af
Compare
|
Released as Consumed downstream by:
On provenance, to be precise: the after-state in the description was measured with |
Carries gpustack/runtime#15: the Kubernetes deployer no longer grants privilege to devices a device plugin allocated, so a divided-mode model instance stays inside its slice instead of enumerating every accelerator on the node. Signed-off-by: thxCode <thxcode0824@gmail.com>
Carries gpustack/runtime#15: the Kubernetes deployer no longer grants privilege to devices a device plugin allocated, so a divided-mode model instance stays inside its slice instead of enumerating every accelerator on the node. Signed-off-by: thxCode <thxcode0824@gmail.com>
What
A privileged container receives all device nodes of the host, so it enumerates — and can use — every accelerator on the node, whatever the device plugin allocated to it. On a multi-card node that undoes both slicing modes.
Drop privilege whenever the devices come from a device plugin: any device plugin resource family (
nvidia.com/gpu,nvidia.com/gpu.sliced*,nvidia.com/gpu.partitioned*, and the same shapes for every other vendor inGPUSTACK_RUNTIME_DEPLOY_RESOURCE_KEY_MAP_CDI), and every mapped device request under the KDP injection policy. Device requests the deployer resolves itself through visible-devices envs keep their privilege, as does a container that asks for no device at all.Why — measured on an 8×H100 node with MIG enabled on exactly one card
Before, with
privileged: true(which every GPUStack inference backend sets unconditionally):NVIDIA_VISIBLE_DEVICESnvidia-smi -LNVIDIA_VISIBLE_DEVICESis correct in all three, so it is not what enforces the bound — it is consumed by the container runtime at creation to decide which device nodes to mount, and a privileged container gets the whole/devregardless.The leak was functional, not cosmetic. In the sliced pod HAMi-core set
CUDA_DEVICE_MEMORY_LIMIT_0=40779m; reportedmemory.totalwas40779 MiBat index 0 and81559 MiBuncapped at indexes 1–7. That pod was allocatedGPU-a158631e, which was index 1 inside the container — index 0 was the MIG card. Once the device set is not narrowed, a container-local_LIMIT_0no longer names the assigned card.After
Same node, two real vLLM model instances (
Qwen3.5-0.8Bongpustack/runner:cuda12.8-vllm0.17.1):3g.40gbgpu.sliced: 1,sliced.memory-percentage: 50,sliced.cores-percentage: 50gpu.partitioned: 1,partitioned.mig-3g.40gb: 1privilegedfalsefalsenvidia-smi -Lmemory.total40779 MiBat index 0, which now is the allocated card[Insufficient Permissions](NVML refusing whole-card queries from inside a MIG container)running,2+2→4running,2+2→4Tests
tests/gpustack_runtime/deployer/test_privileged.py— 13 parametrized cases: no execution, privilege not requested, no resources, CPU/memory only, specific whole cards underenvand underkdp, all-devices underenv, auto-map underkdp, exclusive whole card, soft slice, hard partition, a non-NVIDIA soft slice, and the boundary casenvidia.com/gpu-alike(merely prefixed by a CDI kind — keeps its privilege).make test: 62 passed, 16 skipped.pre-commitclean on both files.Not in scope
In
envinjection mode a privileged container asking for specific devices is deliberately widened to every device and then narrowed at the backend layer viaCUDA_VISIBLE_DEVICES;docker.py/podman.pycarry the identical shape. That predates this change and leaks the same way on a multi-card host, but it is not the Kubernetes divided-mode path.