DF-4737 — amdpm_wait() off-by-one (silent SMBus timeout / spurious last-poll failure)

File: sys/dev/powermng/amdpm/amdpm.c (pass-2 GLM 5.3 re-audit) Sibling of DF-4549 (viapm.c, same while(count--) / if (!count) defect family).

Build

On the audit QEMU guest (DragonFly 6.5-DEVELOPMENT, cc 8.3):

cc -Wall -Wextra -o /tmp/df4737 /tmp/df4737.c

(amdpm_wait_offbyone_demo.c transferred to the guest first — see build.sh)

Run

/tmp/df4737

Expected output (decisive lines)

A  clear_on_read=0     polls=10000  error=0x0    NO-TIMEOUT
   -> genuine timeout (controller never clears HST_STS) reported as
      SMB_ENOERR; amdpm_recvb/readb/readw/bread then return stale
      HSTDATA/FIFO bytes as a SUCCESSFUL transfer
B  clear_on_read=10000 polls=10000  error=0x20   TIMEOUT-REPORTED
   -> transfer that completes on the final (10000th) poll is spuriously
      reported SMB_ETIMEOUT
bugs demonstrated in buggy loop: 2   (exit 0)

The demo's "fixed loop" section re-runs A and B against the one-line fix (if (count < 0)) and shows both cases behave correctly.

Why a userspace replica

The in-kernel path is hardware-gated: amdpm(4) attaches only to AMD 756/766/768/8111 PMU (1022:740b/7413/7443/746b) or nForce SMBus (10de:01b4) PCI devices; the QEMU audit guest has none (guest_hw_evidence.txt: kldload amdpm succeeds, attaches nothing, no /dev/smb*). The loop is pure C semantics, so a verbatim replica of amdpm_wait()'s control flow with a fixture register read proves the defect deterministically. Same method as the accepted DF-4549 evidence pack.

DF-4737 VERDICT — amdpm_wait() off-by-one

Status: not_reproduced at runtime (hardware-gated) — logic defect certain, proven by guest-compiled replica of the exact loop; impact ceiling limited.

Root cause (path:line)

sys/dev/powermng/amdpm/amdpm.c:300-335, amdpm_wait():

int count = 10000;          /* :302 */
...
while(count--) {            /* :308 */
        DELAY(10);
        sts = AMDPM_SMBINW(sc, AMDSMB_GLOBAL_STATUS);
        if (!(sts & AMDSMB_GS_HST_STS))
                break;      /* :313 */
}
...
if (!count)                 /* :319 */
        error |= SMB_ETIMEOUT;  /* :320 */

while(count--) post-decrements on every test, including the final failing one. Exhausting the loop therefore leaves count == -1, never 0:

• Case A — genuine timeout: all 10000 polls (~100 ms) see HST_STS still set; the loop exits with count == -1; !count is false; SMB_ETIMEOUT is never set; the function returns SMB_ENOERR. All ten callers (amdpm_quick :366, amdpm_sendb :393, amdpm_recvb :419-420, amdpm_writeb :448, amdpm_readb :475-476, amdpm_writew :504, amdpm_readw :531-532, amdpm_bwrite :576, amdpm_bread :610-622) treat this as success: reads return the stale HSTDATA register / 32-byte-FIFO content as valid data; writes are reported complete although the controller never executed them. Because error == SMB_ENOERR, amdpm_clear() (:331-332) is skipped, so the sticky HST_STS stays latched and the next transfer fails amdpm_idle() with SMB_EBUSY — the controller is wedged but the first failure is silent.
• Case B — completion on the final poll: the break at :312-313 fires on the 10000th iteration when count has already reached 0; !count is true; a transfer that did complete is reported SMB_ETIMEOUT.

This is the identical defect filed as DF-4549 for viapm_wait() (viapm.c:569/574/587) — both drivers descend from alpm.c. The third sibling, intpm.c (intsmb_wait, :560-585), uses explicit return (SMB_ETIMEOUT) and is why intpm passed pass-2 clean.

Proof

amdpm_wait_offbyone_demo.c replicates the loop verbatim (DELAY dropped — no control-flow effect) with a fixture GLOBAL_STATUS read; compiled with guest cc 8.3 and executed on the guest (run.log):

• A (perpetual busy): error=0x0 — timeout missing, silent success. BUG.
• B (clears on poll #10000): error=0x20 (SMB_ETIMEOUT) — spurious. BUG.
• C (clears on poll #5000): error=0x0 — correct (control).
• Fixed variant (if (count < 0)): A → 0x20, B → 0x0 — both correct, validating fix.diff in silico.

Reachability / threat model

• Userspace reachability is via the smbus(4) char device: /dev/smbN is created UID_ROOT/GID_WHEEL mode 0600 (sys/dev/smbus/smb/smb.c:135-141), so the attacker is already root — OR an in-kernel smbus consumer (e.g. battery/thermal sensors on amd756/766/768/8111 and nForce chipsets) silently reading wrong values with no error. Data-integrity defect, no memory corruption: lengths/buffers are unaffected (bwrite clamps 1..32 at :548; bread's buffer writes are guarded at :618).
• Hardware gate: requires an AMD 756/766/768/8111 PMU or nForce SMBus controller. The QEMU audit guest has none — kldload amdpm loads the module but nothing attaches, and /dev/smb* does not exist (guest_hw_evidence.txt). Runtime reproduction of the in-kernel path on this guest is impossible; hence not_reproduced with certain-by-C-semantics logic proof (same convention as DF-4549).

Fix

One line, amdpm.c:319: if (!count) → if (count < 0). Loop exhaustion leaves count == -1 (timeout, correctly flagged); a break on the final iteration leaves count == 0 (success, correctly not flagged). See fix.diff. (An equivalent alternative: a done flag set at the break, as recommended for DF-4549.)

Fix validation

Not testable on this guest (the kernel path needs AMD/nForce PMU hardware; a single-fix kernel build would have no observable input to feed it). The demo's built-in fixed-loop variant executes the exact fixed predicate against both failure modes and behaves correctly — recorded in run.log.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.