DF-4342 — ngmn_newhook channel aliasing: state-DOWN reuse lets two live hooks share one schan (nhooks imbalance, silent channel teardown, double-free enabler)

• File: sys/dev/netif/mn/if_mn.c
• Lines: 470-479 (newhook EBUSY condition), 474-475 (ts/hook overwrite), 664-666 (connect silent-success), 756-758 (disconnect early-return), 795 (nhooks decrement), 308-310 (ngmn_config line-change guard)
• Severity: Low (hardware-gated + root netgraph; deterministic logic corruption, concurrency follow-on)
• CVSS: CVSS:3.1/AV:L/AC:L/PR:H/UI:N/S:U/C:N/I:L/A:L
• CWE: CWE-667 (improper locking — here: absent serialization of channel ownership) / CWE-415 as follow-on
• Confidence: certain (deterministic from code)

Summary

ngmn_newhook only refuses a second hook on a channel when that channel is UP:

if (!sc->ch[chan])
    mn_create_channel(sc, chan);
else if (sc->ch[chan]->state == UP)
    return (EBUSY);         /* if_mn.c:470-473 */
sc->ch[chan]->ts = ts;          /* overwrites */
sc->ch[chan]->hook = hook;      /* overwrites — old hook unrecorded */
hook->private = sc->ch[chan];
sc->nhooks++;               /* counted again */

Two distinct hook names map to the same channel — e.g. in E1 framing ts2 and ts2-3 (both ffs(ts)-1 == 2, if_mn.c:466-467), or in E1U any two valid 32-bit timeslot specs (both hardcode chan = 1, if_mn.c:468-469). When the first hook is still DOWN, the second newhook silently aliases the same schan: sch->hook is overwritten while the first hook's hook->private still points at the same sch, and ts is replaced.

Both hooks can then reach UP: ngmn_connect(hook2) arms the channel; ngmn_connect(hook1) sees sch->state == UP and returns 0 silently (if_mn.c:664-665) — the caller believes it is connected. Both hooks' rcvdata interleave onto the same wire channel.

Tearing hook1 down executes the full teardown (ngmn_disconnect, if_mn.c:756-794) while hook2 is still core-connected: rings freed, state DOWN. From then on:

• hook2's data is silently blackholed (ngmn_rcvdata:586-589 drops on state != UP);
• hook2's own disconnect early-returns at if_mn.c:756-758 before sc->nhooks-- (if_mn.c:795), so nhooks was incremented twice and decremented once — permanently positive;
• ngmn_config refuses any line reconfiguration forever (if_mn.c:308-310: "Cannot change line when %d hooks open").

Concurrency follow-on (links to DF-4341)

Because nothing serializes the two ngmn_disconnect calls (see DF-4341: no locks anywhere), two threads disconnecting the two aliased hooks concurrently can both pass the sch->state == DOWN check (if_mn.c:756) before either sets DOWN (:758) — both then execute the full teardown and double-free every descriptor on both rings (mn_free_desc, if_mn.c:779-794). Worse, mn_free_desc clobbers dp->vnext with the freelist head (if_mn.c:498-503), so the second thread's walk (dp2 = dp->vnext, if_mn.c:782/792) follows the freelist pointer and frees arbitrary pooled descriptors — M_MN pool-wide corruption without needing any ISR timing.

Threat

Actor: root with netgraph control (same bar as DF-1539/1540/1541); hardware: the mn card. The aliasing itself is deterministic — a plain sequence of ngctl mkpeer/connect/rmhook commands (below) with no racing required produces the blackhole + permanent nhooks drift (local DoS of line administration). The double-free variant needs two concurrent rmhook syscalls (trivially scripted).

Exploit / PoC

trigger_alias.sh (run as root on hardware, node mn0, E1 framing):

ngctl mkpeer mn0: echo ts2 downstream     # hook A on chan 2 (DOWN)
ngctl mkpeer mn0: echo ts2-3 downstream2  # hook B: EBUSY check skipped (DOWN), aliases chan 2
ngctl show mn0:                           # both hooks listed; sch->hook == B
ngctl rmhook mn0: ts2                     # full teardown while B connected
ngctl send mn0: ...                       # B's data silently dropped forever
ngctl msg mn0: textconfig "line e1u"      # refused forever: nhooks stuck > 0

Success criterion (deterministic): ngmn_config error "Cannot change line when 1 hooks open" while ngctl show mn0: lists zero connected hooks; B's tx counters frozen at 0 despite sends.

Double-free variant: ngctl rmhook mn0: ts2 & ngctl rmhook mn0: ts2-3 & concurrently → watch for mn_alloc_desc handing the same trxd to two channels / pool corruption panic.

Not executable on the audit guest — no Munich32X hardware, driver not built (see env_guest.txt / run.log; VERDICT.md).

Recommended fix

Track channel ownership by the hook pointer, not by the UP/DOWN state: clear sch->hook in ngmn_disconnect (which also closes DF-1540's dangling read) and refuse a newhook whenever sc->ch[chan]->hook != NULL. See fix.diff:

@@ ngmn_newhook(...)
-   if (!sc->ch[chan])
-       mn_create_channel(sc, chan);
-   else if (sc->ch[chan]->state == UP)
-       return (EBUSY);
+   if (!sc->ch[chan])
+       mn_create_channel(sc, chan);
+   else if (sc->ch[chan]->hook != NULL)
+       return (EBUSY);     /* a hook already owns this channel */
@@ ngmn_disconnect(...)
    if (sch->state == DOWN)
        return (0);
    sch->state = DOWN;
+   sch->hook = NULL;   /* one owner at a time; also closes DF-1540 */

DF-4342 VERDICT

Status: inconclusive on this guest (finding stands on deterministic code reading) reproduced: 0 · impact on this guest: none · confidence in the finding: certain

What was verified statically (deterministic, no hardware semantics assumed)

1. Two distinct hook names alias one channel: - E1: chan = ffs(ts) - 1 (if_mn.c:466-467) — "ts2" and "ts2-3" both give ffs(...)-1 == 2 (verified against mn_parse_ts, if_mn.c:505-540: "2-3" sets bits 2 and 3). - E1U: every valid hook hardcodes chan = 1 (if_mn.c:468-469). - The netgraph core does not filter this: hooks are keyed by name only (ng_add_hook / ng_findhook, sys/netgraph/netgraph/ng_base.c:697-763).
2. The second newhook is admitted while the first is DOWN: the EBUSY branch tests sc->ch[chan]->state == UP (if_mn.c:470-473) — state DOWN falls through to sc->ch[chan]->hook = hook; (:475) overwriting the first hook's back-pointer while hook1->private still references the same schan, and sc->nhooks++ (:478) counts a second owner.
3. Second connect is a silent success: ngmn_connect returns 0 early on sch->state == UP (if_mn.c:664-665) with no error to the caller.
4. First disconnect executes the full teardown regardless of the second hook (if_mn.c:756-794); the second disconnect early-returns BEFORE sc->nhooks-- (:756-758 vs :795) → permanent nhooks surplus.
5. Permanent admin DoS: ngmn_config gates every line change on sc->nhooks > 0 (if_mn.c:308-310).
6. Double-free enabler: nothing serializes two concurrent ngmn_disconnect calls (DF-4341 evidence: ng_disconnect_hook ng_base.c:839-858 takes no lock); both pass the state == DOWN? check at if_mn.c:756 before either stores DOWN at :758; both then run the free loops :779-794. mn_free_desc overwrites dp->vnext with the freelist head (:498-503), so the loser's dp2 = dp->vnext (:782, :792) walks onto the global freelist and frees live descriptors belonging to other channels.

Guest verification attempt

Same environment as DF-4341 (see env_guest.txt / run.log): no 0x110a PCI device, if_mn.c is optional mn netgraph (not in GENERIC), no if_mn.ko, kldload ENOENT — the mn0 node cannot exist, so trigger_alias.sh cannot run. Nothing was refuted; classification is inconclusive for the on-guest reproduction while the logic defect itself is certain from source.

Fix

fix.diff in this pack (ownership by sch->hook != NULL + clearing the pointer at disconnect). git apply --check clean against the read-only tree. Also closes the DF-1540 dangling-sch->hook read at the writer side. Not buildable/testable on this guest (no hardware).

Files

README.md, VERDICT.md, trigger_alias.sh, run.sh, run.log, env_guest.txt, fix.diff, verdict.json, manifest.json

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.