Skip to content

Kill on a finished execution can signal an unrelated process after PID reuse #1184

Description

@BatmanByte

What happens

Kill on a finished execution can signal an unrelated process, because nothing checks that the stored pid still belongs to that execution.

Two properties combine:

  1. A finished execution keeps its registry entry indefinitely. ExecutionRegistry is a flat HashMap<String, ExecutionState>, and the only removal path is release_ephemeral, which just the SSH bridges call (service/ssh/bridge.rs, service/ssh/reverse_streamlocal.rs). An SDK execution's entry — and its ExecHandle, and with it the leader pid — survives for the life of the guest.
  2. Signalling is unverified. ExecutionState::kill reaches ExecHandle::kill, which is a bare kill(self.pid, signal). ExecutionRegistry::shutdown_all has the same shape one level up: it probes with kill(pid, None) and signals afterwards, so the pid can change owner between the check and the signal.

Once the kernel recycles that pid number, a Kill RPC naming the old execution id, or a guest shutdown, signals whatever process now holds it.

Verified, not theoretical

Code side — a test against unmodified main shows all three preconditions hold: after the execution's process is killed and reaped, registry.get(id) still returns the state, state.get_pid() still returns the dead pid, and state.kill() still targets it. It returns false only because the pid happens to be unallocated at that instant — nothing verified identity.

Kernel side — a standalone C program on Linux 6.8 (pid_max 4194304) took a pid, killed and reaped its owner, then forked until the kernel handed that same pid to a new process: 3,347,452 forks, ~9.5 minutes. A bare kill(pid, SIGTERM) at that point — exactly what ExecHandle::kill does — killed the unrelated new owner.

Impact

Needs a long-lived box with enough process churn to wrap the pid space, so it is not trivially reachable. Reachability scales with how small pid_max is inside the guest: at the traditional Linux default of 32768 it takes ~32k process creations rather than ~4M, which a long-running box doing repeated execs can plausibly reach.

Scope

The single-pid signal paths (Kill with process_group=false, shutdown_all, the timeout watcher) are the ones addressed. Kill with process_group=true reaches kill(-pid) through ExecHandle::kill_process_group's own group-leader check, which is also unguarded; closing that needs a claim-guarded group primitive on Reaper and is left for a follow-up.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions