• File: sys/dev/drm/radeon/radeon_gart.c
• Lines: 254–265 (unbind loop)
• Severity: Low
• CVSS 3.1: CVSS:3.1/AV:L/AC:H/PR:H/UI:N/S:U:C:L/I:L/A:L
• CWE: CWE-665 Improper Initialization, CWE-787 Out-of-bounds Write (stale write to wrong table index)
• Confidence: likely
• Status: new

Summary

In radeon_gart_unbind the inner loop that advances the GPU-page-table index t (radeon_gart.c:257) is nested inside the if (rdev->gart.pages[p]) guard, but the outer for-loop always increments the CPU-page index p (line 254).

When an unbind range contains a NULL (already-unbound) page followed by a bound page, t falls behind p and the dummy-page PTE writes for the bound page land at the wrong pages_entry[] index and the wrong VRAM table offset via radeon_gart_set_page, leaving the bound page's actual GART entries pointing at their original physical address.

radeon_gart_bind (line 302-313) does not have this bug because it unconditionally increments t.

Root cause

radeon_gart_unbind (radeon_gart.c:254-265):

for (i = 0; i < pages; i++, p++) {        /* p ALWAYS advances */
    if (rdev->gart.pages[p]) {              /* guard */
        rdev->gart.pages[p] = NULL;
        for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++, t++) {  /* t ONLY advances inside guard */
            rdev->gart.pages_entry[t] = rdev->dummy_page.entry;
            if (rdev->gart.ptr)
                radeon_gart_set_page(rdev, t, rdev->dummy_page.entry);
        }
    }
}

When pages[p] is NULL (slot already unbound), the inner j-loop that increments t is skipped entirely. But p++ in the outer for-header still fires. After one skipped slot, t == p - 1 (on x86-64 where PAGE_SIZE/RADEON_GPU_PAGE_SIZE == 1, confirmed at sys/cpu/x86_64/include/param.h:77-78 PAGE_SHIFT=12 → PAGE_SIZE=4096 == RADEON_GPU_PAGE_SIZE radeon.h:643).

The next non-NULL page's dummy writes go to pages_entry[p-1] (the skipped page's entry, already dummy) while pages_entry[p] (the page actually being unbound) is never overwritten — it retains its real DMA-address PTE.

On architectures with PAGE_SIZE > 4096 (ratio > 1) the skew is amplified by the ratio.

Contrast radeon_gart_bind (radeon_gart.c:302-313): the j-loop and its t++ are outside any NULL guard, so t and p always stay synchronized.

Threat model

The stale GART entry means the GPU's IOMMU-equivalent translation for that aperture offset still points to the physical page that was supposed to be unbound. If that physical page was returned to the allocator and reused (by another BO, the page cache, or kernel slab), the GPU can DMA to/from freed or reallocated memory — a DMA-based use-after-free / cross-object corruption.

Primary reachable trigger: radeon_gart_fini (radeon_gart.c:374-379) calling radeon_gart_unbind(rdev, 0, rdev->gart.num_cpu_pages) to unbind the entire aperture at once. This produces a mixed NULL/non-NULL range when any BOs remain bound at teardown (ring-buffer BOs, BOs evicted from VRAM by radeon_bo_evict_vram in radeon_device_fini:1563 before asic->fini → xxx_gart_fini calls radeon_gart_fini).

This path requires root privilege (module unload or shutdown). The normal unprivileged path (radeon_ttm_backend_unbind at radeon_ttm.c:669-673) always passes a single BO's uniform fully-bound range, so it does not currently trigger the desync.

During teardown the GPU rings are being stopped (rdev->shutdown=true at radeon_device_fini:1561), limiting concurrent DMA exploitation of the stale entry, but the window between the incorrect unbind and GART hardware disable (xxx_gart_disable, called AFTER radeon_gart_fini in e.g. rs600_gart_fini:627-629) is non-zero.

Proof of concept

