# 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.