# DF-0662 — UAF of `readq`: `ng_device_disconnect` frees `readq` before `destroy_dev` drains in-flight reads

## Verdict: BUG CONFIRMED REAL by source trace — NOT reachable on the default kernel (ng_device is unshipped, unbuildable dead code)

## Bug summary (source-confirmed)

`ng_device_disconnect()` (`sys/netgraph/ng_device.c:386-414`) frees the per-connection
read queue **before** destroying the cdev, and never frees the connection struct:

```c
407      kfree(connection->readq, M_DEVBUF);   // (1) frees readq FIRST
409      destroy_dev(connection->ngddev);      // (2) drains in-flight cdev ops AFTER
411      SLIST_REMOVE(&sc->head, connection, ngd_connection, links);
                                         // (3) connection struct never kfree'd (leak)
```

`destroy_dev()` is precisely the call that waits for in-flight cdev operations
(read/write/ioctl/poll on `/dev/ngdN`) to drain. So freeing `readq` **before**
`destroy_dev` creates a use-after-free window:

- A thread in `ngdread()` (`:505-547`) that already passed the `SLIST_FOREACH`
  (`:518`) holds a raw `connection` pointer (no refcount/lock taken on it) and
  proceeds to `memcpy(buffer, connection->readq, amnt)` (`:531`) and
  `memcpy(connection->readq, ...)` (`:532`).
- If `ng_device_disconnect` runs concurrently (e.g. `ngctl rmhook`) and reaches
  line 407 while that `ngdread` thread is between `:518` and `:531`, the read
  dereferences/`memcpy`s **freed** `readq` (10 KiB, `M_DEVBUF`) → UAF.

Additional defect: the `ngd_connection` struct itself (`kmalloc`'d at `:273`) is
**never freed** — `sizeof(struct ngd_connection)` leaked per disconnect.

The UAF target is a 10 KiB `M_DEVBUF` allocation (`NGD_QUEUE_SIZE = 1024*10`,
`:106`), so it is groomable (spray the bucket, reclaim with a controlled object)
for arbitrary kernel r/w once triggered.

Trigger: concurrent `ngdread` on `/dev/ngdN` vs `ngctl rmhook`/hook teardown.
Race window, so reliability needs grooming, but the ordering bug is unconditional.

## Reachability — DEAD CODE on the default kernel

Same as DF-0660/DF-0661: `ng_device` is **not in any netgraph/netgraph7
`SUBDIR`** and its source uses the removed `struct cdevsw` API, so it is neither
shipped nor compilable on current DragonFly. This is latent/dead code; the UAF is
real but unreachable on a default install.

Also note the device itself is created mode `0600` (`make_dev(&ngd_cdevsw, ...,
0600, "ngd%d", ...)` at `:286-287`), so even if shipped it would be **root-only**
(no unprivileged path).

## Reproduction

Not reproduced at the kernel level: the code is unshipped/unbuildable AND the
trigger is a timing race (concurrent read vs disconnect). The ordering defect is
confirmed by direct source trace. See `fix.diff`.

## Fix (`fix.diff`)

Reorder so `destroy_dev()` drains in-flight cdev ops **before** `readq` is freed,
and free the leaked connection struct:

```c
-     kfree(connection->readq, M_DEVBUF);
-
      destroy_dev(connection->ngddev);
+
+     kfree(connection->readq, M_DEVBUF);

      SLIST_REMOVE(&sc->head, connection, ngd_connection, links);
+     kfree(connection, M_DEVBUF);
```

## Kernel references

- `sys/netgraph/ng_device.c:407` — `kfree(readq)` BEFORE `destroy_dev`
- `sys/netgraph/ng_device.c:409` — `destroy_dev` (should drain first)
- `sys/netgraph/ng_device.c:273` — `connection` `kmalloc` (never freed)
- `sys/netgraph/ng_device.c:518-532` — `ngdread` uses `connection->readq` with no refcount
- `sys/netgraph/ng_device.c:106` — `NGD_QUEUE_SIZE` (10 KiB UAF target)
- `sys/netgraph/ng_device.c:286-287` — `make_dev(..., 0600, ...)` (root-only)
- `sys/netgraph/Makefile` / `sys/sys/conf.h` — ng_device unshipped; cdevsw removed