# DF-1095 — iicsmb_bwrite leaves I2C bus in STARTED state on write failure

## Verdict

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

The bug exists verbatim in `sys/bus/iicbus/iicsmb.c:462-482`. The
`iicsmb_bwrite` function is the **only** SMBus method in the file whose
`error:` cleanup label does **not** call `iicbus_stop()`. When
`iicbus_start()` succeeds (setting `sc->started = slave`, `iiconf.c:178`)
and a subsequent `iicbus_write()` fails (NACK/bus error/timeout), the
function returns via the bare `error:` label at `:480-481` without
issuing STOP. `sc->started` stays set; every subsequent `iicbus_start()`
on that bus returns `EINVAL` (`iiconf.c:174-175`) until driver reload or
reboot. The entire SMBus bridge is therefore permanently wedged for all
consumers (`/dev/smb`, kernel sensors, battery/thermal monitors, IPMI
SSIF).

The audit QEMU guest has **no `iicbus`/`iicsmb` devices** in `dmesg`
(no I2C controller in `pciconf -l`), so the path is not exercised at
runtime here. The trigger requires either local root with an exposed
`/dev/smb` whose slave NACKs the data byte, or a malicious/untrusted
I2C peripheral. The bug is confirmed by source trace + harness.

The `df1095_harness` userspace C program mirrors the iicbus state
machine (`sc->started`) and the exact `iicsmb_bwrite` call graph. In
the **unpatched** mode, phase 1 (start succeeds, write NACKs) leaves
`started = slave` and no `stop` call is made; phase 2 then sees
`iicbus_start` return `EINVAL` (`rc=22`) — the bridge is wedged. In the
**patched** mode (`iicbus_stop()` in the error label), phase 1 calls
stop, and phase 2 succeeds.

## Mechanism (confirmed by source trace)

`iicbus_start` (`iiconf.c:169-183`):

```c
int
iicbus_start(device_t bus, u_char slave, int timeout)
{
    struct iicbus_softc *sc = ...;
    int error = 0;

    if (sc->started)
        return (EINVAL);                  /* :175 — bus already started */

    if (!(error = IICBUS_START(...)))
        sc->started = slave;              /* :178 — mark bus STARTED */
    else
        sc->started = 0;

    return (error);
}
```

`iicbus_stop` (`iiconf.c:213-226`) clears `sc->started = 0` (`:224`).

`iicbus_request_bus` and `iicbus_release_bus` (`iiconf.c:86-148`) manage
only `sc->owner`, NOT `sc->started` — confirmed by `verify.sh` check 8.

`iicsmb_bwrite` (`iicsmb.c:462-482`):

```c
if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
    goto error;                           /* :468-469 — start FAILED, no STOP needed */
if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
    goto error;                           /* :471-472 — start SUCCEEDED, STOP needed */
if ((error = iicbus_write(parent, buf, (int)count, &sent, IICBUS_TIMEOUT)))
    goto error;                           /* :474-475 — start SUCCEEDED, STOP needed */
if ((error = iicbus_stop(parent)))
    goto error;                           /* :477-478 — redundant */

error:
    return (error);                       /* :480-481 — NO STOP!!! */
```

The first `goto error` (start failed) is safe — `sc->started` was not
set. The second and third `goto error`s (start succeeded, write failed)
are buggy — `sc->started` is still set when the function returns.

Compare `iicsmb_bread` (`:484-507`):

```c
if ((error = iicbus_start(...)))
    return (error);                       /* :490-491 — early return */
...
error:
    iicbus_stop(parent);                  /* :505 — ALWAYS stop */
    return (error);                       /* :506 */
```

That pattern is the canonical correct form. `iicsmb_bwrite` is the sole
outlier — every other read/write function in `iicsmb.c` calls
`iicbus_stop()` in its error label.

## Reproduction

```
$ sh verify.sh        # 8/8 static checks
$ cc -O0 -o df1095_harness df1095_harness.c
$ cc -O0 -DFIX -o df1095_harness_fix df1095_harness.c
$ ./df1095_harness        # phase 2: rc=22 (EINVAL), started != 0
$ ./df1095_harness_fix    # phase 2: rc=0, started == 0
```

## Fix

`fix.diff` rewrites `iicsmb_bwrite` to mirror `iicsmb_bread`:

1. Change `iicbus_start` failure from `goto error` to `return error`
   (no STOP needed when start failed — `sc->started` was not set).
2. Move `iicbus_stop()` into the `error:` label so it is always called
   on the success path AND on the write-failure paths.
3. Remove the redundant explicit `iicbus_stop()` + `goto error` at
   `:477-478` (now handled by the error label).

The `nativekernel` build of the patched file succeeds; the harness
validates the algorithm-level correctness.