DF-5148 — AUX completion poll off-by-one misreports a completed transfer as -EIO

File: sys/dev/drm/radeon/radeon_dp_auxch.c Severity: Info (correctness; no memory-safety impact) Status: untested (control-flow arithmetic proved by executed harness; on-HW reproduction impossible in the audit guest — no AMD/ATI PCI device, QEMU emulated VGA only, so radeon_dp_aux_transfer_native() can never attach/run there; see env.txt)

What

radeon_dp_aux_transfer_native() polls the AUX_SW_STATUS register for AUX_SW_DONE (radeon_dp_auxch.c:152-158):

    int retry_count = 0;
    do {
        tmp = RREG32(AUX_SW_STATUS + aux_offset[instance]);
        if (tmp & AUX_SW_DONE) {
            break;
        }
        usleep_range(100, 200);
    } while (retry_count++ < 1000);

    if (retry_count >= 1000) {              /* keyed on the COUNTER */
        DRM_ERROR("auxch hw never signalled completion, error %08x\n", tmp);
        ret = -EIO;
        goto done;
    }

The do-while executes the body with retry_count == 0..1000 (1001 body executions): after the body entered with retry_count == 999, the guard retry_count++ < 1000 increments to 1000 and re-enters the body one last time. If the hardware asserts AUX_SW_DONE during that 1001st poll, the loop breaks with retry_count == 1000 — and the counter-keyed error gate then reports a completed transaction as -EIO ("auxch hw never signalled completion"). The correct signal is available in tmp at that point; the gate just tests the wrong variable.

Secondary aggravations (same loop, noted for completeness): * DFBSD's usleep_range(100,200) maps to DELAY(100) (bounded busy-wait, sys/dev/drm/include/linux/delay.h:55-58), so a wedged AUX block busy-waits up to ~100 ms per transaction with chan->mutex held (locked at radeon_dp_auxch.c:96) — 100x upstream Linux's udelay(1)-based ~1 ms. Bounded, but a latency wart under retry storms. * The error goto done paths (lines 163/168/174) skip the second AUX_SW_DONE_ACK clear at line 194; stale DONE state is however re-acked by the unconditional write at line 145 of the next call before GO is set, so this is cosmetic.

Impact

A DisplayPort AUX transaction that completes on the final poll iteration is spuriously failed with -EIO. Upper layers (drm_dp_aux / DPCD accessors / link training) retry aux transactions, so the practical effect is a rare transient failure (EDI D/EDID read, DPCD access, link training step), not a memory-safety issue. Info severity.

Related known findings (pass 1, NOT part of this one): DF-2146 (unbounded AUX_SW_REPLY byte count vs 16-byte buffer), DF-2147 (unguarded aux_offset[instance]).

Reproduce (control-flow proof)

./build.sh    # compiles loop_offbyone.c on the audit QEMU guest (or any POSIX cc)
./run.sh      # runs the stimulus table

Expected decisive output (see run.log):

DONE on 1001st poll  (rc=1000)  <-- last body -5             0              0   BUG: completed xfer misreported as -EIO
never DONE (true HW timeout)           -5             -5             -5

RESULT: shipped loop misclassifies a DONE-on-last-poll transfer as -EIO (radeon_dp_auxch.c:160).

The "fixed gate" column shows the proposed if (!(tmp & AUX_SW_DONE)) gate behaves identically to the shipped gate for every other stimulus, i.e. the fix only flips the misclassified case.

Fix

fix.diff — gate the timeout on the observed status word, not the counter. ```

VERDICT — DF-5148

Verdict: real code defect, certain from the control flow; Info-class correctness bug with no security impact. PoC status untested — the audit guest has no radeon GPU (env.txt: pciconf -l shows no AMD/ATI device, QEMU emulated VGA only), so radeon_dp_aux_transfer_native() cannot execute on real hardware there; per contract, Info-class findings skip QEMU verification. The loop arithmetic itself was executed and proved on the guest with loop_offbyone.c (run.log): a DONE asserted on the 1001st poll body exits the loop with retry_count == 1000 and the counter-keyed gate at sys/dev/drm/radeon/radeon_dp_auxch.c:160 returns -EIO for a completed transfer, while the proposed status-keyed gate returns success; every other stimulus (DONE at polls 1, 501, 1000; never) is classified identically by both gates.

Why it happens (path:line)

• sys/dev/drm/radeon/radeon_dp_auxch.c:66 — int retry_count = 0;
• :152-158 — do { tmp = RREG32(AUX_SW_STATUS...); if (tmp & AUX_SW_DONE) break; usleep_range(100,200); } while (retry_count++ < 1000); The guard is evaluated after the body, so the body runs with retry_count == 0..1000 inclusive (1001 executions; note retry_count is only incremented in the guard).
• :160-164 — if (retry_count >= 1000) { DRM_ERROR("auxch hw never signalled completion..."); ret = -EIO; goto done; } — a DONE observed in the body entered with retry_count == 1000 breaks the loop with the counter at exactly 1000, so a successful completion is misclassified.
• Aggravation: sys/dev/drm/include/linux/delay.h:55-58 maps usleep_range(min,max) to DELAY(min) (busy-wait), so the poll spins ~100 µs per iteration, up to ~100 ms per failed transaction while holding chan->mutex (locked at :96, released at :199). Upstream Linux uses udelay(1) (~1 ms worst case). Bounded — no infinite wait exists in this file.

Why Info and not higher

• No memory-safety consequence: the misclassified path takes the same goto done cleanup (mutex release) that real timeouts take; tmp, ack, buf are untouched.
• The wrong result surfaces as -EIO to drm_dp_aux callers, which own retry/nack handling; effect is a rare transient DP aux failure.
• Trigger requires real DisplayPort hardware completing the transaction in the ~100 ms window's final poll.

Fix validation

fix.diff replaces the counter test with !(tmp & AUX_SW_DONE). Not boot-tested (fix_status: not_testable): requires radeon+DP hardware; the harness proves behavioral equivalence for all non-buggy stimuli and the correct classification for the buggy one. sys/ remains untouched (diff authored read-only).

Negative results recorded during this pass-2 review (classes hunted, killed)

• Waits/timeouts: the completion poll is the ONLY loop besides the two bounded for-loops (i < msg->size <= 16 at :138; the reply loop is known DF-2146). No unbounded/forever wait in this file.
• NACK/retry handling: absent by design here (ack nibble propagated at :201-202 to the drm_dp_aux core, which owns retries).
• Native vs I2C bridge: request dispatch (:74-84) masks MOT and accepts only the 4 canonical request types, else -EINVAL; msg->size > 16 rejected at :71-72 (WARN_ON + -E2BIG); msize = size-1 fits u8; write loop bounded by the checked size.
• Register field overflow: AUX_SW_WR_BYTES(x)=((x&0x1f)<<16) (nid.h:840) self-masks, bytes <= 20 fits; AUX_HPD_SEL(x)=((x&0x7)<<20) (nid.h:832) self-masks and rec.hpd is a radeon_hpd_id enum (<= RADEON_HPD_6), and the field is pre-cleared via tmp &= AUX_HPD_SEL(0x7) at :106.
• buf == NULL handled at :187/:190; mutex held across the whole MMIO sequence (:96-:199).
• DF-2146 (:40,:177-191 reply byte count up to 31 vs <=16-byte buffer) and DF-2147 (:44-52,:63 aux_offset[0..15] vs 6 entries) are the known pass-1 findings and are deliberately not re-reported here.

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.