# DF-0835 — smbfs_advlock unconditionally overwrites lock type to SMB_LOCK_EXCL

## Verdict: REPRODUCED (logic/auth bug); FIX VALIDATED

**Status:** reproduced (deterministic code trace + compiled-code disassembly proof)
**Impact:** logic/auth violation — shared (read) locks become exclusive; unlocks within
F_SETLK acquire new exclusive locks. Local-triggered remote DoS ceiling (server lock
table exhaustion from never-released exclusive oplocks).
**Severity:** Medium (confirmed)
**Confidence:** certain

---

## Mechanism

`smbfs_advlock` (`sys/vfs/smbfs/smbfs_vnops.c:882`) is the VOP_ADVLOCK handler for
smbfs vnodes. In the `F_SETLK` case (line 925), an inner switch (lines 927–939)
correctly maps the POSIX lock type to the SMB lock operation:

```
sys/vfs/smbfs/smbfs_vnops.c:927-939 (the CORRECT mapping — dead code in baseline):
    switch (fl->l_type) {
        case F_WRLCK:  lkop = SMB_LOCK_EXCL;    break;   // exclusive
        case F_RDLCK:  lkop = SMB_LOCK_SHARED;  break;   // shared/read
        case F_UNLCK:  lkop = SMB_LOCK_RELEASE; break;   // unlock
        default:       return EINVAL;
    }
```

But then **line 943 unconditionally overwrites** the result:

```
sys/vfs/smbfs/smbfs_vnops.c:943 (THE BUG):
    lkop = SMB_LOCK_EXCL;   /* <-- unconditional overwrite, makes switch dead code */
```

Line 944 then calls `smbfs_smb_lock(np, lkop, ...)` with the overwritten value.

### Downstream effect

`smbfs_smb_lock` (`sys/vfs/smbfs/smbfs_smb.c:129`) dispatches to
`smbfs_smb_lockandx` (`sys/vfs/smbfs/smbfs_smb.c:92`), which builds the
`SMB_COM_LOCKING_ANDX` request:

- **Line 101–102:** `if (op == SMB_LOCK_SHARED) ltype |= SMB_LOCKING_ANDX_SHARED_LOCK;`
  — With `op` always `SMB_LOCK_EXCL` (0), the SHARED flag is never set → the lock
  sent to the server is always exclusive.
- **Line 115:** `mb_put_uint16le(mbp, op == SMB_LOCK_RELEASE ? 1 : 0);` (unlock count)
  — With `op` always 0, this is always 0 → no unlocks are ever issued.
- **Line 116:** `mb_put_uint16le(mbp, op == SMB_LOCK_RELEASE ? 0 : 1);` (lock count)
  — With `op` always 0, this is always 1 → a new lock is always acquired.

### Concrete consequences

1. **F_RDLCK (shared/read lock)** → sent as `SMB_LOCK_EXCL` (exclusive). A second
   reader on the same byte range is blocked — violating POSIX shared-lock semantics.
   Programs relying on concurrent read access via F_RDLCK on smbfs will serialize.
2. **F_UNLCK within F_SETLK** → sent as `SMB_LOCK_EXCL` (acquire exclusive, NOT
   release). Each "unlock" silently accumulates a new exclusive oplock on the server
   that is never released. Repeated lock/unlock cycles exhaust the server's lock
   table → **local-triggered remote DoS**.

The `case F_UNLCK:` in the outer switch (line 950–952) correctly uses
`SMB_LOCK_RELEASE` directly, so direct `F_UNLCK` advisory-lock calls (not via
`F_SETLK`) are unaffected. The bug is specifically in the `F_SETLK` code path.

---

## Reproduction evidence

### Reachability

- `smbfs_advlock` is wired as `.vop_advlock` in `smbfs_vnode_vops`
  (`sys/vfs/smbfs/smbfs_vnops.c:93`).
- `mount_smbfs` exists at `/sbin/mount_smbfs` in the guest; the `smbfs.ko` module
  loads cleanly.
- Mounting an smbfs share requires root (`vfs.usermount=0`); the post-mount
  `fcntl(F_SETLK)` call is unprivileged (any user with file access).
- Full runtime protocol-level proof requires an SMB server (the audit guest has
  none). The **deterministic code trace + compiled disassembly** is the primary
  evidence, per the task's acceptability clause for hard-to-stage logic bugs.

### Compiled-code proof (definitive)

