# DF-0716 — smb_strdupin ignores copyin return value — TOCTOU race

## Verdict
**REPRODUCED** — root→kernel TOCTOU info leak. The code defect is real and
confirmed by source trace (`smb_subr.c:129` ignores `copyin` return) and by a
demonstrated race win (stale slab bytes `b0 3b 02 00 00 00 00 00` at positions
120-127 of the returned buffer). The fix (`check copyin return → kfree +
return NULL; add M_ZERO`) is validated: disassembly of the fixed `smbfs.ko`
confirms all three changes compiled in, and the race produces zero stale-byte
results on the patched module.

## Severity / impact
**Low** — root→kernel heap info leak via TOCTOU race. The only path to
`smb_strdupin` is via `/dev/nsmb` (the nsmb clone device), created `0700
root:wheel` (`smb_dev.c:355-356`). An unprivileged user cannot open the device
(`vfs.usermount=0`, no devfs rules loosening nsmb). Additionally, `smb_subr.c`
is `optional netsmb` (`sys/conf/files:1876`) — NOT compiled into the default
`X86_64_GENERIC` kernel; it only exists in the `smbfs.ko` loadable module,
which requires root `kldload`. No memory-corruption primitive is derived —
the leak is stale slab contents (previous `M_SMBSTR` allocations) sent to an
SMB server that root voluntarily connected to. This is a hardening gap, not
an escalation vector.

## Mechanism (trigger → primitive → effect)

### The code defect
`smb_strdupin` (`smb_subr.c:113-131`):
```c
char *
smb_strdupin(char *s, int maxlen)
{
    char *p, bt;
    int len = 0;

    for (p = s; ;p++) {           // length loop: reads byte-by-byte
        if (copyin(p, &bt, 1))    //   checks copyin return ← OK
            return NULL;
        len++;
        if (maxlen && len > maxlen)
            return NULL;
        if (bt == 0)
            break;
    }
    p = kmalloc(len, M_SMBSTR, M_WAITOK);  // NOT zeroed (no M_ZERO)
    copyin(s, p, len);                      // return IGNORED ← THE BUG
    return p;
}
```

Line 129: `copyin(s, p, len)` — the return value is discarded. If `copyin`
fails (EFAULT), `p` is partially filled with stale slab contents and returned
as non-NULL. The caller (`smb_usr.c:308-312`) checks for NULL but the buffer
is non-NULL, so it proceeds to use it as `t2p->t_name`.

### TOCTOU race
1. Thread A: issues `SMBIOC_T2RQ` with `ioc_name` → `smb_strdupin` starts.
2. Length loop reads the user string byte-by-byte (checks copyin return).
   Computes `len`.
3. `kmalloc(len, M_SMBSTR, M_WAITOK)` — may sleep (widens race window).
4. Thread B: `mprotect(page_b, PROT_NONE)` — makes a page within the string
   unreadable.
5. Bulk `copyin(s, p, len)` — faults at the `PROT_NONE` page. Returns EFAULT.
6. **Return value IGNORED** → `p` returned with stale bytes where the faulted
   page's data should be.
7. `p` → `t2p->t_name` → `smb_t2_request(t2p)` → sent to SMB server via TRANS2.

### Demonstrated race win
Using a test harness module (`strdup_test.ko`) that calls `smb_strdupin`
directly (bypassing the SMB protocol, which requires a live server not
available on the guest), with a 128-byte string spanning a page boundary
(127 bytes on page A + NUL on page B):

```
[RACE WON iter 1] result bytes (hex, first 128):
  4141414141414141414141414141414141414141414141414141414141414141
  4141414141414141414141414141414141414141414141414141414141414141
  4141414141414141414141414141414141414141414141414141414141414141
  414141414141414141414141414141414141414141414141b03b020000000000
```

- Bytes 0-119: `0x41` ('A') — correctly copied by bulk `copyin`.
- Bytes 120-127: `b0 3b 02 00 00 00 00 00` — **stale slab contents**.

The bulk `copyin` reads in 8-byte chunks. The last chunk (bytes 120-127)
straddled the page boundary (bytes 120-126 on page A, byte 127 on page B).
When the racing thread set page B to `PROT_NONE`, the 8-byte read faulted,
leaving bytes 120-127 as stale heap data from a previous `M_SMBSTR` allocation.

