# DF-3519 — non-atomic `node->refs++` / `priv->timers++` in ng_pptpgre timer start vs `ng_unref` under crit-only

**Verdict: NOT REPRODUCED on this guest — instruction-level SMP race with
the same CPU-0 netisr vector blocker as DF-0590/DF-3518; code-confirmed
non-atomic RMW pair.**

`status = not_reproduced`, `reproduced = 0`, `impact = none (on this
vector)`, `confidence = certain (code-level)`.

## The defect (code trace)

The timer-start paths do bare, non-atomic increments of two shared
fields:

* `sys/netgraph/pptpgre/ng_pptpgre.c:768-769`
  (`ng_pptpgre_start_recv_ack_timer`):
  ```c
  node->refs++;            /* :768 — plain RMW, no atomic, no crit */
  priv->timers++;          /* :769 — plain RMW */
  ```
* `sys/netgraph/pptpgre/ng_pptpgre.c:871-872`
  (`ng_pptpgre_start_send_ack_timer`): same pair.

The matching decrements run in other contexts:
* `ng_unref()` — `sys/netgraph/netgraph/ng_base.c:474-485`:
  `crit_enter(); if (--node->refs <= 0) ...` — crit sections are
  per-CPU on DragonFly; they do **not** exclude a plain `++` executing
  on another CPU.
* `priv->timers--` in the two timeout callbacks
  (`:811`, `:915`) and in the stop paths (`:788`, `:891`).

Two CPUs doing `++refs` (timer start) and `--refs` (any unref, e.g. the
*previous* timer instance's callback or a message-dispatch release) on
the same plain int lose one update → `refs` undercounts → the node is
freed while the new timer's callback/item still dereferences it
(`*((node_p *)arg)` at `:802`/`:906`, then `node->private`, `->flags`,
`ng_unref`) → **UAF on `struct ng_node`** (~120 B M_NETGRAPH chunk,
containing `type` pointer and LIST links — `ng_unref`'s
`LIST_REMOVE(node, nodes/idnodes)` on freed memory corrupts the global
node hash lists). The same lost-update on `priv->timers` biases the
drain count (leak direction; a double-zero was analyzed and is NOT
reachable because `ng_pptpgre_rmnode`'s reset uses synchronous
`callout_stop` before checking `timers`).

This is DF-0492's "Refcount also racy" finding instantiated for
ng_pptpgre — DF-0596 does not cover it (it is scoped to the
`ackp`/`xmitWin` state TOCTOU).

## Trigger model

Timer start/stop churns once per ack in a live session
(`:679-681` stop+restart per received ack). With the data path
dispatching on multiple CPUs (ng_ether per-flow input — the realistic
VPN-concentrator deployment; NOT reachable from ng_socket, which
serializes on CPU 0's netisr port per `uipc_socket.c:254-259`), the
`++`/`--` pair executes concurrently → lost update. Window is a couple
of instructions; requires sustained SMP traffic.

## Fix

Use the framework's atomic helper — replace both `node->refs++` with
`NG_NODE_REF(node)` (atomic_add_int, netgraph.h:132) and make
`priv->timers` atomic (`atomic_add_int(&priv->timers, 1)` /
`atomic_add_int(&priv->timers, -1)`), or (better) serialize the node
with a per-node lock as recommended for DF-0590/DF-3518, which removes
the whole class.

## Why no in-guest run

The race requires concurrent timer start/stop from multiple CPUs; from
userspace every ng_socket send (data and control) is processed
serially on CPU 0's netisr port (see DF-0590 VERDICT §2, empirically
demonstrated with millions of frames; DF-3518's 52k-lifecycle hammer
re-confirmed for this node type). A dedicated run would reproduce the
same negative; the code-level defect is certain.

## Files

* `VERDICT.md` (this file) — the full analysis; no binary PoC for the
  reasons above.

## 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.

(No fix.diff for this item: documented recommendation only - see Fix above.)