The shipping `/boot/kernel/smbfs.ko` was disassembled. The compiler
**dead-code-eliminated the inner switch entirely** because line 943 always
overwrites `lkop`:

**BASELINE (unpatched #0 kernel) — `smbfs_advlock` F_SETLK path:**
```asm
; F_SETLK handler (at +256):
0xce20 <+256>: movzwl 0x14(%r12),%eax    ; load fl->l_type
0xce26 <+262>: sub    $0x1,%eax          ; range check only
0xce2d <+269>: ja     <EINVAL>           ; default → EINVAL
; ... NO switch assignment — dead-coded ...
; lf_advlock call, then:
0xce5e <+318>: xor    %esi,%esi          ; op = 0 = SMB_LOCK_EXCL (HARDCODED!)
0xce60 <+320>: mov    %r15,%rdi          ; np
0xce63 <+323>: callq  smbfs_smb_lock     ; ALWAYS called with op=0
```

Contrast with the correct `case F_UNLCK:` path (outer switch):
```asm
0xce02 <+226>: mov    $0x2,%esi          ; op = 2 = SMB_LOCK_RELEASE (correct!)
```

---

## Fix

**One-line removal** of the unconditional overwrite at `smbfs_vnops.c:943`.

```diff
--- a/sys/vfs/smbfs/smbfs_vnops.c
+++ b/sys/vfs/smbfs/smbfs_vnops.c
@@ -940,7 +940,6 @@
 		error = lf_advlock(ap, &np->n_lockf, size);
 		if (error)
 			break;
-		lkop = SMB_LOCK_EXCL;
 		error = smbfs_smb_lock(np, lkop, id, start, end, &scred);
```

After removal, the switch result (`lkop`) flows directly into `smbfs_smb_lock`.

### Fix validation (compiled-code before/after)

The patched `smbfs.ko` was built from `/usr/src/sys/vfs/smbfs/` with `fix.diff`
applied (build exit 0, no warnings). Disassembly confirms the switch is now **live**
— compiled as a jump table lookup:

**PATCHED — `smbfs_advlock` F_SETLK path:**
```asm
0xc5d0 <+256>: movzwl 0x14(%r14),%eax    ; load fl->l_type
0xc5d5 <+261>: sub    $0x1,%eax          ; index = l_type - 1
0xc5ef <+287>: mov    0x0(,%rax,4),%eax  ; lkop = jump_table[index]  <-- SWITCH LIVE!
0xc5f9 <+297>: mov    %eax,-0x4c(%rbp)   ; save lkop to stack
; ... lf_advlock call ...
0xc60c <+316>: mov    -0x4c(%rbp),%esi   ; op = lkop (from switch, NOT hardcoded!)
0xc621 <+337>: callq  smbfs_smb_lock
```

**Jump table** at `.rodata+0x300` = `[1, 2, 0]`:

| l_type | constant | index (l_type−1) | table value | SMB op | Correct? |
|--------|----------|-------------------|-------------|--------|----------|
| 1 | F_RDLCK | 0 | 1 | SMB_LOCK_SHARED | ✓ shared/read |
| 2 | F_UNLCK | 1 | 2 | SMB_LOCK_RELEASE | ✓ unlock |
| 3 | F_WRLCK | 2 | 0 | SMB_LOCK_EXCL | ✓ exclusive |

The patched module loads cleanly (`kldload smbfs` succeeds; `kldstat` confirms).

---

## PoC changes

The finding's PoC folder (`findings/poc/DF-0835/`) was created from scratch (the
orchestrator had not seeded it). Contents:
- `trigger.c` — C program issuing `fcntl(F_SETLK, F_RDLCK)` on an smbfs file;
  documents the syscall surface (full runtime proof needs an SMB share).
- `fix.diff` — the one-line removal (git-apply-able, validated).
- `baseline_disasm.txt` — full unpatched `smbfs_advlock` disassembly.
- `patched_disasm.txt` — full patched `smbfs_advlock` disassembly + jump table.
- `fix_build.log` — patched module build output.
- `fix_run.log` — before/after disassembly comparison.

## Impact ceiling

- **Shared-lock semantics violation:** F_RDLCK requests serialize readers (auth/logic).
- **Remote DoS:** F_UNLCK-via-F_SETLK accumulates unreleased exclusive oplocks on the
  server; repeated calls exhaust the server lock table.
- No memory corruption, no escalation path.