fix(spurd): kernel-enforced GPU device isolation across every launch path by biluriuday · Pull Request #814 · ROCm/spur

biluriuday · 2026-09-03T05:47:11Z

What this fixes

A job could open any GPU device node on its node, including one allocated to
another user's job. The existing *_VISIBLE_DEVICES deny was advisory — a job
could re-export the variable and regain access.

Closing it took two things: the kernel has to do the denying, and every process
a job runs has to be subject to it. Only the batch payload was in the job's
cgroup at all — srun steps, spur exec, and interactive attach ran in
spurd's own cgroup, so a filter attached to the job would have missed them.

Approach

spurd attaches a default-deny BPF_PROG_TYPE_CGROUP_DEVICE program to each
job's cgroup. The instructions are generated in-process and loaded through the
raw bpf() syscall — the technique systemd uses — so there is no eBPF build
step and no nightly toolchain. The allow-list is the job's allocated device
paths, plus standard pseudo-devices, plus host-infrastructure nodes that are
never per-job allocatable.

Containment was then extended so the filter actually covers everything.
Interactive allocations create the cgroup at RegisterJobAllocation, and every
process the agent starts joins it from pre_exec while still privileged, since
an unprivileged process cannot write another cgroup's cgroup.procs. That
includes both spawn shapes of spur exec, one of which had no pre_exec hook
at all because it drops privilege inside the namespace via setpriv.

Container jobs come along for free — they already live in the same job_N
cgroup, so there is no container-specific device code.

Design choices

Per-job program load, no BPF map. Matches Slurm's cgroup/v2 plugin. A
map-based design would trade one BPF_PROG_LOAD for MAP_CREATE plus
MAP_UPDATE and a hand-encoded lookup: more unsafe for no measured win on
a path dwarfed by fork/exec.
No detach call. BPF_PROG_ATTACH takes its own kernel reference, so
removing the cgroup at teardown frees the program. Both descriptors drop
before install returns; holding one would pin a program per job for spurd's
lifetime.
Access is a subset test (requested & !granted == 0), with the complement
spanning the full 16-bit access field so an access bit a future kernel adds
fails closed rather than being ignored.
Per-GPU compute nodes are deliberately absent from the host-infrastructure
list — gating those on the allocation is the security property. What is on
it (InfiniBand verbs, NVIDIA control/UVM/MIG, /dev/fuse) is shared by every
workload and owned by no allocation, so denying it breaks jobs that were never
reaching for a GPU.
Steps share the job cgroup rather than getting nested ones. Correct for
device isolation and much simpler; the cost is no per-step limits or
accounting.

Behavior changes

[1] constrain_devices defaults on, so a job can no longer open a device node
its allocation does not cover. A site needing an extra node can list it in
[cgroup] extra_device_paths instead of disabling the filter.

[2] A step now counts against the job's memory.max and cpuset, where it
previously ran unbounded. A site whose srun steps routinely overrun what the
job asked for will start seeing OOM kills.

[3] [cgroup] required now also gates allocation registration, not just launch.

[4] spurd raises its own RLIMIT_MEMLOCK at startup, since kernels before
5.11 charge BPF program memory against it. A job configured
rlimits.memlock = "inherit" therefore inherits the raised value; the
"unlimited" default is unaffected.

Testing

Unit tests execute the real generated instruction stream through a minimal BPF
interpreter rather than asserting structure, because the kernel verifier is not
reachable from CI. That caught a jump-overshoot bug where a mismatched rule
skipped the next rule's device-type check — a silent isolation hole that
structural assertions would have passed.

Validated on a bare-metal node using synthetic character devices in the local
major range declared as GRES, which separates the filter from a missing driver:
an allowed open fails ENXIO, a filtered one fails EPERM. A zero-GPU job was
denied both devices while /dev/urandom still worked; a one-GPU job opened its
own and was denied the other (same major, different minor); re-exporting
ROCR_VISIBLE_DEVICES did not restore access; the program was freed when the
cgroup went away, with no leak across twelve back-to-back jobs; and
required = true refused a launch when the filter could not be installed.

