# DF-0884 — VERDICT

## Verdict: REPRODUCED (UAF via deterministic harness), then FIX VALIDATED

The bug is real and the mechanism is exactly as the finding describes. A
dead-code vnode lock upgrade in `smbfs_readvnode` lets two concurrent `read(2)`
on a directory vnode run `smbfs_readvdir()` under a shared lock, racing
`smbfs_findnext()` against `smbfs_findclose()` and using-after-free the
`smbfs_fctx`. The authored `fix.diff` restores `vn_islocked()` + the
conditional `vn_lock(LK_UPGRADE/DOWNGRADE)`; the patched `smbfs.ko` is
object-code-confirmed to emit them and the fixed-mode harness shows the race is
eliminated.

The live trigger needs a mounted SMB share, which is not present on this
isolated KVM guest (no network beyond QEMU user-mode NAT to the host, no SMB
server). Per the smbfs harness precedent (DF-0598 / DF-0599), the bug is proved
deterministically by transcribing the exact code path into a userspace harness
that models the SMB network round-trip as a controllable interleaving point and
uses a poisoned allocator to detect the write-UAF on the freed `smbfs_fctx`.

## Mechanism (every hop cited)

1. **read(2) takes the vnode SHARED** — `vfs_vnops.c:751`
   `vn_lock(vp, LK_SHARED | LK_RETRY)` inside `vn_read()`. The directory vnode
   is held shared for the whole read.

2. **The shared->exclusive upgrade is dead code** — `smbfs_io.c:201-204`
   ```c
   if (vp->v_type == VDIR) {
       lks = LK_EXCLUSIVE;/*lockstatus(&vp->v_lock, td);*/   /* :202 */
       if (lks == LK_SHARED)                                /* :203 ALWAYS FALSE */
           vn_lock(vp, LK_UPGRADE | LK_RETRY);              /* :204 DEAD CODE */
       error = smbfs_readvdir(vp, uiop, cred);              /* :205 runs SHARED */
   ```
   The `lockstatus()` call that would detect the real shared mode is commented
   out; `lks` is hardcoded to `LK_EXCLUSIVE`, so `if (lks == LK_SHARED)` is
   always false and the `vn_lock(LK_UPGRADE)` is never reached.
   Object-code confirmation: in the unpatched `smbfs.ko`, `smbfs_readvnode`'s
   VDIR branch contains **0** `vn_islocked` and **0** `vn_lock` calls — the
   optimizer elided the dead upgrade entirely (`disasm_evidence.txt`).

3. **smbfs_readvdir mutates per-vnode directory state** — `smbfs_io.c:78-174`.
   It reads and writes `np->n_dirseq` and `np->n_dirofs` (lines 118-141) and
   drives the SMB find context through `smbfs_findopen` / `smbfs_findnext` /
   `smbfs_findclose`. Because step 2 left the lock shared, two `read(2)`
   callers can execute this body **concurrently**.

4. **The race -> write-UAF**:
   - **Thread A** enters `smbfs_readvdir`, takes the reopen branch
     (`smbfs_io.c:118-135`), allocates `ctx_A` via `smbfs_findopen`
     (`smbfs_smb.c:1170-1193`, `kmalloc` at `:1177`), stores it in
     `np->n_dirseq` (`smbfs_io.c:132`), and calls `smbfs_findnext(ctx_A, ...)`
     (`smbfs_io.c:137` or `:151`).
   - `smbfs_findnext()` (`smbfs_smb.c:1196`) blocks for a full SMB network
     round-trip inside `smbfs_findnextLM1` / `smbfs_findnextLM2`
     (`smbfs_smb.c:854` / `:1049`; the blocking `smbfs_smb_search` at `:868`
     / `smb_t2_request`). Thread A is parked in network I/O still holding the
     `ctx_A` pointer.
   - **Thread B** enters `smbfs_readvdir` concurrently (the lock is still
     shared). B's `offset != np->n_dirofs` (A advanced `n_dirofs` at
     `smbfs_io.c:124/138/154`), so B takes the reopen branch and calls
     `smbfs_findclose(np->n_dirseq, &scred)` at `smbfs_io.c:121` — which is
     A's `ctx_A`.
   - `smbfs_findclose()` (`smbfs_smb.c:1224-1236`) frees `ctx_A->f_rname`
     (`:1233`) and then `kfree(ctx, M_SMBFSDATA)` at `:1234`.
   - **Thread A** resumes from the blocked `smbfs_findnext()` and writes
     `ctx->f_attr.fa_ino` at `smbfs_smb.c:1220`, then reads it back at
     `smbfs_io.c:157` (`vop_write_dirent(..., ctx->f_attr.fa_ino, ...)`).
     Both are accesses through the now-freed `ctx_A` -> **write/read UAF on a
     freed `smbfs_fctx`**.