The stale bytes (`0x0000000000023bb0` in LE) are **real kernel heap data**
(`debug.use_weird_array=0`, so no `0xdeadc0de` slab poisoning by default) —
potentially containing pointers, credential fragments, or other sensitive data
from previous `M_SMBSTR` allocations (SMB strings, passwords, addresses).

The race is extremely narrow (~1 win in 200K+ iterations across 2.6M total
iterations). The `kmalloc(M_WAITOK)` between the length loop and the bulk
`copyin` is the only widening factor, and it does not reliably sleep on this
guest. The code defect is confirmed regardless by source trace.

## Exploit chain
**Not applicable** — this is a pure info-leak / hardening gap, not a
memory-corruption primitive. The stale bytes are **read** from the slab and
sent to the SMB server; there is no write, no UAF, no type confusion. No
heap grooming, no victim object, no escalation chain is possible. The finding
is correctly classified as Low severity (root→kernel heap info leak).

Additionally, the path is **root-only** (device 0700 + `kldload`), so there is
no privilege boundary to cross. This is a valid hard blocker for escalation
per Phase 6: "reachable only from an already-root context."

## PoC changes
Authored from scratch (no prior PoC existed in `findings/poc/DF-0716/`):
- `strdup_test.c` — kernel module test harness that creates `/dev/strdup_test`
  (root-only 0600) and calls `smb_strdupin` directly on ioctl. Bypasses the
  SMB protocol (which requires a live server not available on the guest).
- `strdup_race.c` — userspace driver that mmaps a 2-page buffer, places a
  128-byte string spanning the page boundary, and races `smb_strdupin` by
  toggling page B protection (`PROT_NONE` ↔ `PROT_READ|PROT_WRITE`) via a
  pthread. Examines each result for stale bytes.
- `Makefile` — builds `strdup_test.ko` via `bsd.kmod.mk`.
- `build.sh` / `run.sh` — exact build/run commands.
- `fix.diff` — git-apply-able unified diff: check `copyin` return + add
  `M_ZERO` in `smb_strdupin` (`smb_subr.c:128-130`).

## Fix validation (Phase 8)

### Baseline (unpatched #0 kernel + unpatched smbfs.ko)
- Race won once in 200K iterations: stale bytes `b0 3b 02 00 00 00 00 00`
  at positions 120-127.
- Code defect confirmed by source trace: line 129 ignores `copyin` return.
- Subsequent runs (2.4M+ iterations) did not win again — the race is
  extremely narrow (~1 in 200K+).

### Patched (fixed smbfs.ko module)
- Applied `fix.diff` to `/usr/src/sys/netproto/smb/smb_subr.c`.
- Rebuilt `smbfs.ko` module (`make` in `sys/vfs/smbfs/`).
- Installed to `/boot/kernel/smbfs.ko`, loaded.
- **Disassembly confirms all three fix changes** compiled in:
  - `mov $0x102,%edx` — `M_WAITOK(0x2) | M_ZERO(0x100)` = 0x102.
  - `test %eax,%eax` + `je` — `copyin` return value checked.
  - `callq kfree` + `xor %ebx,%ebx` — on failure, `kfree(p)` and return NULL.
- Race test: 2.5M iterations across 3 runs → `RACE WON: 0` (consistent with
  the fix: `copyin` failure now returns NULL, not stale bytes).
- No regression: normal case (`copyin` succeeds) returns correct string.

### fix_status: fixed
The code path is closed by disassembly verification + the race behavior is
consistent (RACE WON = 0 on fixed, RACE WON = 1 on unfixed in the earlier
run). The race is too narrow for a statistical before/after comparison on
every run, but the disassembly is definitive.

## Recommended fix
`fix.diff` changes `smb_strdupin` (`sys/netproto/smb/smb_subr.c:128-130`) from:
```c
p = kmalloc(len, M_SMBSTR, M_WAITOK);
copyin(s, p, len);
return p;
```
to:
```c
p = kmalloc(len, M_SMBSTR, M_WAITOK | M_ZERO);
if (copyin(s, p, len) != 0) {
    kfree(p, M_SMBSTR);
    return NULL;
}
return p;
```
This follows the same pattern already used by `smb_memdupin` (`smb_subr.c:144-147`),
which correctly checks the `copyin` return and frees on failure. **Supersedes
the finding proposal** (the finding summary mentions "check copyin return
kfree+return NULL on failure add M_ZERO" — the implemented fix matches this
exactly).