Workspace suite is at 3558 tests. The e2e suite adds 11 device-isolation cases,
which skip without GPU hardware.

Follow-ups

Nested per-step cgroups, for per-step limits and accounting. A filter attached
at job_N is inherited by descendants, so it keeps working unchanged.
seccomp and Landlock still reach only the batch path.
cleanup_cgroup calls remove_dir straight after SIGKILL, which returns
EBUSY until processes are reaped, so cancelling an allocation with live
steps can strand the directory and its program.
required is not verified on spur exec and attach; making the join fatal in
the child would close that race-free.

Copilot

🔵 Needs a closer look

It introduces a new kernel-enforced security boundary (BPF device filtering + cgroup containment across multiple launch paths), which warrants careful human review despite only a few targeted issues found.

Pull request overview

This PR hardens spurd’s isolation boundary by enforcing GPU/device-node access in the kernel (cgroup-v2 BPF device filter) and ensuring all job-related launch paths (batch, steps, spur exec, interactive attach, interactive allocations) actually execute inside the job’s cgroup so the filter and resource limits apply consistently.

Changes:

Attach a default-deny BPF_PROG_TYPE_CGROUP_DEVICE program to each job cgroup (with config-controlled allow-list extensions) and raise spurd’s RLIMIT_MEMLOCK to support older kernels.
Extend cgroup containment so non-batch paths (steps, spur exec, attach, interactive allocations) join the job cgroup via pre_exec while still privileged; add “required” gating for allocation registration / step join failures.
Add unit + e2e coverage plus operator documentation for rollout and upgrade considerations.

File summaries

File	Description
`tests/native_host/e2e/test_device_isolation.py`	New native-host e2e coverage asserting kernel-enforced device denial/allow across batch, steps, exec, and config toggles.
`examples/spur.conf`	Documents new `[cgroup]` knobs for `constrain_devices` and `extra_device_paths`.
`docs/deployment/upgrading.rst`	Adds upgrade guidance and rollout cautions for default-on device filtering.
`docs/deployment/native-host.rst`	Updates native-host deployment docs for new containment + device-filter behavior.
`docs/admin-guide/configuration.rst`	Documents new config fields and enforcement/required behavior; includes cgroup guidance.
`crates/spurd/src/main.rs`	Raises spurd’s own `RLIMIT_MEMLOCK` at startup and logs updated cgroup status fields.
`crates/spurd/src/job_entry.rs`	Extends `JobEntry` to carry the job cgroup path for downstream launch paths.
`crates/spurd/src/executor.rs`	Installs device filter during cgroup setup; introduces `CgroupJoin` for safe `pre_exec` cgroup membership.
`crates/spurd/src/device_cgroup.rs`	New module generating/loading/attaching the cgroup-device BPF program via raw `bpf()` syscalls + tests.
`crates/spurd/src/agent_server.rs`	Wires containment into exec/step/attach paths, creates cgroup at allocation registration, and handles required enforcement semantics.
`crates/spur-core/src/config.rs`	Adds `constrain_devices` + `extra_device_paths` to `CgroupConfig` with defaults + parsing tests.
`.wordlist.txt`	Adds new project vocabulary used in docs/comments/tests.

Review details

