# DF-1091 — amdsmb_bread slave-controlled block-read count drives OOB MMIO

## Verdict

**NOT REPRODUCED at runtime (hardware-gated) — STATIC VERIFICATION + HARNESS CONFIRMED.**

The bug exists verbatim in `sys/bus/smbus/amdsmb/amdsmb.c:517-547`. The
SMBus block-read function reads the slave-supplied byte count
(`SMB_BCNT`, 0..255) into `len` and uses it as the bound for a
`for (i = 0; i < len; i++) amdsmb_ec_read(sc, SMB_DATA + i, &data)` loop.
`SMB_DATA` is a 32-byte register file (`:75`, offsets `0x04..0x23`); any
`len > 32` produces OOB reads of `SMB_BCNT` / `SMB_ALRM_*` / follow-on EC
register space — 223 reads when `len = 255`. The function also writes
`*count = len` (`:539`), passing the unclamped slave-supplied count back
to the caller, breaking the SMBus 32-byte block invariant. The
`if (i < *count) buf[i] = data;` write at `:537` is bounded by the
caller's original `*count` (the entry-check at `:524` rejects anything
outside `[1,32]`), so the **buf write** does not overflow; the **MMIO
read** and the **returned count** are both unclamped.

The audit QEMU guest has **no AMD SMBus PCI device** (`pciconf -l` shows
Intel PIIX4 PM, class `0x068000`, not the AMD-8111 SMBus controller at
`1022:746a`), so `amdsmb_bread` is never invoked at runtime here. The
trigger requires AMD-8111 SMBus hardware with a malicious I2C peripheral
on the bus, or an IPMI BMC reachable via SSIF. Same hardware-gating class
as DF-1076 (ichsmb) — the sibling finding for which the same kind of
clamp was already merged.

The `df1091_harness` userspace C program mirrors the algorithm against a
256-byte EC register model. With `SMB_BCNT = 255`:
- **Unpatched**: 223 OOB MMIO reads past `SMB_DATA[31]`, returned
  `*count = 255` (would cause `smb.c` to copyout 223 bytes of
  uninitialized stack).
- **Patched**: 0 OOB, returned `*count` clamped to 32.

## Mechanism (confirmed by source trace)

`amdsmb_bread` (`amdsmb.c:517-547`) is the SMBus block-read method of the
AMD-8111 SMBus host driver. After waiting for the previous transaction
to complete, it reads the slave-supplied byte count from `SMB_BCNT` and
loops:

```c
/* amdsmb.c:532-540 — the bug */
if ((error = amdsmb_wait(sc)) == SMB_ENOERR) {
    amdsmb_ec_read(sc, SMB_BCNT, &len);          /* :533 slave value 0..255 */
    for (i = 0; i < len; i++) {                  /* :534 bound is len       */
        amdsmb_ec_read(sc, SMB_DATA + i, &data); /* :535 OOB when i >= 32   */
        if (i < *count)
            buf[i] = data;                       /* :537 buf write bounded  */
    }
    *count = len;                                /* :539 unclamped return   */
}
```

The 32-byte `SMB_DATA` register file (`amdsmb.c:75`) is followed by
`SMB_BCNT` (`:76`, offset `0x24`), `SMB_ALRM_A` (`:77`, `0x25`),
`SMB_ALRM_D` (`:78`, `0x26`). With `len = 255`, the loop reads offsets
`0x04..0x101`, walking past the SMBus register block entirely into the
ACPI embedded-controller register space and beyond.

Three reasons this is more than a benign OOB read:

1. **EC register reads can have side effects.** Many ACPI EC registers
   are read-to-clear status flags or strobe latches; 223 spurious EC
   reads can perturb ACPI state (e.g. clear a pending SMBus alarm before
   `amdsmb_callback` sees it, or trip an EC IRQ).
2. **`*count = len` unclamped** breaks the SMBus 32-byte block invariant
   for every consumer of `smbus_bread`. In `sys/dev/smbus/smb/smb.c:318`,
   `SMB_BREAD` then does
   `s->rcount = min(s->rcount, bcount); copyout(buf, s->rbuf, s->rcount);`
   — copying up to 255 bytes from a 1024-byte `buf[1024]` whose bytes
   `32..254` were never written (because `amdsmb_bread` only wrote
   `buf[i]` for `i < *count` where the entry-check capped `*count` at 32).
   That is a per-call **uninitialized-kernel-stack leak to userspace**
   via `/dev/smb` on any host that exposes an `smb` device node bound to
   an `amdsmb` controller. (The `/dev/smb` device is root/wheel only, so
   this leak requires root/wheel — but the IPMI-SSIF path is reachable
   from any process that can talk to `ipmi0`.)
3. **`AMDSMB_LOCK` hold time** — each `amdsmb_ec_read` can `DELAY` up to
   500µs; with `len = 255` the spinlock is held for ~340ms, a localized
   DoS on the calling CPU.

The cross-driver control: `amdsmb_bwrite` (`:491-514`) takes `count` by
**value** and validates `count < 1 || count > 32` (`:497`) before any
MMIO. `amdsmb_bread` takes `count` by pointer and validates only the
caller's pre-existing `*count`, never the slave-supplied `len`. DF-1076
fixed the equivalent bug in `ichsmb` by clamping `sc->block_count` after
the ISR read it from `ICH_D0`.

## Reproduction

```
$ sh verify.sh        # 9/9 static checks
$ cc -O0 -o df1091_harness df1091_harness.c
$ cc -O0 -DFIX -o df1091_harness_fix df1091_harness.c
$ ./df1091_harness        # 223 OOB MMIO reads, *count=255
$ ./df1091_harness_fix    # 0 OOB, *count=32
```

## Fix

`fix.diff` clamps `len` to 32 immediately after it is read from
`SMB_BCNT`, mirroring DF-1076's ichsmb fix and matching the implicit
32-byte invariant enforced by the `SMB_DATA` register file layout and by
the sibling `amdsmb_bwrite`. The single inserted `if (len > 32) len = 32;`
both bounds the MMIO read loop and clamps the value returned via
`*count = len`. `nativekernel` build of the patched file succeeds; the
harness validates the algorithm-level correctness.