# DF-2443 — dm_dev_remove lifecycle: deadlock-forces-UAF in dm_dev_remove_ioctl

## Verdict
**REPRODUCED (panic / local DoS via kernel list corruption — use-after-free) + FIX VALIDATED.**

DF-2443 is the **same underlying lifecycle bug** as sibling DF-2447, framed
here from the **deadlock-design angle**: `disable_dev()`'s
`while (dmv->ref_cnt != 0) cv_wait(...)` makes it **impossible** to call
`dm_dev_remove(dmv)` while the caller holds a busy reference on `dmv` — doing
so would cv_wait forever for the caller's OWN reference to drain (deadlock).
This **forces** `dm_dev_remove_ioctl()` to `dm_dev_unbusy(dmv)` (drop its busy
ref) BEFORE calling `dm_dev_remove(dmv)`, and that drop-ref-then-remove window
is the use-after-free a concurrent remover wins. The authored `fix.diff`
(`dm_dev_destroy_by_key` atomic removal that never takes a caller-held long-lived
reference) is built, installed as `dm.ko`, and confirmed to close the bug:
the SAME PoC that **deterministically panicked** the unpatched dm module now
**completes cleanly** through all 2000 iterations on the patched module.

Escalation to `uid=0` is **blocked by a valid hard blocker** (the whole dm ioctl
surface is root/operator-only — `kldload dm` is root-only and
`/dev/mapper/control` is `0640 root:operator`), see below.

## Mechanism (trigger → primitive → effect)

### The deadlock-forces-UAF design (DF-2443's framing)

`disable_dev()` in `sys/dev/disk/dm/dm_dev.c:65-77`:

```c
 65: static void
 66: disable_dev(dm_dev_t *dmv)
 67: {
 68:     KKASSERT(lockstatus(&dm_dev_mutex, curthread) == LK_EXCLUSIVE);
 69:
 70:     TAILQ_REMOVE(&dm_dev_list, dmv, next_devlist);
 71:     dm_dev_counter--;
 72:
 73:     lockmgr(&dmv->dev_mtx, LK_EXCLUSIVE);
 74:     while (dmv->ref_cnt != 0)
 75:         cv_wait(&dmv->dev_cv, &dmv->dev_mtx);   /* waits for ref_cnt==0 */
 76:     lockmgr(&dmv->dev_mtx, LK_RELEASE);
 77: }
```

`dm_dev_remove()` (`dm_dev.c:304-316`) calls `disable_dev(dmv)`:

```c
304: int
305: dm_dev_remove(dm_dev_t *dmv)
306: {
307:     /* Remove device from list and wait for refcnt to drop to zero */
308:     lockmgr(&dm_dev_mutex, LK_EXCLUSIVE);
309:     disable_dev(dmv);
310:     lockmgr(&dm_dev_mutex, LK_RELEASE);
311:
312:     /* Destroy and free the device */
313:     dm_dev_destroy(dmv);
314:
315:     return 0;
316: }
```

Therefore a caller that **holds a busy reference** on `dmv` (`ref_cnt >= 1`,
acquired via `dm_dev_lookup` → `dm_dev_busy`) **cannot** call `dm_dev_remove`:
`disable_dev`'s `while(ref_cnt != 0) cv_wait` would wait forever for the
caller's OWN reference to drain → **deadlock**.

This **forces** `dm_dev_remove_ioctl()` (`dm_ioctl.c:330-362`) into the unsafe
pattern:

```c
330: int
331: dm_dev_remove_ioctl(prop_dictionary_t dm_dict)
332: {
333:     dm_dev_t *dmv;
334:     const char *name, *uuid;
335:     uint32_t flags, minor, is_open;
...
349:     if ((dmv = dm_dev_lookup(name, uuid, minor)) == NULL) {  /* ref_cnt 0->1 */
...
354:     is_open = dmv->is_open;
355:
356:     dm_dev_unbusy(dmv);          /* <-- MUST drop the busy ref here,
357:                                       otherwise dm_dev_remove below
358:                                       would deadlock in disable_dev.  */
359:     if (is_open)
360:         return EBUSY;
361:
362:     return dm_dev_remove(dmv);   /* <-- uses dmv AFTER the ref is dropped */
363: }
```

The window between line 356 (`dm_dev_unbusy`) and line 362 (`dm_dev_remove`) is
the bug: the caller dereferences `dmv` while holding **no** reference.

### The race (remove-vs-remove) → UAF → panic

Two threads issuing `remove` on the same device race through this window:

1. **A**: `dm_dev_lookup` → `ref_cnt` 0→1. Reads `is_open`.
2. **B**: `dm_dev_lookup` → `ref_cnt` 1→2. Reads `is_open`. (Both hold a ref.)
3. **A**: `dm_dev_unbusy` → `ref_cnt` 2→1.
4. **B**: `dm_dev_unbusy` → `ref_cnt` 1→0, `cv_broadcast` (no waiter yet).
5. **A**: `dm_dev_remove(dmv)` → `disable_dev` under `dm_dev_mutex`:
   `TAILQ_REMOVE` (dmv off the list), wait `ref_cnt==0` (already 0), release mutex.
