DF-3534 — netgraph7 ng_tty: ngt_shutdown() never drops the constructor

node reference — every ng_tty node ever created leaks

What

In netgraph7 the node-teardown contract (see ng_rmnode, sys/netgraph7/netgraph/ng_base.c:686-751) is: when a node type supplies a shutdown method, ng_rmnode() drops only its own extra reference; the constructor (creation-time) reference must be dropped by the shutdown method itself. ng_pred1 in the same tree documents the idiom explicitly:

sys/netgraph7/ng_pred1.c:349-351
    kfree(priv, M_NETGRAPH_PRED1);
    NG_NODE_SET_PRIVATE(node, NULL);
    NG_NODE_UNREF(node);        /* Let the node escape. */

ng7 ngt_shutdown() (sys/netgraph7/tty/ng_tty.c:553-571) frees sc but never calls NG_NODE_UNREF(node) (and never clears the private pointer) — so after ngt_close() tears the line discipline down, the node's refcount stops at 1 and the ng_node is never freed:

leak = one ng_node (~176 B, M_NETGRAPH_NODE) + its ID-hash entry
       per netgraph-tty ldisc install/removal, forever.

The v1 twin got this right (sys/netgraph/tty/ng_tty.c:554 does ng_unref(sc->node)); the ng7 port dropped the call.

Trigger (root: ngt_open requires SYSCAP_NONET_NETGRAPH)

Bare minimum — no hook, no data, no timer needed:

posix_openpt(); grantpt(); unlockpt();
slave = open(ptsname(master));
ioctl(slave, TIOCSETD, &(int){6});    /* NETGRAPHDISC  -> ngt_open  */
ioctl(slave, TIOCSETD, &(int){0});    /* back to termios -> ngt_close */
close(slave); close(master);          /* -> one ng_node leaked */

Repeatable without bound; the accumulating nodes permanently pin the ng_tty module (kldunload: Device busy), squat the node ID/name hashes and — as DF-3513/DF-3514 already established — push ng_nodes across the ng7 hash-rehash panic threshold (on stock module builds) turning routine ldisc churn into panic: hashdestroy: hash not empty.

Isolation proof (isolation.log, tty_leak2.c)

On a kernel/module with the DF-3531 ng_uncallout item leak already fixed (so items are flat and cannot mask anything):

nodd 500      (ldisc on/off only)   nodes 3 -> 503   (+1/cycle)
nodata 500    (+ socket connect)    503 -> 1003      (+1/cycle)
socksock 500  (socket<->socket, no tty) 1003 -> 1003 (FLAT)
full 500      (+ data)              1003 -> 1.47K    (+1/cycle)

=> the leak is the tty ldisc lifecycle alone; the generic connect/data path is clean; the chand callout is irrelevant (nodd leaks with no timer ever armed).

Fix (fix.diff — validated)

Add NG_NODE_SET_PRIVATE(node, NULL); NG_NODE_UNREF(node); at the end of ngt_shutdown (mirroring ng_pred1.c:350-351). Validated on the guest (fix_validation.log): nodd 500 and 2x2000 stall-close cycles all leave netgraph_node in-use flat at 2 (was +1/cycle); netgraph_hook stays 0.

Build / Run

cc -O2 -o tty_leak2 tty_leak2.c
vmstat -m | grep netgraph_node
./tty_leak2 nodd 500
vmstat -m | grep netgraph_node      # stock: +500

DF-3534 — VERDICT

REPRODUCED (leak, certain). Discovered during the DF-3531 Phase V when the post-fix node counts refused to go flat: a refcount ledger (instrumented ng_unref_node + inlined _NG_NODE_REF with return-address tagging) showed the tty node acquiring 9 refs and releasing only 8 in a single connect+data cycle, with the orphan being the creation-time reference — and isolation runs then proved the leak needs nothing but a ldisc install/remove.

Mechanism

ng7 ng_rmnode() (ng_base.c:686-751) unrefs only its own extra reference when the type has a shutdown method; the constructor ref is the shutdown method's job (ng_pred1.c:351 does exactly this). ng7 ngt_shutdown (tty/ng_tty.c:553-571) never does NG_NODE_UNREF(node) → after ngt_close the node refcount parks at 1 forever.

Ledger excerpt (1 cycle, node 0xfffff8004f1e87f0; full trace in the DF-3531 verification session): 9 NGREF acquisitions (creation via ngt_open→ng_make_node_common; hook; timer items; connect-path noderef; rmnode extra; …) vs 8 NGUNREF releases — last observed state refs=1, never freed. Socket nodes in the same cycles reach -> FREED — only the tty nodes strand.

Reproduction numbers

Stock-equivalent behavior (kernel #1 + layer-2 module, i.e. the DF-3531 item leak already neutralized — see isolation.log):

nodd 500:     netgraph_node 3 -> 503    (+1/cycle, no hook/data/timer)
nodata 500:   -> 1003                   (+1/cycle)
socksock 500: -> 1003                   (FLAT — generic path clean)
full 500:     -> 1.47K                  (+1/cycle)

On the fully stock kernel/module the effect is the same +1 node/cycle stacked under DF-3531's item leak (baseline run.log of DF-3531: netgraph_node 2 → 1.96K after 2000 stall cycles).

Fix validation

fix.diff = 2 lines (SET_PRIVATE(NULL) + NG_NODE_UNREF) in ngt_shutdown, module rebuild only. Result (fix_validation.log, kernel #1 + layer-2 + this fix):

nodd 500:                 nodes 2 -> 2   (FLAT)
stall-close 2000 x2:      nodes 2 -> 2 -> 2 (FLAT), items 9 in-use flat
netgraph_hook in-use 0 throughout

The patched state later panicked during DF-3531's drain-mode stress with memory chunk already free — that is the DF-3531 layer-1/layer-2 close-vs-fire race (double-free of an in-flight timer item), NOT this fix: nodd-mode (no timer at all) ran clean and flat, and the same panic signature was reproduced on a build without the ngt_shutdown change.

Environment

Same guest/session as DF-3531 (see ../DF-3531/env.txt).

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.