DF-5089 — si_copy_dma discards radeon_sync_resv()/radeon_sync_rings() returns (SI twin of DF-4871)

File: sys/dev/drm/radeon/si_dma.c (pass 2)

What

si_copy_dma() calls both sync functions and ignores that both return int (si_dma.c:255-256). On any sync failure the DMA copy packets are still emitted and the ring committed (:258-279), so the GPU-side copy runs unordered with respect to a pending write on the same bo — silent corruption of the migrated buffer's contents, with a healthy-looking fence on top.

This is the SI twin of DF-4871 (r600_dma.c:466-467, same pass-2 batch family) and was explicitly flagged in the r600 pass-2 task_log ("identical ignored radeon_sync_resv/rings returns at 255-256 — check in own-file audits"). DF-4871's verdict already cross-referenced this exact line pair.

Failure modes that make the ignored return real (radeon_sync.c): - radeon_sync_rings() → -EINVAL "Syncing to a disabled ring" (radeon_sync.c:153-156) — reachable when a source ring went not-ready (failed ib test / GPU reset path). - interrupted manual radeon_fence_wait() fallback (:160-163) and radeon_semaphore_create() SA-allocator failure (:165-167) on memory pressure. - radeon_sync_resv() → interrupted dma_fence_wait(f, true) on a foreign (non-radeon) resv fence (radeon_sync.c:106, 119).

Contrast: the sibling caller radeon_ib_schedule() checks the identical call and unwinds with radeon_ring_unlock_undo() (radeon_ib.c:149-154).

Reproduce

Runtime trigger requires real SI (Tahiti/Pitcairn/Verde/Oland/Hainan/ Sea Islands-PCIe) hardware in a degraded state — the audit guest has none (QEMU std VGA, 0 drm modules, no /dev/dri — env.txt). Hardware-gated Low finding, per contract PoC-skippable; recorded untested. Line-level trace in VERDICT.md.

Fix

See fix.diff — checks both returns, moves radeon_sync_resv() before the ring lock (its waits may sleep and must not hold rdev->ring_lock), unwinds with radeon_ring_unlock_undo() + radeon_sync_free() + ERR_PTR(r) on radeon_sync_rings() failure. Mirrors the DF-4871 fix shape.

DF-5089 VERDICT — si_copy_dma commits the DMA copy even when cross-ring sync fails

Status: untested (hardware-gated; no SI GPU on the verification guest). Impact: silent GPU bo-content corruption on degraded SI hardware (I:L). Confidence: certain — the ignored int returns are unambiguous line-level facts; the failure modes of the sync layer are traced below.

1. The defect

si_copy_dma() (si_dma.c:231-283) — the ttm bo-migration copy callback for all SI asics (radeon_asic.c si_asic: .copy.dma = &si_copy_dma, .copy.dma_ring_index = R600_RING_TYPE_DMA_INDEX; reached from radeon_move_bl()/radeon_ttm.c on every VRAM⇄GTT move):

248:    r = radeon_ring_lock(rdev, ring, num_loops * 5 + 11);
249:    if (r) { ... return ERR_PTR(r); }
...
255:    radeon_sync_resv(rdev, &sync, resv, false);
256:    radeon_sync_rings(rdev, &sync, ring->idx);
257:
258:    for (i = 0; i < num_loops; i++) { ...emit copy packets... }
...
272:    r = radeon_fence_emit(rdev, &fence, ring->idx);
279:    radeon_ring_unlock_commit(rdev, ring, false);

Both functions return int (radeon.h:628, radeon.h:632) and both returns are discarded. On failure the copy is emitted and committed anyway.

2. How the sync layer fails (all paths in this tree)

radeon_sync_resv() (radeon_sync.c:89-125): returns the error from dma_fence_wait(f, true) on any non-radeon exclusive/shared resv fence (:106, :119 — interruptible wait, e.g. signal during a long foreign-fence wait under memory pressure).

radeon_sync_rings() (radeon_sync.c:137-199) — documented "The caller must hold the ring lock" (:135): - -EINVAL when the fence's source ring is not ready (:153-156, "Syncing to a disabled ring!") — a ring becomes !ready after a failed ib test or GPU reset handling (radeon_ib.c sets ring->ready = false on ib-test failure), which is exactly the degraded state a bo migration can then race with; - error from the manual radeon_fence_wait() fallback when RADEON_NUM_SYNCS is exhausted (:158-163); - error from radeon_semaphore_create() (SA allocator, -ENOMEM) (:165-167) or radeon_ring_alloc() on the source ring (:172-174); - error from the manual wait when semaphore emission failed (:177-193).