5. **The getdents(2) path is NOT affected** — `smbfs_vnops.c:725`
   `smbfs_readdir()` unconditionally takes `vn_lock(vp, LK_EXCLUSIVE | LK_RETRY
   | LK_FAILRECLAIM)` before calling `smbfs_readvnode`, so two `getdents(2)`
   callers cannot race. Only the `read(2)` path (`smbfs_read` -> `smbfs_readvnode`)
   is vulnerable, because `vn_read` holds the lock shared and the upgrade is
   dead.

## Harness proof (deterministic)

`harness.c` transcribes steps 1-4 into userspace:
- The vnode lock is a `pthread_rwlock_t` (shared = read-lock, exclusive =
  write-lock).
- The SMB network round-trip inside `smbfs_findnext` is a controllable barrier:
  Thread A blocks on a `pthread_cond_t`; the orchestrator releases it only
  AFTER Thread B has run `smbfs_findclose()` on A's `ctx`, modelling the full
  round-trip race window.
- The allocator is poisoned: every `free()` overwrites the object with `0xDD`
  and marks it freed, so Thread A's post-I/O write through the dangling `ctx`
  is detected unambiguously.

Bug mode output (`run.log`):
```
[*] After Thread B ran: A's ctx=0x8005108c0 freed? YES (poison=0xDD)
[*] Releasing Thread A ... it will now write ctx->f_attr.fa_ino THROUGH FREED MEMORY
[+] A wrote ctx->f_attr.fa_ino (0xcafebabe) into an object that was ALREADY freed by Thread B's smbfs_findclose()
>>> UAF CONFIRMED: smbfs_findnext wrote through freed smbfs_fctx ...
```
Deterministic across 3 runs (`run.log`, `run.2.log`, `run.3.log`).

## Impact ceiling

The primitive is a **write/read UAF on a freed `struct smbfs_fctx`**
(`kmalloc(sizeof(struct smbfs_fctx), M_SMBFSDATA, M_WAITOK | M_ZERO)`). The
`M_SMBFSDATA` malloc type backs several smbfs objects of similar size
(smbfs_fctx, smbnode aux, names), so the freed slot is reusable by an attacker
who can drive concurrent allocations — the classic path to corrupting a victim
object's function pointer / refcount / credential pointer. On the default
GENERIC kernel (`options INVARIANTS`), `kern_slaballoc.c`'s
`chunk_mark_free` / `WEIRD_ADDR` (0xdeadc0de) poisoning and magic checks would
trap the cross-type reuse / double-free and **panic** before grooming lands,
so the realistic on-GENERIC impact is **panic / local DoS**; a clean `uid=0`
escalation would require the `noinv` kernel (INVARIANTS off) and is therefore a
non-default-kernel result. **No live escalation chain was developed** because
the live trigger requires a mounted SMB share, which this isolated KVM guest
cannot provide (no SMB server reachable) — the harness transcribes the race
deterministically per the smbfs precedent.

Realistic preconditions: an unprivileged local user with **read access to a
directory on a mounted SMB share** (the share mounted by an admin or made
mountable via `vfs.usermount=1` + a root-created creds/SMB image owned by the
attacker). Two threads issuing `read(2)` on the same directory fd reproduce
the race. The trigger is a normal syscall surface; no `kldload`, no setuid
helper, no non-default kernel required for the bug to fire.