Not directly reachable from an unprivileged DRM_IOCTL_RADEON_CS path. Requires privileged teardown with active bindings. Reproduction of the code-level bug (proving the desync writes to the wrong index):

1. Build a kernel with a radeon GPU present, load the driver.
2. Create several GTT BOs (via DRM_IOCTL_RADEON_GEM_CREATE with RADEON_GEM_DOMAIN_GTT) to populate scattered GART slots, then destroy some to create NULL holes between bound pages.
3. Trigger driver unload (kldunload radeondrm as root) — radeon_device_fini → asic fini → xxx_gart_fini → radeon_gart_fini → radeon_gart_unbind(0, num_cpu_pages).
4. Instrument radeon_gart_unbind (or add a WARN_ON(t != p * (PAGE_SIZE/RADEON_GPU_PAGE_SIZE)) after the outer loop) to observe t < expected after a NULL slot.

The observable artifact: after fini, some pages_entry[t] entries still contain a real RADEON_GART_PAGE_VALID PTE instead of dummy_page.entry (0 on pre-init, or the dummy encoding post-init).

A simpler proof: add the invariant check WARN_ON(t != (offset / RADEON_GPU_PAGE_SIZE) + (i+1) * (PAGE_SIZE / RADEON_GPU_PAGE_SIZE)) inside the outer loop and run any workload that produces a partially-bound aperture at unload — the WARN fires, proving the desync.

The orchestrator can place a minimal kernel-module patch (the WARN_ON) plus a loader.conf tunable in findings/poc/DF-NNNN/ to demonstrate on a QEMU guest with a radeon GPU.

Recommended fix

Move the t increment outside the if-guard so t always advances by PAGE_SIZE/RADEON_GPU_PAGE_SIZE per outer iteration, mirroring radeon_gart_bind.

This ensures pages_entry[] and the VRAM table are written at the correct index for every page in the range, regardless of whether the slot was previously bound.

--- a/sys/dev/drm/radeon/radeon_gart.c
+++ b/sys/dev/drm/radeon/radeon_gart.c
@@ -251,17 +251,18 @@ void radeon_gart_unbind(struct radeon_device *rdev, unsigned offset,
            WARN(1, "trying to unbind memory from uninitialized GART !\n");
        return;
    }
    t = offset / RADEON_GPU_PAGE_SIZE;
    p = t / (PAGE_SIZE / RADEON_GPU_PAGE_SIZE);
-   for (i = 0; i < pages; i++, p++) {
+   for (i = 0; i < pages; i++, p++, t += (PAGE_SIZE / RADEON_GPU_PAGE_SIZE)) {
        if (rdev->gart.pages[p]) {
            rdev->gart.pages[p] = NULL;
-           for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++, t++) {
+           for (j = 0; j < (PAGE_SIZE / RADEON_GPU_PAGE_SIZE); j++) {
                rdev->gart.pages_entry[t + j] = rdev->dummy_page.entry;
                if (rdev->gart.ptr) {
-                   radeon_gart_set_page(rdev, t,
+                   radeon_gart_set_page(rdev, t + j,
                                 rdev->dummy_page.entry);
                }
            }
        }
    }

This makes the outer loop increment both p and t unconditionally (matching radeon_gart_bind's structure at radeon_gart.c:302-313), and the inner loop indexes relative to the now-correct base t via t + j.

No behavioral change for the common case (all pages bound); only fixes the stale-entry case when a NULL slot precedes a bound slot.

References

• sys/dev/drm/radeon/radeon_gart.c:254-265 — unbind loop with t-inside-guard
• sys/dev/drm/radeon/radeon_gart.c:302-313 — bind loop with t-outside-guard (correct)
• sys/dev/drm/radeon/radeon_gart.c:374-379 — radeon_gart_fini full-aperture unbind trigger
• sys/cpu/x86_64/include/param.h:77-78 — PAGE_SIZE=4096 == RADEON_GPU_PAGE_SIZE on x86-64
• sys/dev/drm/radeon/radeon.h:643 — RADEON_GPU_PAGE_SIZE definition