Files reviewed: 12/12 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…lter A job could open any GPU device node on its node, including one allocated to another user's job. The Phase 0 *_VISIBLE_DEVICES sentinel was advisory only: a job could re-export the variable and regain access. spurd now attaches a default-deny BPF_PROG_TYPE_CGROUP_DEVICE program to each job's cgroup. The instructions are generated in-process and loaded through the raw bpf() syscall, so no eBPF build step or nightly toolchain is needed. The allow-list is the job's allocated device paths plus standard pseudo-devices and host-infrastructure nodes (InfiniBand verbs, NVIDIA control/UVM/MIG, /dev/fuse) that are never per-job allocatable. Container jobs live in the same job_N cgroup, so they are covered with no container-specific code. Behavior change: constrain_devices defaults on, so a job can no longer open a device node its allocation does not cover. A site needing an extra node can list it in [cgroup] extra_device_paths instead of disabling the filter. The filter binds to the cgroup, so it covers the batch payload only: srun steps and spur exec run outside job_N and are not filtered. This is documented alongside the existing containment gaps rather than left implied. spurd also raises its own RLIMIT_MEMLOCK at startup, since kernels before 5.11 charge BPF program memory against it. Jobs configured rlimits.memlock = "inherit" therefore inherit the raised value; the "unlimited" default is unaffected. Verified on a bare-metal node with synthetic device nodes standing in for GPUs: an unallocated device returns EPERM while base pseudo-devices still work, re-exporting *_VISIBLE_DEVICES does not restore access, the program is freed when the job cgroup is removed, and required = true refuses the launch when the filter cannot be installed. Co-authored-by: Cursor <cursoragent@cursor.com>

…path Exercises the cgroup-v2 BPF device filter end to end: a zero-GPU job is denied the GPU control node while base pseudo-devices stay reachable, a job that was allocated a GPU can open it, re-exporting the *_VISIBLE_DEVICES selectors does not restore access, constrain_devices = false and extra_device_paths behave as documented, and the program is released when the job cgroup is removed. The probes target /dev/kfd rather than /dev/dri/renderD*. It reaches a job's allow-list only through an actual GPU allocation, and it lives outside /dev/dri, which the namespace wrapper replaces with a tmpfs — so a denial on a render node could be the tmpfs hiding it, while a denial on /dev/kfd can only be the filter. Two preconditions guard against passing for the wrong reason: the suite skips if /dev/kfd is not already openable by the test user outside a job, since file permissions would otherwise mask the filter, and the lifecycle test skips unless non-interactive sudo can actually run bpftool, which would otherwise report zero loaded programs and make its comparison trivially true. Device isolation is not covered on the Kubernetes path: spurd is not part of it, a SpurJob becomes a Pod created by the operator's virtual agent, and the kubelet owns the resulting cgroups. Co-authored-by: Cursor <cursoragent@cursor.com>

The comments that came with the device filter ran long: several inline blocks and most doc comments spanned four or more lines, against a project rule that caps a comment at two lines and treats three as a rare exception reserved for a design decision. Rewrites 48 blocks for density rather than dropping the reasoning. Every paragraph is now one or two lines and none sits even at the three-line exception, while the load-bearing explanations survive: the jump-resumption formula behind the offset patch, the access subset test and why its complement spans the full 16-bit field, why insn_cnt is derived inside the loader, the fd-drop contract that makes a detach call unnecessary, why per-GPU compute nodes are deliberately absent from the host-infrastructure list, and the MIG assumption behind granting /dev/nvidia-caps wholesale. Also removes a SPUR-192 task ID from a test comment, which AGENTS.md forbids in source — that context belongs in git history. No code changed; only comment and docstring text. Co-authored-by: Cursor <cursoragent@cursor.com>

Only the batch payload lived in the job's cgroup. An srun step, a spur exec, or an interactive attach ran in spurd's own cgroup instead, so none of them were bounded by the job's limits or checked against its device filter — a user could reach a GPU their job was never allocated from inside their own job. Interactive allocations had no cgroup at all, since RegisterJobAllocation starts no process. The job cgroup is now created when an allocation is registered, and every process the agent starts for a job joins it from pre_exec while still privileged, an unprivileged process being unable to write another cgroup's cgroup.procs. That covers both spawn shapes of exec, including the nsenter one, which previously had no pre_exec hook because it drops privilege inside the namespace via setpriv. Under [cgroup] required a step that failed to join is refused and its process group killed, rather than running unfiltered. The allocation's cgroup is released when the job leaves the agent's tracking. Behavior change: a step now counts against the job's memory.max and cpuset, where it previously ran unbounded. A site whose srun steps routinely overrun what the job asked for will start seeing OOM kills. Size --mem for the whole job, steps included, or raise allowed_ram_percent. [cgroup] required also gates allocation registration now, not just job launch: a node that cannot enforce refuses the allocation instead of accepting one it cannot serve. Steps share the job's cgroup rather than getting nested ones, so there are still no per-step limits or accounting and kill-by-step stays coarse. Nested job_<id>/step_<n> cgroups are future work; a device filter attached at the job is inherited by descendant cgroups, so it will keep working unchanged when they arrive. Agent-local throughout — no proto, config, or CLI surface moves. Co-authored-by: Cursor <cursoragent@cursor.com>

