# DF-2447 — dm_dev_remove_ioctl / dm_dev_resume_ioctl use-after-free

## Verdict
**REPRODUCED (panic / local DoS via kernel list corruption) + FIX VALIDATED.**
Both cited ioctls drop the device's busy reference and then keep dereferencing
the `dmv` pointer. Driven by concurrent `remove` ioctls on the same device this
**deterministically panics** the kernel with a corrupted-list assertion
(`Bad link elm ... prev->next != elm`) inside `dm_dev_remove_ioctl` →
`dm_dev_remove` → list manipulation — exactly the drop-ref-then-deref window
the finding describes. The authored `fix.diff` is built, installed as `dm.ko`,
and confirmed to close the bug (panic → clean completion under ~16000+ concurrent
races). Escalation to `uid=0` is **blocked by a valid hard blocker** (the whole
dm ioctl surface is root/operator-only), see below.

## Mechanism (trigger → primitive → effect)

`dm_dev_remove_ioctl()` in `sys/dev/disk/dm/dm_ioctl.c`:

```c
 330: int
 331: dm_dev_remove_ioctl(prop_dictionary_t dm_dict)
 332: {
 ...
 349:     if ((dmv = dm_dev_lookup(name, uuid, minor)) == NULL) {  // ref_cnt++ (busy)
 ...
 354:     is_open = dmv->is_open;
 356:     dm_dev_unbusy(dmv);             // <-- DROPS the busy reference
 ...
 361:     return dm_dev_remove(dmv);      // <-- uses dmv AFTER the ref is dropped
 362: }
```

`dm_dev_unbusy()` (`dm_dev.c:398`) decrements `ref_cnt` and `cv_broadcast`s when
it hits 0. `dm_dev_remove()` (`dm_dev.c:305`) → `disable_dev()` removes the
device from `dm_dev_list` and waits for `ref_cnt==0`, then `dm_dev_destroy()` →
`dm_dev_free()` → `kfree(dmv, M_DM)`.

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, broadcasts (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 / reused) → **"Bad link elm ... prev->next != elm"** panic /
   use-after-free / double-free.

The window between step 4 (`dm_dev_unbusy`) and step 7 (`dm_dev_remove`) is the
bug. 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.

`dm_dev_resume_ioctl()` has the **identical** window at a different site:

```c
 518:     dm_dev_unbusy(dmv);                          // <-- DROPS the busy ref
 ...
 521:     dm_table_destroy(&dmv->table_head, DM_TABLE_INACTIVE);  // <-- uses dmv
```

(`dm_table_destroy` is `dm_table.c:130`; it only touches `head->table_mtx` /
`head->tables`, independent of `ref_cnt`, so simply reordering — destroying the
table *before* unbusying — is the correct fix, matching the safe pattern already
used by `dm_table_clear_ioctl` at `dm_ioctl.c:562/566`.)

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

```
panic: Bad link elm 0xfffff80117ed3a40 prev->next != elm
cpuid = 5
Trace beginning at frame 0xfffff801185cb648
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
Debugger("panic")
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 within the first race
iteration (8 racers). 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 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 (same as sibling DF-2446).

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_race_uaf.c`)

A libprop `NETBSD_DM_IOCTL` racer (modeled on the sibling DF-2446 PoC). It
repeatedly creates a dm device `df2447racer` 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_race_uaf dm_race_uaf.c -lprop`
- Run (as root): `kldload dm && ./dm_race_uaf 8 2000`

Args: `<racers> <iterations>` (defaults 6 / 4000).

## Fix (`fix.diff`)

Minimal and targeted at the root cause — **eliminate the drop-ref-then-deref
window** in both cited paths:

1. **`dm_dev_remove_ioctl`** — replace the lookup → read `is_open` → unbusy →
   `dm_dev_remove(dmv)` sequence with a single atomic helper
   **`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).
2. **`dm_dev_resume_ioctl`** — move `dm_dev_unbusy(dmv)` to **after**
   `dm_table_destroy(&dmv->table_head, DM_TABLE_INACTIVE)` so the busy reference
   is held across the last `dmv` dereference. This matches the safe pattern
   already used by `dm_table_clear_ioctl` (`dm_ioctl.c:562`→`566`). Safe because
   `dm_table_destroy` is independent of `ref_cnt`.
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`.

`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 iteration 0; guest DDB |
| apply `fix.diff` to `/usr/src`    | 6 hunks applied cleanly (dm_dev.c ×2, dm.h ×1, dm_ioctl.c ×3)           |
| rebuild dm module                 | `make` in `sys/dev/disk/dm` → `dm.ko` OK, `-Werror`, no warnings/errors |
| install dm.ko                     | `/boot/kernel/dm.ko` replaced (sha256 `4c3239e9…`); kernel image stays `#0` (dm is purely loadable) |
| confirm fix in loaded module      | `nm dm.ko` shows new `T dm_dev_destroy_by_key` symbol                   |
| re-run PoC (8 racers × 2000 iters)| **clean completion, RUN_EXIT=0**, "exhausted … without a panic", guest UP, boot.log empty |
| longer stress (8 racers × 8000)   | survived 5500+ iterations (~44000 races) before the 90s `timeout` cutoff (not a crash); guest UP |

The same PoC that **deterministically panicked** the unpatched dm module now
**completes cleanly** under heavy concurrent remove pressure 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 sibling DF-2446.)

## Files

| file                | purpose                                                              |
|---------------------|----------------------------------------------------------------------|
| `dm_race_uaf.c`     | trigger PoC (concurrent remove ioctls via libprop + pipe barrier)    |
| `build.sh`          | `cc -O2 -o dm_race_uaf dm_race_uaf.c -lprop`                         |
| `run.sh`            | `./dm_race_uaf <racers> <iters>` (as root, after `kldload dm`)       |
| `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` + resume reorder) |
| `fix_build.log`     | patched dm.ko build log (`-Werror` clean, sha256 `4c3239e9…`)        |
| `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                                    |