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

Summary

smb_strdupin (sys/netproto/smb/smb_subr.c:113-131) calls copyin(s, p, len) at line 129 but ignores its return value. A TOCTOU race (racing thread toggling a page between the length loop and the bulk copyin) causes the bulk copyin to fault partway, leaving the kmalloc'd buffer (NOT zeroed) partially filled with stale kernel heap data from a previous M_SMBSTR allocation. The non-NULL buffer is returned to the caller (smb_usr.c:308) and used as t2p->t_name — transmitted to the attacker-controlled SMB server via TRANS2.

Build

# Build the kernel module test harness + userspace race driver
make                          # strdup_test.ko (kernel module)
cc -o strdup_race strdup_race.c -lpthread

Run

# Must be root: load smbfs.ko (provides smb_strdupin) + test harness, then race
kldload smbfs
kldload ./strdup_test.ko
./strdup_race [iterations]    # default 100000

Expected (bug present, unpatched)

Most runs: RACE WON: 0 (the race is extremely narrow — ~1 in 200K+). Occasionally: RACE WON: N with stale bytes (e.g. b0 3b 02 00 00 00 00 00) at the page-boundary straddling positions (bytes 120-127), proving the copyin return is ignored and stale slab contents are returned.

Expected (fixed)

RACE WON: 0 always — the fixed smb_strdupin checks the copyin return and returns NULL on failure (caller handles NULL → ENOMEM). No stale bytes possible.

Why a test harness module?

smb_strdupin is only reachable via SMBIOC_T2RQ → smb_usr_t2request (smb_usr.c:308), which requires sdp->sd_share != NULL (smb_dev.c:218), which requires SMBIOC_OPENSHARE → smb_smb_treeconnect → a live SMB server. No SMB server is available on the audit guest. The strdup_test.ko module calls smb_strdupin directly (bypassing the SMB protocol) to characterize the primitive. This is a test harness, not an exploit — the finding is root-only and there is no escalation.

Fix

fix.diff changes smb_strdupin (smb_subr.c:128-130): 1. Add M_ZERO to kmalloc — defense-in-depth (buffer zeroed even if copyin fails). 2. Check copyin return — if non-zero, kfree(p) and return NULL.

This matches the pattern already used by smb_memdupin (smb_subr.c:144-147).

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):

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:

p = kmalloc(len, M_SMBSTR, M_WAITOK);
copyin(s, p, len);
return p;

to:

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).