codecov-commenter · 2026-09-03T05:58:37Z

Codecov Report

❌ Patch coverage is 90.72967% with 155 lines in your changes missing coverage. Please review.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #814      +/-   ##
==========================================
+ Coverage   80.15%   80.47%   +0.32%     
==========================================
  Files         184      186       +2     
  Lines       87772    90206    +2434     
==========================================
+ Hits        70348    72591    +2243     
- Misses      17424    17615     +191

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

A zero-GPU job running with constrain_devices = false was denied /dev/kfd with EPERM — a device-filter denial, for a job that installs no filter at all. The job cgroup directory had outlived its job, and create_dir_all silently adopted it along with the BPF device program still attached. Nothing replaced that program, because this job was configured not to install one. The directory survived because cleanup_cgroup called remove_dir immediately after SIGKILLing the cgroup's members, and rmdir returns EBUSY until those processes have actually left it: kill returning does not mean the process is gone. setup_cgroup now attempts a plain remove_dir before creating the directory. A bare rmdir is the right tool precisely because it fails EBUSY on a cgroup that still holds processes, so it can only succeed on a genuinely stale directory and cannot disturb a live run that a re-dispatch gave the same job id. cleanup_cgroup retries removal for up to 200ms, bounded because the monitor loop holds the running-jobs lock across the call. The bug failed closed rather than open: when the filter is enabled the job installs its own program and an exclusive attach replaces the stale one, so the effect was a job more restricted than intended, not one with access it should not have. Found by the e2e device-isolation suite on GPU hardware, where every test starts a fresh controller and so reuses job id 1 against a node-global cgroup path. Co-authored-by: Cursor <cursoragent@cursor.com>

… lock The monitor loop extracted the completed jobs under the `running` lock and then held it across the whole teardown: rootfs and spool removal, cgroup cleanup, the allocation release and the MPI plugin hook. Cgroup cleanup SIGKILLs the group's stragglers and then blocks retrying `rmdir` until they exit, up to 200ms per job, because the kernel returns EBUSY while any process is left. Every other task that wants the map -- a launch committing, a cancel, a status query -- waited behind that, and the blocking sleep sat on a tokio worker thread rather than yielding it. Removal and teardown are now separate. The loop drops each entry from the map, releases the lock, and only then tears the jobs down. `remove_tracked_job` hands its cgroup back as a `#[must_use]` `CgroupGuard` instead of removing it inline, so every caller picks the point where the blocking removal is acceptable; `refuse_allocation` returns one alongside the status for the same reason. The reconcile pass re-takes the lock on its own, in the same order `commit_job` uses. Splitting them opens a window where the id is out of the map but its state is not yet released, and the controller can re-dispatch that id into it. The new run derives the same `job_<id>` cgroup and holds the reservation, so teardown re-checks `running` and leaves everything alone if the id is tracked again. Co-authored-by: Cursor <cursoragent@cursor.com>

