# DF-2980 — tokenless `stopprofclock()` in `addupc_task()` (non-atomic `p_flags` RMW race)

**File:** `sys/kern/subr_prof.c` (pass-2 finding; DF-0251 / DF-0252 known, not affected here)
**Severity:** Medium · **Confidence:** certain (mechanism reproduced + fix-validated)
**Class:** CWE-667 (improper locking) / CWE-362 (race) — concurrent non-atomic RMW on
`struct proc::p_flags` from the profiling AST/syscall-return path.

## What it is

`addupc_task()` calls `stopprofclock(p)` at `sys/kern/subr_prof.c:145` **without holding
`p->p_token`**, violating `stopprofclock()`'s documented contract
(`sys/kern/kern_clock.c:1303-1305` "caller must hold p->p_token"). `stopprofclock()` does a
non-atomic `p->p_flags &= ~P_PROFIL` (`kern_clock.c:1310`; `p_flags` is a plain `int`,
`sys/sys/proc.h:241`). Every other caller (sys_profil `subr_prof.c:61-76`, exec
`kern_exec.c:226→444`, exit `kern_exit.c:304→365`) holds the token; `addupc_task` is the
sole violator, and both of its call sites (`platform/pc64/x86_64/trap.c:231` in `userret()`
and `trap.c:507` on the `T_ASTFLT`/`RQF_AST_OWEUPC` path) are tokenless.

Amplifier: `userret()` calls `addupc_task()` on **every syscall exit** of a `P_PROFIL`
process. A thread that keeps re-arming `profil(2)` with an unmapped sample buffer
(`pr_base` in dead user space) makes every `copyin()` in `addupc_task()` fault, so the
tokenless `p_flags &= ~P_PROFIL` executes at **syscall rate (MHz)**, entirely unprivileged.

When one of those stale RMW stores lands on top of a token-held writer's store — e.g.
`PT_ATTACH`'s `p->p_flags |= P_TRACED` (`sys/kern/sys_process.c:305`) or its
`&= ~(P_TRACED|P_WAITED)` clear at detach (`sys_process.c:362`), or `setsugid()`'s
`|= P_SUGID` (`kern_prot.c:1304`, called on setuid exec at `kern_exec.c:493`) — the
concurrent update is **annihilated**. P_SUGID gates ptrace attach (`sys_process.c:202-204`),
sugid coredumps (`kern_sig.c:2543`) and ktrace (`kern_ktrace.c:680`); P_TRACED gates the
whole debug relationship; P_CONTINUED gates WCONTINUED reporting. Losing the P_SUGID set on
a setuid-root exec would let a same-ruid parent ptrace a root-euid image — a speculative
full privesc chain (nanosecond window inside `setsugid()`; not demonstrated).

## PoC (100% unprivileged)

- `poc.c` — victim child (2 threads) loops `profil(0x400000000000, 4 GiB, 0, 1.0)` into an
  unmapped window; the tracer parent (same uid) hammers `PT_ATTACH`/stop/`PT_DETACH`.
  Hit signatures, both false-positive-free:
  - **HIT1** `PT_DETACH == EPERM` after a successful `PT_ATTACH` + observed stop ⇒ the
    `|= P_TRACED` set was annihilated (`sys_process.c:268-271` gate);
  - **HIT2** `PT_ATTACH == EBUSY` immediately after a successful `PT_DETACH` ⇒ the detach's
    `&= ~(P_TRACED|P_WAITED)` clear was annihilated and P_TRACED resurrected
    (state-verified: child `kp_stat=SACTIVE` with P_TRACED still set — no `tstop()` can be
    in flight while the process is fully running, so the only tokenless `p_flags` writer in
    that window is `addupc_task`→`stopprofclock`).

## Build / run (guest)

```
cc -O2 -Wall -pthread -o poc poc.c
./poc 300000        # exit 0 = RACE HIT, 2 = no hit within budget
```

## Results

| kernel | result |
|---|---|
| baseline `#0` Jul 2 2026 | HIT2 after 99310 cycles (1.9 s); 300000-cycle run no-hit; HIT2 after 16166 cycles (0.33 s) — 2/3 runs |
| patched `#1` Sep 4 2026 (fix.diff: `lwkt_gettoken(&p->p_token)` around the `stopprofclock()` in `addupc_task`) | sanity: profiling still accumulates samples; 3 × 1,000,000 cycles (~49 s, ≈10× baseline exposure) — **zero hits** |

## Fix

`fix.diff` — take `p->p_token` around the `stopprofclock()` call in `addupc_task()`
(thread context only; both call sites hold no conflicting token; `userret()` itself
acquires `p_token` a few lines later at `trap.c:247`, so this is an established pattern).

Suggested hardening beyond this finding: `p_flags` has several RMW writers that are not
mutually serialized (`tstop()` clears P_WAITED under the *parent's* token at
`kern_synch.c:1368` while ptrace writes under the child's token) — a wider cleanup would
make `p_flags` updates atomic (e.g. `atomic_clear_int`) or consistently token-interlocked.