6. **A**: `dm_dev_destroy(dmv)` → `kfree(dmv)`.  ← dmv is freed.
7. **B**: `dm_dev_remove(dmv)` → `disable_dev` on **freed** `dmv`:
   `TAILQ_REMOVE(&dm_dev_list, dmv, next_devlist)` reads `dmv->next_devlist`
   (slab-poisoned 0xdeadc0de under INVARIANTS / reused) → **"Bad link elm ...
   prev->next != elm"** panic / use-after-free / double-free.

The mutex does *not* save B: A releases the mutex at the end of its
`disable_dev` (step 5), B then acquires it and touches the already-freed `dmv`
while A is concurrently freeing it in step 6.

## Confirmed effect (unpatched `#0` kernel)

```
panic: Bad link elm 0xfffff8008de72700 prev->next != elm
cpuid = 5
Trace beginning at frame 0xfffff8011837b648
dm_dev_insert() at dm_dev_insert 0xffffffff82600f00
dm_dev_insert() at dm_dev_insert 0xffffffff82600f00
dm_dev_remove() at dm_dev_remove+0x25 0xffffffff82601305
dm_dev_remove_ioctl() at dm_dev_remove_ioctl+0xb7 0xffffffff82601bf7
dmioctl() at dmioctl+0x2eb 0xffffffff8260083b
dev_dioctl() at dev_dioctl+0x65 0xffffffff8062cdb5
Stopped at      Debugger+0x7c:  movb    $0,0xbdaf09(%rip)
db>
```

The backtrace names `dm_dev_remove_ioctl` → `dm_dev_remove` → list operation —
the exact drop-ref-then-deref window. The corrupted doubly-linked list
(`prev->next != elm`) is the direct consequence of two removers racing on the
same `dm_dev_t`: the second remover's `disable_dev`→`TAILQ_REMOVE` operates on
`dmv->next_devlist` links that the first remover already invalidated (removed
from the list, and/or freed+reused memory). Reproduced deterministically within
~500-1000 iterations (~4000-8000 concurrent races). Guest wedged in DDB.

## Primitive characterization

- **Class:** use-after-free / double-remove on a `kmalloc(sizeof(dm_dev_t),
  M_DM)` object (`dm_dev_alloc`, `dm_dev.c:355`). `dm_dev_t` is ~1.5 KB (name
  + uuid + devt + locks + cv + table_head + disk + devstat) → it lives in the
  `kmalloc-2048` slab bucket (M_DM objcache).
