Skip to content

Add integration test for setuid runtime cleanup leak - #1404

Draft
jrray wants to merge 1 commit into
mainfrom
setuid-runtime-cleanup-leak-test
Draft

Add integration test for setuid runtime cleanup leak#1404
jrray wants to merge 1 commit into
mainfrom
setuid-runtime-cleanup-leak-test

Conversation

@jrray

@jrray jrray commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

What

Adds crates/spfs/tests/integration/privileged/test_runtime_cleanup_setuid_command.sh, a reproducer for a permanent runtime leak.

This test is expected to fail. It is the regression test for a bug that is not fixed yet, opened as a draft so the red build is not mistaken for a broken PR.

The bug

A runtime whose entry command is a setuid binary is never cleaned up.

spfs-monitor identifies the runtime's mount namespace by reading /proc/<owner>/ns/mnt. For a non-dumpable process that read does not fail the way the code expects — it succeeds and yields an empty string:

target pid 1306617, dumpable-0
  /proc/PID/ns owner/mode: dr-x--x--x root:root
  os.readlink -> SUCCESS, value=''  (len=0)
  control (self)          -> 'mnt:[4026531832]'

So identify_mount_namespace_of_process returns Some("") and the monitor adopts "" as the runtime's namespace identity. Every non-dumpable process on the host then compares equal to it, kernel threads included, so the tracked set can never empty and the runtime is never removed.

RetryOnPermissionDenied (monitor.rs:99-120) cannot catch this — it only fires on ErrorKind::PermissionDenied, and this call succeeds.

Why setuid makes it deterministic

Measured timings for the owner pid, sampled every 0.5ms:

Elapsed Owner state Event
+50 ms spfs-enter, ns reads '' exec of a file-capability binary → non-dumpable
+96 ms uid 0, ns reads '' become_root()
+100 ms uid 1000, ns reads mnt:[…] become_original_user() — readable again
+105 ms payload exec'd
+126 ms monitor reads the namespace

For an ordinary command this is safe by construction: become_original_user() runs inside initialize_runtime, and spawn_monitor_for_runtime is only called after it returns. Across 8 runs the monitor read 25.8–28.7 ms after the owner became readable (mean +27.2), safe every time.

A setuid payload is non-dumpable from its own exec, ~30 ms before the monitor reads, so it loses every time. A payload that self-marks non-dumpable at startup is a coin flip instead — one measured run lost by 3.0 ms and leaked, another won.

What the test does

Three stages, so a failure is unambiguous:

  1. Premise — runs the setuid helper directly and asserts its namespace link reads back empty. If a kernel or mount-option change alters that, the test says so rather than failing mysteriously.
  2. Control — an ordinary sleep runtime, asserted to clean up. Proves the fixture works.
  3. Reproducer — the same runtime with a setuid entry command. This is what fails.

Sample local output:

setuid payload ns link reads: ''
control ok: runtime with an ordinary command was cleaned up
FAILED: runtime with a setuid entry command was never cleaned up
  1 runtime(s) still in storage 30s after the command exited:
  f4ea72aa-...  running=true  status=running
  live monitors: 1
  monitor is tracking 379 pids as members of this runtime, expected 0

That last line comes from tracing the monitor at RUST_LOG=spfs=trace during the reproducer. Because cleanup never runs, spfs-enter --exit never truncates the log, so the evidence survives into CI output.

Notes

  • It lives in privileged/ because the bug needs both halves: root to create the setuid helper, and an unprivileged user to run spfs. A root monitor could read the payload's /proc and this would not reproduce.
  • Picked up automatically by run_privileged_tests.sh; no workflow change. Uses the user1 that runner already creates, overridable via SPFS_TEST_USER for local runs.
  • Waits with a timeout — the condition never becomes true, so the existing unbounded wait_for_* idiom would hang the suite.
  • Cleans up after itself via an EXIT trap: removes the leaked runtime, kills the stranded monitor, deletes the helper and its private storage root. It sorts last among privileged tests, so a stray monitor cannot perturb the others.

Fix direction

An empty namespace read means unknown, never an identity — hold previous state and retry. That needs to land together with explicit helper registration, because spfs-fuse is currently excluded from occupancy by this same empty-string behaviour; closing the hole alone would make fuse processes count as occupants and stop fuse runtimes from ever cleaning up.

🤖 Generated with Claude Code

@jrray
jrray force-pushed the setuid-runtime-cleanup-leak-test branch 5 times, most recently from 121b6eb to a8dd141 Compare August 18, 2026 01:51
A runtime whose entry command is a setuid binary is never cleaned up.

spfs-monitor identifies the runtime's mount namespace by reading
/proc/<owner>/ns/mnt. For a non-dumpable process that read does not fail
the way the code expects -- it succeeds and yields an empty string. So
identify_mount_namespace_of_process returns Some("") and the monitor
adopts "" as the runtime's namespace identity, which compares equal to
every non-dumpable process on the host. Its tracked set never empties
and the runtime is never removed.

A setuid entry command makes this deterministic rather than racy: the
payload is non-dumpable from its own exec, roughly 30ms before the
monitor reads. Measured locally, the monitor adopts ~380 unrelated pids
including kernel threads, and neither the runtime nor the monitor ever
goes away.

The test is expected to fail until this is fixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jrray
jrray force-pushed the setuid-runtime-cleanup-leak-test branch from a8dd141 to a69158b Compare August 18, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant