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

```c
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):

```sh
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:

```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 */
```