# DF-3521 — ng_pptpgre timer-accounting leak via broken `callout_stop()` contract

**Verdict: REPRODUCED (in-guest, deterministic, quantified). Fixed by 1-line
`kern_timeout.c` fix validated with a rebuilt kernel.**

`status = reproduced`, `reproduced = 1`, `impact = leak`, `confidence = certain`.

## What leaks and why

`ng_pptpgre` pairs every armed timer with exactly one cleanup, gated on
`callout_stop()` returning non-zero:

```c
/* sys/netgraph/pptpgre/ng_pptpgre.c:786-791 */
if (callout_stop(&a->rackTimer)) {
        kfree(a->rackTimerPtr, M_NETGRAPH);   /* :787 */
        priv->timers--;                       /* :788 */
        ng_unref(node);                       /* :789 */
}
a->rackTimerPtr = NULL;                       /* :791 */
```
(same pattern for the send-ack timer at `:889-894`).

The pairing is correct **only if** `callout_stop()` returns non-zero when it
cancels an armed-but-not-yet-fired callout (the documented contract). On this
kernel it returns **0** for exactly that case, because the stop is completed
*inline* by `_callout_update_spinlocked()` (kern_timeout.c:266-277: removes
the callout from the wheel, clears `CALLOUT_SET`, sets `CALLOUT_PREVENTED`)
and the early exit at `sys/kern/kern_timeout.c:888-891`

```c
if (sync == 0 || (c->flags & (CALLOUT_INPROG | CALLOUT_SET)) == 0) {
        exis_drop_gd(gd);
        return 0;                    /* <-- SET was just cleared inline */
}
```

fires **before** the `PREVENTED`-reporting path at `:895-900`. This is the
same contract violation DF-3515 live-instrumented on `ng_hci`; DF-3515 fixed
the ng7 consumer (`ng_uncallout`); **the v1 `ng_pptpgre` consumer
(`if (callout_stop(...))`) has the identical dependency and leaks
deterministically** — this was not covered by DF-3515 or DF-0596.

Consequence chain (single CPU, no race needed):

1. `xmit` arms `rackTimer` (`ng_pptpgre.c:762-774`: `kmalloc` +
   `node->refs++` + `priv->timers++` + `callout_reset`).
2. Any of the *normal* stop sites fires while the timer is armed:
   * ack received → `ng_pptpgre_stop_recv_ack_timer` (`:679`) — happens on
     every ack of a live PPTP session;
   * piggy-backed ack on xmit (`:525`);
   * `NGM_PPTPGRE_SET_CONFIG` → `ng_pptpgre_reset` (`:977-978`).
3. `callout_stop()` returns 0 although it cancelled the pending timer → the
   `kfree`/`timers--`/`ng_unref` block is skipped, `rackTimerPtr` is only
   NULLed → **callback will never run** → nobody ever does the cleanup.
4. Per stop: one 8-byte `M_NETGRAPH` allocation, one node reference, and one
   `priv->timers` unit leak — permanently.
5. At shutdown, `ng_pptpgre_rmnode` (`:437`) `if (priv->timers == 0)` never
   succeeds → the ~208-byte `priv` leaks too, and the leaked node reference
   pins the `ng_node` (and its ID/name) forever. The NG_INVALID-branch free
   in the callbacks (`:813-816`, `:917-920`) can't happen either — the
   callback was cancelled, not fired.

## Trigger (root-local; netgraph control socket is gated by
`caps_priv_check(SYSCAP_RESTRICTEDROOT)` at sys/netgraph/socket/ng_socket.c:172)

`leak_test.c` builds: `ctlA:p0 <-> pptp:upper`, `ctlB:p1 <-> pptp:lower`,
then per cycle: one 64-byte frame through `upper` (arms `rackTimer` with a
~10 s `ato`), then `NGM_PPTPGRE_SET_CONFIG` (reset → stop armed timer →
leak). Mode 1 additionally tears the node down each cycle (full lifecycle).

## Measured (stock kernel, guest up, INVARIANTS `X86_64_GENERIC #0`)

```
baseline (after 500-cycle smoke) : netgraph  Count=503    4.50K
after 30,000 arm/cancel cycles   : netgraph  Count=29.8K  239K    (long-lived node)
after  3,000 full lifecycles     : netgraph  Count=38.6K  1.54M
```

* ~1 leaked allocation per cancelled timer (29.3k for 30k cycles) — exactly
  the leaked `rackTimerPtr`.
* ~2.9 allocations / ~440 bytes per node lifecycle — `rackTimerPtr` +
  `priv` + `node`(+name) — proving the `priv->timers` never drains and
  `ng_pptpgre_rmnode`'s free is skipped.
* Contrast: the DF-3518 hammer ran 52k node lifecycles **without armed
  timers** (`vmstat -m` Count unchanged) — the leak appears iff a timer was
  armed and then stopped. Deterministic, not a race.

## Fix

`fix.diff` — `sys/kern/kern_timeout.c`: in `_callout_cancel_or_stop()`,
when the request completed inline (neither `INPROG` nor `SET` remains),
return `(c->flags & CALLOUT_PREVENTED) != 0` instead of 0. This restores
the `callout_stop(9)` contract for every consumer (and is the same
direction as DF-3515's root-cause note). Validated: kernel rebuilt in-guest
with the fix, `leak_test` re-run — see `fix_run.log`; leaked-count growth
per cycle dropped to **0**.

## Files

* `leak_test.c` — PoC (build: `cc -O2 -lpthread -o leak_test leak_test.c`)
* `build.log`, `run.log` — full untrimmed build + stock-kernel run
* `fix_run.log` — post-fix run (fixed kernel)
* `fix.diff` — `git apply`-able against the read-only master tree
* `env.txt` — guest kernel/modules/compiler

## Privilege / exposure

Root (or delegated `SYSCAP_RESTRICTEDROOT`/netgraph caps) is needed to build
the graph — same prerequisite class as DF-0589/0590/0596. Impact is
unbounded kernel-memory exhaustion under normal PPTP-session operation
(every ack-restart cycle leaks), degrading the whole system — kernleak
bucket.

## Fix status (batch validation)
Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest `make -j6 nativekernel KERNCONF=X86_64_GENERIC` completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.