On any of these, the correct behavior is to abort the copy (ttm falls back to memcpy in radeon_move_bl()); instead the copy is committed unordered — the destination bo can hold stale/partial data while carrying a good fence, i.e. silent data corruption of migrated GEM contents. This is the exact defect class already accepted as DF-4871 for r600_copy_dma() (r600_dma.c:466-467, Low); DF-4871's verdict cross-referenced si_dma.c:255-256 as an open sibling. Same class, same severity calibration: CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U/C:N/I:L/A:N, CWE-754.

The check-and-unwind pattern this file should use exists 90 lines away in the same subsystem: radeon_ib_schedule() treats the identical radeon_sync_rings() failure as fatal (radeon_ib.c:149-154: radeon_ring_unlock_undo(rdev, ring); return r;).

3. Why not higher severity / why not memory-safety

• The unordered window corrupts the bo's own contents, not kernel memory; there is no length/pointer consequence — the ring writes stay within the locked, mask-bounded circular ring.
• Trigger requires SI hardware in a degraded state (disabled source ring, allocation failure, or interrupted foreign-fence wait) coinciding with a bo migration — hence AC:H.

4. Pass-2 negative notes for si_dma.c (classes hunted and killed)

• DF-2091 (known, not re-reported): size_in_bytes = (num_gpu_pages << 12) u32 truncation for ≥4GB moves (:246); num_loops derives from the truncated value (:247).
• fill sizes: no fill function exists in si_dma.c (DMA_PACKET_CONSTANT_FILL appears only in r600/evergreen CS validators) — nothing to audit.
• vm_*_pages integer math: count*8 (:75) would overflow only at count ≥ 2^29; callers bound nptes ≤ ~3.07e8 via the ndw > 0xfffff → -ENOMEM gate in radeon_vm_bo_update (radeon_vm.c:997-998), and PDE counts are ≤ 2^22 — no overflow. Caps 0xFFFF8/0xFFFFE fit the 20-bit DMA_PACKET count fields (sid.h:1853-1866); chunk decrements are exact multiples (0xFFFF8 = 8·131071; ndw always even) → loops terminate with exact consumption.
• IB overflow via emitters: ib->ptr[ib->length_dw++] writes are unbounded in the emitters but every caller's estimate dominates the emission: copy 5 dws/chunk vs estimate 7/cmd; write 3+2/PTE vs 4/cmd + 2·nptes; set 9/chunk vs 10/cmd + 2·10 + 64 padding (radeon_vm.c:971-994, wrapper dispatch radeon_vm.c:359-379; fragment splits add ≤2 boundary calls, covered by padding; full-block PTE ranges are fragment-aligned because PT BOs are ≥4KB-aligned). PDE update path: count<3 breaks emit ≤7 dws vs 6/PDE estimate (radeon_vm.c:653-656 vs wrapper write path 3+2·count). WARN_ON tripwires exist at radeon_vm.c:706/1021.
• ring space / OOB write: radeon_ring_alloc() guarantees ndw < actual free dwords (radeon_ring.c:125-137 waits on fences until space exists) and radeon_ring_write() masks wptr into the circular buffer (radeon.h:2708) — over-commit (e.g. ≥2 semaphore waits vs the 4-dw slack after the 7-dw fence in num_loops*5 + 11) only trips the DRM_ERROR at radeon.h:2710; ring_free_dw is decremented per actual write so later allocs stay truthful. Upstream-inherited accounting wart, not memory corruption.
• vm flush indexes: vm_id comes from radeon_vm_grab_id() (radeon_vm.c:177-222) bounded to [1, nvm-1], nvm = 16 for SI (si.c:4787), so both register branches (:192-196) address existing VM_CONTEXT0-15 registers and 1 << vm_id (:207, :213) is defined (≤ 1<<15). radeon_vm_flush() passes exactly that id (radeon_vm.c:250-251).
• fence pairs / error paths: fence-emit failure unwinds with radeon_ring_unlock_undo + radeon_sync_free(NULL) (:272-277) — the sync->semaphores[] array is released by radeon_sync_free, no leak (this file has no ib_test — SI DMA rings reuse r600_dma_ib_test, radeon_asic.c:1901-1915, so the DF-4870 leak twin belongs to r600_dma.c, already filed).
• Sibling callers with the same ignored returns (NOT this file, for the orchestrator's cross-refs): evergreen_dma.c:130-131, cik.c:3695-3696.

5. Why untested (honest status)

Runtime verification needs SI-class Radeon hardware driving the ttm migration path while a sync dependency fails; the verification guest is QEMU std-VGA-only (env.txt: chip=0x11111234, 0 drm modules, no /dev/dri). Hardware-gated Low finding → per contract PoC-skippable → recorded untested (the persist enum has no not_testable/skipped member).

6. Fix

fix.diff mirrors the accepted DF-4871 fix shape: check radeon_sync_resv() before taking the ring lock (its waits may sleep and must not hold rdev->ring_lock), check radeon_sync_rings() after the lock and unwind with radeon_ring_unlock_undo() + radeon_sync_free() + ERR_PTR(r).

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.