# DF-4584 — radeon_bo_get_surface_reg cross-BO steal race (NULL-deref / slab UAF write)

## One-line

`radeon_bo_get_surface_reg()` steals hardware surface registers from other
BOs with **no lock covering the victim**: `reg->bo` (a pointer to another
`radeon_bo`) is read, dereferenced (`old_object->pin_count`, later
`ttm_bo_unmap_virtual(&old_object->tbo)`) and written
(`old_object->surface_reg = -1`) while the victim's final unref path
(`radeon_ttm_bo_destroy` → `radeon_bo_clear_surface_reg` → `kfree(bo)`) can
run concurrently, giving a NULL-deref panic (easy) or a 4-byte `0xFFFFFFFF`
write at a fixed offset into freed kmalloc-slab memory (harder).

## Full narrative

### Root cause (all in `sys/dev/drm/radeon/radeon_object.c`)

* `radeon_bo_get_surface_reg()` opens with
  `lockdep_assert_held(&bo->tbo.resv->lock.base)` (radeon_object.c:609) —
  it holds the reservation of the BO **being registered** only.
* The registry `rdev->surface_regs[RADEON_GEM_MAX_SURFACES=8]`
  (radeon.h:2406, radeon.h:449) is device-global.  When all 8 slots are
  busy, the function picks a victim ("steal"):

  ```c
  620  steal = -1;
  621  for (i = 0; i < RADEON_GEM_MAX_SURFACES; i++) {
  623      reg = &rdev->surface_regs[i];
  624      if (!reg->bo) break;
  627      old_object = reg->bo;
  628      if (old_object->pin_count == 0)     /* <-- victim deref, unlocked */
  629          steal = i;
  630  }
  ...
  637  reg = &rdev->surface_regs[steal];
  638  old_object = reg->bo;                   /* <-- re-read, unlocked */
  641  ttm_bo_unmap_virtual(&old_object->tbo); /* <-- deref */
  642  old_object->surface_reg = -1;           /* <-- WRITE to victim */
  ```

* The victim teardown path is
  `radeon_ttm_bo_destroy` (radeon_object.c:69-86) →
  `radeon_bo_clear_surface_reg` (radeon_object.c:656-669) → `kfree(bo)`
  (radeon_object.c:85).  `radeon_bo_clear_surface_reg` writes
  `reg->bo = NULL; bo->surface_reg = -1;` (lines 667-668) under **no lock
  that the stealer holds** — the stealer's resv belongs to a different BO.

### Interleavings

1. Stealer reads non-NULL `reg->bo` at :624/:627 or :638; victim destroy
   executes :667 (`reg->bo = NULL`) and completes `kfree(bo)`; stealer then
   executes :628 (`old_object->pin_count`) or :641/:642 on freed memory →
   **slab UAF read + 4-byte write of `0xFFFFFFFF` at
   `offsetof(struct radeon_bo, surface_reg)` into a freed kmalloc object**
   (`struct radeon_bo` embeds `ttm_buffer_object`, several hundred bytes —
   a groomable slab zone reachable with ordinary GEM create/close).
2. Stealer checks `!reg->bo` at :624; victim destroy sets `reg->bo = NULL`
   in between; stealer reads `old_object = NULL` at :627 → :628
   NULL-deref → **kernel panic**.  Same outcome at :638→:641.
3. Two stealers picking the same slot corrupt the registry bookkeeping
   (bounded — no OOB; wrong surface configuration, GPU-side effects only).

Interleaving 2/3 have wide windows (dozens of instructions) and are
trivially winnable by repetition; interleaving 1's UAF-write window
(:638→:642) is tighter but stable under CPU pinning.

### Unprivileged reachability (traced)

* `radeon_gem_set_tiling_ioctl` (radeon_gem.c:503-519) — DRM ioctl, any
  owner of the GEM handle, no privilege/family check →
  `radeon_bo_set_tiling_flags` (radeon_object.c:671) stores
  `RADEON_TILING_SURFACE` (0x10) in `bo->tiling_flags` after validating
  only the EG bankw/bankh/… sub-fields (not the SURFACE bit).
* mmap of the handle + page fault → TTM cpu-fault →
  `radeon_bo_fault_reserve_notify` (radeon_object.c:793) →
  `radeon_bo_check_tiling(bo, 0, 0)` (:804) → with
  `mem_type == TTM_PL_VRAM` and `surface_reg == -1` (:757, :766) →
  `radeon_bo_get_surface_reg` (:769).
  (`has_moved` is never non-zero anywhere in the tree — the only live
  entry routes are `radeon_bo_kmap` :289 and `fault_reserve_notify` :804.)
* Occupying all 8 regs is unprivileged: create 8+ VRAM BOs, set
  SURFACE tiling, fault each.  Every 9th registration takes the steal path.

### Verification attempt (this run)

* Trigger: `df4584_trigger.c` — occupies all 8 surface regs, then races
  registration (steal path) against register+destroy (clear path) on two
  pthreads, 20000 iterations per run.
* Built and ran on the audit QEMU guest (DragonFly 6.5-DEVELOPMENT):
  `cc -O2 -Wall -pthread` → **build OK**; run →
  `open /dev/dri/renderD128: No such file or directory` (and card0).
  The guest has **no radeon hardware** (only virtio-VGA 0x1111:1234, no
  DRM attach, `/dev/dri` absent) and **no radeonkms.ko** in `/boot/kernel`
  (only radeonkmsfw_* firmware modules).  The cited code path is therefore
  live-unreachable on this guest — neither reproducible nor refutable here.
* Conclusion: **inconclusive on-guest; code-trace certain** (the lock
  protocol violation is provable by inspection — the only locks involved
  are per-BO reservations and the destroy path holds none of them).

## Recommended fix

Serialize registry access with a device-level lock (see `fix.diff`):
`struct lock rdev->surface_regs_lock` held across
`radeon_bo_get_surface_reg()`'s registry walk/steal and across
`radeon_bo_clear_surface_reg()`'s body.  Because the victim's destroy
calls `radeon_bo_clear_surface_reg` **before** `kfree(bo)`, holding this
lock in the stealer guarantees the victim cannot be freed mid-steal: if
its destroy already passed the clear, `reg->bo` is NULL and the slot is
simply free.  (Authored post-verification against the read-only tree;
never applied.)

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