- **What the race yields:** the second remover re-enters `disable_dev` on freed
  memory → reads/writes `dmv->next_devlist` (list links), `dmv->dev_mtx` (a
  `struct lock`), `dmv->dev_cv`, then `dm_dev_destroy` re-runs
  `dm_table_destroy` / `disk_destroy` / `kfree(dmv)` on freed memory → double
  free + lock-object reuse. With slab grooming (fill the `M_DM` bucket with
  attacker-shaped objects after the free, before the second remover's derefs)
  this is a controlled corruption primitive.
- **Observed outcome on this guest (INVARIANTS ON):** deterministic panic via the
  TAILQ sanity check before silent corruption lands — i.e. a reliable local DoS,
  and a *hint* of the underlying write primitive. On a noinv kernel the same
  race would silently corrupt the slab.

## Why no `uid=0` chain (valid hard blocker — privilege gate)

Per the Phase-6 hard-blocker rules, escalation is blocked because the vulnerable
path is **reachable only from an already-root/operator context** — there is no
unprivileged boundary for this bug to cross:

1. **Module load:** `dm` is a KLD module (`DECLARE_MODULE(dm, …)`,
   `device-mapper.c`); it is **not** in the GENERIC kernel and **not**
   auto-loaded. Reaching any dm ioctl requires `kldload dm`, a root-only op.
2. **Device node permission:** the control device is created as
   `make_dev(&dmctl_ops, 0, UID_ROOT, GID_OPERATOR, 0640, "mapper/control")`
   (`device-mapper.c:181`) — `crw-r----- root operator`.
3. **Unprivileged user cannot reach it — verified on the guest:**
   `maxx` (uid 1001, gid 1001, **not** in `operator` or `wheel`) gets
   `open /dev/mapper/control: Permission denied`. There is no devfs rule
   relaxing this, and `dmsetup`/`lvm` are not setuid.

Root→kernel is game-over by definition (root can already set `uid=0`). So this
is a **root/operator → kernel memory-corruption / local-DoS / hardening gap**,
not an unprivileged→root escalation. The realistic impact ceiling: a root
operator (or any `operator`-group member) can deterministically panic the kernel
(DoS) and — with slab grooming on a noinv kernel — potentially corrupt the
kernel heap toward code execution. Worth fixing as defense-in-depth.

## PoC (`dm_deadlock_uaf.c`)

A libprop `NETBSD_DM_IOCTL` racer (modeled on sibling DF-2447's PoC). It
repeatedly creates a dm device `df2443racer` and, via a pipe barrier, fires **N
concurrent `remove` ioctls** at it simultaneously so the lookups stack
(`ref_cnt` 0→1→2…) before any `dm_dev_unbusy` runs — the exact precondition for
the UAF. The barrier is what makes two removers reliably land in the window
together.

- Build: `cc -O2 -o dm_deadlock_uaf dm_deadlock_uaf.c -lprop`
- Run (as root): `kldload dm && ./dm_deadlock_uaf 8 2000`

Args: `<racers> <iterations>` (defaults 8 / 2000).

## Fix (`fix.diff`)

Minimal and targeted at the root cause — **eliminate the deadlock-forces-UAF
design** by doing the lookup + is_open check + removal + disable_dev drain +
destroy **all atomically under `dm_dev_mutex`, without ever taking a caller-held
long-lived busy reference**:

1. **`dm_dev_destroy_by_key(name, uuid, minor)`** (new, in `dm_dev.c`). It does
   the lookup, the `is_open` check, the removal from `dm_dev_list` and the
   `disable_dev` wait-for-refcnt-drain **all under `dm_dev_mutex`**, then
   destroys. Because the lookup-and-claim is atomic, two concurrent removers are
   **serialized**: the first removes+frees the device, the second finds nothing
   (`ENOENT`). There is no window in which `dmv` is dereferenced without a
   reference, and no double-remove. The helper **never takes a long-lived busy
   reference of its own**, so `disable_dev`'s `while(ref_cnt!=0) cv_wait` can
   complete (no deadlock). This closes BOTH the deadlock (the helper doesn't
   hold a ref) AND the UAF (no drop-ref-then-deref window).
2. **`dm_dev_remove_ioctl`** — replace the lookup → read `is_open` → unbusy →
   `dm_dev_remove(dmv)` sequence with a single call to
   `dm_dev_destroy_by_key(name, uuid, minor)`.
3. Add a NOTE to `dm_dev_remove` documenting that the caller must not hold a busy
   reference (it never did in practice, but the contract is now explicit), and
   declare `dm_dev_destroy_by_key` in `dm.h`.

This is the **same fix** validated for sibling DF-2447 (the root cause is
identical); DF-2443 frames the deadlock-design angle, DF-2447 the
remove/resume-deref angle, and the atomic-removal helper closes both.
`git apply --check` passes against the read-only `sys/` tree.

## Fix validation (Phase 8)

| step                              | result                                                                  |
|-----------------------------------|-------------------------------------------------------------------------|
| baseline `#0` kernel, unpatched dm| **panic** `Bad link elm … prev->next != elm` in `dm_dev_remove_ioctl` → `dm_dev_remove` within ~500-1000 iterations; guest DDB |
| apply `fix.diff` to `/usr/src`    | 5 hunks applied cleanly (dm_dev.c ×2, dm.h ×1, dm_ioctl.c ×2)           |
| rebuild dm module                 | `make` in `sys/dev/disk/dm` → `dm.ko` OK (`rc=0`), no errors            |
| install dm.ko                     | `/boot/kernel/dm.ko` replaced (sha256 `095e0798…`); kernel image stays `#0` (dm is purely loadable) |
| confirm fix in built module       | `nm dm.ko` shows new `T dm_dev_destroy_by_key` symbol                   |
| re-run PoC (8 racers × 2000 iters)| **clean completion, PATCHED_RACER_EXIT=0**, "exhausted 2000 iterations without a panic", guest UP, boot.log empty |

The same PoC that **deterministically panicked** the unpatched dm module now
**completes cleanly** through all 2000 iterations (~16000 concurrent races) on
the patched `dm.ko`. `fix_status = fixed`.

(dm is a purely loadable KLD module, so the fix lives entirely in `dm.ko` — the
kernel image is left at the `#0` baseline and only `dm.ko` is swapped, exactly
as was done for siblings DF-2446/DF-2447.)

## Files

| file                | purpose                                                              |
|---------------------|----------------------------------------------------------------------|
| `dm_deadlock_uaf.c` | trigger PoC (concurrent remove ioctls via libprop + pipe barrier)    |
| `build.sh`          | `cc -O2 -o dm_deadlock_uaf dm_deadlock_uaf.c -lprop`                 |
| `run.sh`            | `./dm_deadlock_uaf <racers> <iters>` (as root, after `kldload dm`)   |
| `build.log`         | PoC build output (as maxx)                                           |
| `run.log`           | baseline (unpatched) run + panic signature                           |
| `panic.txt`         | `Bad link elm … prev->next != elm` panic from `boot.log`             |
| `fix.diff`          | git-apply-able fix (atomic `dm_dev_destroy_by_key`)                  |
| `fix_build.log`     | patched dm.ko build log (`rc=0` clean)                               |
| `fix_run.log`       | patched dm.ko run → clean completion, guest up, no panic             |
| `env.txt`           | guest uname / kern.version / cc / dm.ko hash / control dev / maxx id |
| `VERDICT.md`        | this file                                                            |
| `manifest.json`     | machine-readable artifact catalog                                    |