# DF-0928 — VERDICT

## Verdict: REPRODUCED AT SOURCE LEVEL — missing lock confirmed; race not triggered from local userspace

The bug described in DF-0928 is **real and confirmed at the source level**: the
UFS/FFS inode hash (`sys/vfs/ufs/ufs_ihash.c`) has **zero synchronization**
protecting the hash table or the `inode.i_next` chain. Every function —
`ufs_ihashinit`, `ufs_ihashuninit`, `ufs_ihashget`, `ufs_ihashlookup`,
`ufs_ihashcheck`, `ufs_ihashins`, `ufs_ihashrem` — walks or modifies the chain
with no mutex, no `lwkt_token`, no held reference. The otherwise-identical
sibling `sys/vfs/ext2fs/ext2_ihash.c` wraps every one of these operations in a
`static struct lwkt_token ext2_ihash_token` (`ext2_ihash.c:53`).

However, despite extensive testing (multiple PoC variants, 5+ minutes of
aggressive racing, a delay-injected kernel that widened the TOCTOU window to
5ms inside `ufs_ihashins`), **no panic was triggered from local userspace**.
The root cause is that the DragonFlyBSD namecache provides **incidental
serialization** that prevents concurrent `ufs_iget`/`ufs_ihashins` calls for the
same inode from local processes.

## Mechanism (confirmed by source analysis)

### The missing lock (FACT)

`ufs_ihash.c` (entire 190-line file read) contains **not a single**
synchronization primitive. Compare:

| Function | `ufs_ihash.c` | `ext2_ihash.c` (sibling) |
|---|---|---|
| `*_init` | no lock | `lwkt_token_init` (`:67`) |
| `*_uninit` | no lock | `lwkt_gettoken`/`reltoken` (`:73,76`) |
| `*_get` | no lock | `lwkt_gettoken`/`reltoken` (`:94,114,117`) |
| `*_ins` | no lock | `lwkt_gettoken`/`reltoken` (`:131,135,143`) |
| `*_rem` | no lock | `lwkt_gettoken`/`reltoken` (`:156,169`) |

### The TOCTOU in ufs_ihashins (ufs_ihash.c:154-164)

```c
// SCAN: check for existing entry (no lock held)
while ((iq = *ipp) != NULL) {
    if (ip->i_dev == iq->i_dev && ip->i_number == iq->i_number)
        return(EBUSY);
    ipp = &iq->i_next;
}
// *** RACE WINDOW: another thread can scan+store here ***
ip->i_next = NULL;
*ipp = ip;              // STORE: overwrites any concurrent store
ip->i_flag |= IN_HASHED;
```

Two concurrent `ffs_vget()` calls for the same `(dev,ino)` can both scan the
empty bucket, both see no duplicate, and both store — the second store silently
overwrites the first, orphaning the loser's inode (`IN_HASHED` set but not on
the chain). On INVARIANTS kernels, the orphan trips `KKASSERT(ip == iq)` at
`ufs_ihash.c:184` during `ufs_reclaim` → `ufs_ihashrem`.

### The walker-UAF (ufs_ihash.c:105, 138, 179)

All walker functions dereference `ip->i_number`, `ip->i_dev`, `ip->i_next`
from the chain with no lock and no reference on the walked inode. A concurrent
`ufs_reclaim` (`ufs_inode.c:145` → `ufs_ihashrem`) → `kfree(ip)`
(`ufs_inode.c:162`) leaves a use-after-free-read window.

## Why the race was NOT triggered from local userspace

Instrumented-kernel testing (kprintf + 5ms DELAY inside the TOCTOU window of
`ufs_ihashins`) confirmed:

1. **`ufs_ihashins` IS called** — 1139 calls observed during an 8-second run,
   from multiple CPUs (cpu=0, cpu=1, cpu=5).
2. **`ihashins collision` (EBUSY) count = 0** — no two threads ever called
   `ufs_ihashins` for the same inode concurrently.
3. **Root cause**: the DragonFlyBSD namecache provides incidental serialization.
   When any thread resolves ANY path to an inode and calls `ufs_ihashins`, the
   vnode is created and inserted into the hash. All subsequent lookups via ANY
   path (including different hardlink paths in separate directories) call
   `ufs_ihashget` → hit → return the existing vnode. No second `ufs_ihashins`
   call occurs for the same inode.

The dual-insert race window requires the inode's vnode to have been fully
reclaimed (removed from hash by `ufs_ihashrem`) AND two lookups to both miss
`ufs_ihashget` before either's `ufs_ihashins` stores. The namecache resolves
the first lookup within microseconds, closing the window before a second
lookup can enter.

### The realistic trigger: NFS READDIRPLUS

The finding's NFS variant (`nfs_serv.c` builds READDIRPLUS replies by calling
`VFS_VGET` once per directory entry, `nfs_serv.c:3417,3454`) bypasses the
namecache and drives concurrent `ffs_vget` for overlapping entries directly.
Two NFS clients issuing concurrent READDIRPLUS for the same directory would
race. This was not tested (no NFS server setup on this guest).

## Exploit chain assessment

This is a **race condition** (CWE-362), not a deterministic memory-corruption
primitive. There is no reliable write/control primitive to escalate. On an
INVARIANTS kernel (default GENERIC), the impact is DoS (panic). On a production
(non-INVARIANTS) kernel, the impact would be cache incoherence (two vnodes for
one inode → stale permissions). No `uid=0` escalation path was identified.

## Fix

Authored `fix.diff`: adds a per-mount `struct lwkt_token um_ihash_token` to
`struct ufsmount` (`ufsmount.h`) and acquires/releases it in all 7 functions
(`ufs_ihashinit`, `ufs_ihashuninit`, `ufs_ihashget`, `ufs_ihashlookup`,
`ufs_ihashcheck`, `ufs_ihashins`, `ufs_ihashrem`). This exactly mirrors the
proven `ext2_ihash.c` pattern.

### Fix validation

- **Applies**: `git apply --check` passes cleanly (133-line diff, 2 files).
- **Compiles**: `make -j6 nativekernel` rc=0 (full build log in `fix_build.log`).
- **Boots**: fixed kernel `#1` boots and runs normally.
- **PoC on fixed kernel**: runs 50s without panic (same as unpatched — the
  race can't be triggered from local userspace on either kernel due to
  namecache serialization).
- **Behavioral before/after**: `not_testable` — the PoC cannot trigger the
  race on either kernel. The fix is validated at the code level: all 7 hash
  operations are now serialized by the token, matching the proven ext2fs
  sibling.

## PoC files

- `race_ufs_ihash.c` — original PoC (reviewer-written, operates on `/tmp` which
  is tmpfs on this guest — corrected to use a UFS mount).
- `race_ufs_ihash2.c` — improved PoC using hardlinks in separate directories.
- `race_ufs_ihash3.c` — maximum-aggression PoC: 16 racers + 8 churners + 500
  pre-created files on a tiny-hash UFS mount.

## PoC changes

1. Changed target from `/tmp` (tmpfs on this guest — does NOT exercise UFS
   inode hash) to a UFS mount (`/mnt/ufs_test`, created via `vnconfig` + `newfs`).
2. Added hardlink-based racing across separate directories to bypass the
   parent-directory vnode lock that serializes same-directory lookups.
3. Added churner threads (create+delete files) to maintain vnode recycling
   pressure.
4. Created two additional PoC variants (`race_ufs_ihash2.c`, `race_ufs_ihash3.c`)
   with progressively more aggressive racing strategies.