Two pages still described the pre-containment world: the `[cgroup]` section said the settings bound the batch payload only and that `srun` steps, `spur exec` and interactive attach run outside the job cgroup, and the GPU Isolation section said the same of the device filter -- that steps and exec shells keep host-wide device access. Steps, exec and attach all join the job cgroup now, so both passages told operators to expect an escape hatch that is closed, the GPU one implying device isolation is trivially bypassable from inside a job. Both now say the cgroup and its filter cover every process the agent starts for a job. The `[cgroup]` warning is re-pointed at the caveat that does still apply: enforcement needs a root agent, and `required` defaults to false, so a host that cannot apply a constraint warns and runs the work unconstrained anyway. The GPU section carries the same caveat for the filter's `CAP_BPF` / `CAP_SYS_ADMIN` requirement. Both keep pointing at the containment-gaps section for what genuinely remains -- per-step granularity and the site-supplied task hooks. Co-authored-by: Cursor <cursoragent@cursor.com>

The handler took `uid`, `gid` and `user` straight off the wire onto the tracked job, and it was the one state-changing agent RPC with no authorization gate. `exec_in_job` and `interactive_session` then authorize against that `user` while resolving the uid they execute as from the same record, so the two fields could be set independently: register an allocation naming yourself as owner and another account's uid, and the ownership check passes while the step runs as that account. `check_root_execution_allowed` still refuses uid 0 unless the operator opted in, so the reach is every non-root account on the node. Registering also reserves and commits the node's CPUs and GPUs, so the same call let an unauthenticated caller squat a node's resources and desynchronize the controller's view of it. Gate it with `require_controller`, as every sibling already is. The only production caller is the controller's `register_allocation_to_agent`, over the same client `run_command` uses, so `srun` and `salloc` are unaffected. Note the gate still admits an unauthenticated caller under `permissive`, matching the rest of the agent — this closes the hole for `auth.mode = required` and brings the RPC to parity, it does not by itself make the agent port safe. Co-authored-by: Cursor <cursoragent@cursor.com>

…tach - Serialize per-job setup and teardown so a re-dispatched id cannot have its cgroup torn down by the prior run, and refuse a populated cgroup rather than adopt its device filter. - Fail the exec and interactive-attach pre-exec when `[cgroup] required` and the cgroup join does not land, instead of running outside the device filter. - Correct the capability guidance: a cgroup-device program needs CAP_BPF and CAP_NET_ADMIN (or CAP_SYS_ADMIN), not CAP_BPF alone. - Add regression tests for same-major/different-minor device denial and the exec required-join. Co-authored-by: Cursor <cursoragent@cursor.com>

Cgroup: OOM group/single-kill, disabled master switch, leftover root-owned cgroup degrade regression, and an srun step bound by the job's memory.max. Device filter: partial-GPU sibling hiding, zero-GPU container denial, and --pty attach denial. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI lite review requested due to automatic review settings September 3, 2026 05:47

biluriuday requested review from sajmera-pensando, sgopinath1, shiv-tyagi and yansun1996 as code owners September 3, 2026 05:47

Copilot started reviewing on behalf of biluriuday September 3, 2026 05:47 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread crates/spurd/src/agent_server.rs Outdated

Comment thread docs/admin-guide/configuration.rst Outdated

Comment thread docs/deployment/native-host.rst Outdated

biluriuday and others added 4 commits September 3, 2026 05:53

biluriuday force-pushed the cg-p1b branch from 6028d1b to b55479c Compare September 3, 2026 05:53

biluriuday and others added 5 commits September 3, 2026 07:18

biluriuday force-pushed the cg-p1b branch 2 times, most recently from 3a463bd to a247807 Compare September 3, 2026 19:01

biluriuday force-pushed the cg-p1b branch from a247807 to 4745a8e Compare September 3, 2026 19:25

Navigation Menu

Uh oh!

fix(spurd): kernel-enforced GPU device isolation across every launch path - #814

fix(spurd): kernel-enforced GPU device isolation across every launch path#814
biluriuday wants to merge 10 commits into
ROCm:mainfrom
biluriuday:cg-p1b

biluriuday commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 3, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

biluriuday commented Sep 3, 2026

What this fixes

Approach

Design choices

Behavior changes

Testing

Follow-ups

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Sep 3, 2026 •

edited

Loading