DF-5163 — amdgpu atom-i2c scratch race (missing port of radeon scratch_mutex)

PoC verdict: not_reproduced on audit guest (virtio VGA, amdgpu never attaches). Structural race proven by inspection; see VERDICT.md.

Reproduce (on amdgpu hardware)

Two hw_capable DDC buses under concurrent probe (amdgpu_hw_i2c=1 default in this tree), e.g. repeated display hotplug polling on two connectors while reading EDIDs; look for EDID bytes from one connector appearing on the other (torn/interleaved reads). ddc_race_sketch.c documents the window; there is no userland-only driver for this path on DragonFly (the amdgpu i2c adapters are internal to drm DDC probing).

Artifacts

• ddc_race_sketch.c — documented trigger sketch (compile-clean)
• fix.diff — 4-file port of radeon's scratch_mutex fix (git-apply-able)
• VERDICT.md — full line-cited analysis

DF-5163 — amdgpu atom-i2c reads the shared ATOM scratch outside any scratch lock (radeon's scratch_mutex fix never ported)

Verdict (short)

Not reproduced on the audit guest — the guest VGA is virtio (chip 0x11111234, Red Hat), amdgpu never attaches, and the race requires real ASICs with two hw-capable DDC buses under concurrent probe load. The structural defect is proven by inspection (likely confidence: the window is unambiguous in the source; manifestation needs concurrent atom-fb users, which is plausible during multi-connector hotplug storms but was not observable here). Memory-safe: the read is bounded (num <= 255, atombios_i2c.c:79) and the scratch is >= 20 KiB (amdgpu_atombios.c:1813-1821) — impact ceiling is cross-contaminated DDC data, not corruption.

The defect, line by line

1. amdgpu_atombios_i2c_process_i2c_ch() takes only the per-bus lock: mutex_lock(&chan->mutex) — sys/dev/drm/amd/amdgpu/atombios_i2c.c:52. chan is the single i2c channel; it does NOT serialize anything across different buses.

2. The atom table is executed synchronously — amdgpu_atom_execute_table(adev->mode_info.atom_context, index, ...) — sys/dev/drm/amd/amdgpu/atombios_i2c.c:94. Inside, ctx->mutex serializes concurrent executions (sys/dev/drm/amd/amdgpu/atom.c:1263) and is released when execute_table returns (atom.c:1278).

3. The firmware deposits the received i2c bytes in the shared fb scratch atom_context->scratch (allocated once per GPU: amdgpu_atombios.c:1814-1821, default 20 KiB, may be enlarged per VBIOS), and the driver then copies them out: amdgpu_atombios_copy_swap(buf, base, num, false) — sys/dev/drm/amd/amdgpu/atombios_i2c.c:103-104 — with no lock held that another atom-fb user respects.

4. Window: bus X executes (scratch := Xdata); bus Y executes (scratch := Ydata); bus X's copy_swap returns Ydata (or a torn mix) as X's i2c read. Any concurrent atom command that touches the fb window (a second ProcessI2cChannelTransaction on another DDC bus is the natural one — the algorithm is selected per-bus at sys/dev/drm/amd/amdgpu/amdgpu_i2c.c:187-190 whenever rec->hw_capable && amdgpu_hw_i2c, and amdgpu_hw_i2c defaults to 1 in this tree, sys/dev/drm/amd/amdgpu/amdgpu_drv.c:88) can interleave.

5. The radeon twin already fixed this class: sys/dev/drm/radeon/atom.h:128 declares struct lock scratch_mutex, it is initialized at sys/dev/drm/radeon/radeon_device.c:994 (LK_CANRECURSE), taken around the whole table execution in sys/dev/drm/radeon/atom.c:1247-1252, and held across execute+read by sys/dev/drm/radeon/atombios_i2c.c:51/99 (and atombios_dp.c:101/149). The amdgpu struct atom_context (sys/dev/drm/amd/amdgpu/atom.h:128-147) has no such member — the fix was never ported to the amdgpu copy of this driver pair.

Impact

• Data confusion only: a DDC/EDID read attributed to the wrong bus, or torn. EDID parsing then consumes bytes from the other connector's transaction. No OOB (num bounded; scratch oversized), no uninit leak (scratch is kzalloc'd), no write to shared state (copy direction is scratch→buf).
• Severity Info; realistic ceiling is a misdected monitor — worth fixing because the identical code path in radeon was deemed race-worthy upstream, and because the same unprotected post-execute scratch read also exists in sys/dev/drm/amd/amdgpu/atombios_dp.c:114 (DP aux path), which the same one-lock fix umbrella would cover.

Fix

fix.diff (4 files, mirrors the radeon fix exactly): - atom.h: add struct lock scratch_mutex to struct atom_context; - atom.c: rename the body to amdgpu_atom_execute_table_scratch_unlocked() and make the public amdgpu_atom_execute_table() hold scratch_mutex around it (lock order scratch → ctx->mutex, same as radeon); - atombios_i2c.c: hold scratch_mutex across execute+copy_swap (LK_CANRECURSE makes the nesting with execute_table legal); - amdgpu_atombios.c: lockinit at the context-creation site next to the existing agmiacm lockinit.

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.