## PoC changes (what I authored)

The finding shipped no trigger source, so I authored `harness.c` from the cited
path. Iteration notes:
- First cut deadlocked in fixed mode: I modelled `LK_UPGRADE` as
  `pthread_rwlock_wrlock`, but a thread already holding the read lock cannot
  take the write lock on the same pthread rwlock (POSIX). I verified DragonFly's
  `lockmgr_upgrade` (`kern_lock.c:576-660`) has an explicit anti-deadlock rule:
  if another upgrade is pending, the caller **releases its shared lock and
  acquires exclusive normally** (`:616-625`). I re-modelled the upgrade as
  that same safe sequence (drop shared, take exclusive; reverse on exit), which
  both avoids the pthread deadlock and faithfully represents the kernel's
  no-deadlock upgrade.
- The verdict captures write-vs-free timing (`g_a_write_saw_freed`) rather than
  the final freed state, because in fixed mode B legitimately frees A's `ctx`
  only after A has finished — so "is ctx freed at the end" is true in both
  modes. The UAF is specifically "A writes while ctx is already freed"; the
  timing snapshot distinguishes the two cases crisply.

## Fix (verified)

`fix.diff` (git-apply-able, `git apply --check` clean) replaces the dead
`lks = LK_EXCLUSIVE; if (lks == LK_SHARED)` with `lks = vn_islocked(vp);`
(`vn_islocked` at `vfs_vnops.c:1136` is the idiomatic wrapper for
`lockstatus(&vp->v_lock, curthread)`, which is exactly what the commented-out
code intended). This is correct for BOTH callers of `smbfs_readvnode`:
- `smbfs_read` (VOP_READ): lock is shared -> `vn_islocked` returns `LK_SHARED`
  -> upgrade fires -> readvdir runs exclusive -> downgrade. Race closed.
- `smbfs_readdir` (VOP_READDIR, `smbfs_vnops.c:725`): lock is already
  exclusive -> `vn_islocked` returns `LK_EXCLUSIVE` -> upgrade/downgrade are
  skipped (the `if (lks == LK_SHARED)` guards them) -> no lock-state
  disturbance for the readdir caller. This is why the fix must DETECT the
  state rather than unconditionally upgrade: an unconditional `LK_DOWNGRADE`
  on exit would incorrectly downgrade the exclusive lock `smbfs_readdir`
  acquired, breaking its `vn_unlock`.

The finding's `## Recommended fix` proposed an unconditional `vn_lock
LK_UPGRADE`; this **refines** it to `vn_islocked()` detection, which is
required for correctness with the second caller (`smbfs_readdir`).

### Validation (Phase 8)
- Applied `fix.diff` to `/usr/src/sys/vfs/smbfs/smbfs_io.c` (`patch -p1`,
  hunk #1 succeeded at line 199).
- Rebuilt `smbfs.ko` from `/usr/src/sys/vfs/smbfs` (`fix_build.log`):
  `-Werror` clean, `rc=0`.
- Object-code proof (`disasm_evidence.txt`): `smbfs_readvnode` VDIR branch
  went from `vn_islocked`=0 / `vn_lock`=0 (unpatched) to `vn_islocked`=1 /
  `vn_lock`=2 (patched).
- Installed the patched `smbfs.ko` to `/boot/kernel/smbfs.ko`, `kldload` rc=0,
  `kldunload` rc=0 (valid module, valid symbol resolution).
- Fixed-mode harness (`fix_run.log`): `>>> FIXED: no UAF`.

### Before / after
| | smbfs.ko | `vn_islocked` in VDIR branch | `vn_lock` in VDIR branch | harness |
|---|---|---|---|---|
| **baseline** | unpatched | 0 | 0 | `>>> UAF CONFIRMED` |
| **patched**  | rebuilt w/ fix | 1 | 2 (upgrade+downgrade) | `>>> FIXED: no UAF` |

(No live SMB share is mountable on this guest, so module-compile +
object-code comparison + logic-transcription harness is the validation level,
matching the DF-0598 / DF-0599 smbfs precedent.)