DF-2204 — amdgpu_xgmi: no device-removal path, stale/UAF; non-idempotent add (Low)

Claim

Two related defects in sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:

1. No removal path. There is no amdgpu_xgmi_remove_device() / amdgpu_xgmi_fini() anywhere in the tree (zero list_del of xgmi.head — confirmed by grep). Once a device is list_add_tail()-ed into hive->device_list (line 85) it stays linked forever, even after amdgpu_device is torn down and freed. Any later amdgpu_xgmi_add_device() for any device in the same hive then iterates the freed node (UAF) via list_for_each_entry at lines 86 and 98.

2. Non-idempotent add. amdgpu_xgmi_add_device() never checks whether adev is already linked. A second call (e.g. driver reset / rebind re-entering amdgpu_device_init without first removing the node) list_add_tail()s an already-linked node, corrupting the doubly-linked list (infinite loops / visits to arbitrary addresses).

Verification approach

HW-gated / source-only. Reachable only on Vega20 hardware with a driver reset / unbind-rebind cycle. No GPU on this audit guest. Per the task brief, source-only confirmation is acceptable.

1. grep the entire sys/dev/drm tree for xgmi.head / amdgpu_xgmi_remove / amdgpu_xgmi_fini — confirm the only list_add_tail/iterators are in amdgpu_xgmi.c and there is no list_del.
2. Confirm list_add_tail is unconditional (no list_empty check).
3. Confirm amdgpu_device_fini() (the natural teardown counterpart to amdgpu_device_init's amdgpu_xgmi_add_device() call) does not call any xgmi cleanup.
4. Author fix.diff (idempotency guard + new amdgpu_xgmi_remove_device + hook into amdgpu_device_fini).
5. Phase 8 — apply all 5 batched fixes and rebuild amdgpu.ko with -Werror.

Source trace (confirmed)

• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:85 — only list_add_tail(&adev->gmc.xgmi.head, ...) in the tree.
• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:86,98 — only iterators over hive->device_list.
• grep -rn 'list_del.*xgmi.head\|amdgpu_xgmi_remove\|amdgpu_xgmi_fini' sys/dev/drm → 0 matches (confirmed).
• sys/dev/drm/amd/amdgpu/amdgpu_device.c:1691 — call to amdgpu_xgmi_add_device(adev) (init).
• sys/dev/drm/amd/amdgpu/amdgpu_device.c:2727-2779 — amdgpu_device_fini() does not call any xgmi teardown before freeing adev.
• sys/dev/drm/amd/amdgpu/amdgpu.h:1252 — only amdgpu_xgmi_add_device is declared; no remove counterpart.

Files

• VERDICT.md — full narrative.
• fix.diff — three-part fix in three files:
• amdgpu_xgmi.c: add idempotency guard (if (!list_empty(...)) goto exit) before the existing list_add_tail; append amdgpu_xgmi_remove_device() that does list_del + INIT_LIST_HEAD under the xgmi_mutex.
• amdgpu.h: declare void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);.
• amdgpu_device.c: call amdgpu_xgmi_remove_device(adev) near the top of amdgpu_device_fini(), before any of the device's memory is freed.
• build_baseline.log — unpatched amdgpu.ko build, rc=0, -Werror.
• build_patched.log — patched amdgpu.ko rebuild (amdgpu_xgmi.o + amdgpu_device.o recompiled), rc=0, -Werror.
• env.txt — guest environment.

Reproduce

./build.sh    # applies fix.diff + rebuilds amdgpu.ko
./run.sh      # HW-gated no-op (see VERDICT.md)

DF-2204 — VERDICT

Verdict: REPRODUCED (source-only, HW-gated).

Class: UAF + list corruption (memory corruption).

Mechanism

Two related lifetime defects in the amdgpu XGMI bookkeeping.

(1) No removal path → stale node → UAF

amdgpu_xgmi_add_device() (sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c) links a device into a hive:

/* amdgpu_xgmi.c:85 */
list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);

There is no counterpart anywhere in the tree. Verified by exhaustive grep of sys/dev/drm:

$ grep -rn 'list_del.*xgmi\.head\|amdgpu_xgmi_remove\|amdgpu_xgmi_fini' sys/dev/drm
(0 matches)

The only operations on xgmi.head in the entire tree are the list_add_tail at line 85 and the two iterators at lines 86 and 98.

The natural teardown counterpart of the init call (amdgpu_device_init → amdgpu_xgmi_add_device(adev) at amdgpu_device.c:1691) is amdgpu_device_fini() (amdgpu_device.c:2727). Reading the whole function (lines 2727-2779) confirms it never calls any xgmi cleanup. So once a Vega20 GPU is added to a hive, its gmc.xgmi.head node stays linked into hive->device_list forever — across kfree(adev), across module unload, across driver rebind.

The next amdgpu_xgmi_add_device() for any other device in the same hive then list_for_each_entry-walks hive->device_list (lines 86 and 98) and dereferences the freed amdgpu_device → classic UAF read.

(2) Non-idempotent add → list corruption

list_add_tail() at line 85 is unconditional. There is no list_empty(&adev->gmc.xgmi.head) guard. A second amdgpu_xgmi_add_device() for the same adev (which a driver reset / unbind-rebind path will trigger, because amdgpu_device_init re-runs without first removing the node) re-links an already-linked node. In a doubly-linked list, that produces a corrupted structure: the same node appears twice, loops can become infinite, or iterators visit arbitrary addresses.

Trigger surface / reachability

amdgpu_xgmi_add_device() early-returns for non-Vega20 ASICs and APUs, so this is Vega20-only. The realistic trigger is a driver reset / rebind cycle on a Vega20 host (root or root-equivalent; unbind/rebind of a PCI device). On this audit guest there is no GPU, so the UAF / list corruption cannot be exercised live; the bug is confirmed at the source level and the fix compiles cleanly (Phase 8).

Fix

fix.diff is a three-part patch:

• amdgpu_xgmi.c (part 1): add an idempotency guard before the existing list_add_tail — if adev->gmc.xgmi.head is not empty the device is already linked, log a warning and goto exit with ret = 0.
• amdgpu_xgmi.c (part 2): append a new amdgpu_xgmi_remove_device(adev) that, under the existing xgmi_mutex, does list_del + INIT_LIST_HEAD if the node is linked.
• amdgpu.h: declare the new function next to amdgpu_xgmi_add_device.
• amdgpu_device.c: call amdgpu_xgmi_remove_device(adev) near the top of amdgpu_device_fini() (right after adev->shutdown = true), before any of the device's memory or MMIO is freed.

Phase 8 build validation

Applied fix.diff (plus the four other batched DRM fixes) to the in-guest /usr/src, rebuilt amdgpu.ko with -Werror: * baseline (unpatched) amdgpu.ko: rc=0 (build_baseline.log). * patched amdgpu.ko: rc=0, amdgpu_xgmi.o and amdgpu_device.o recompiled (build_patched.log); no warnings, no errors.

Verdict

REPRODUCED at source level (HW-gated; no live repro possible on guest). Both halves of the claim — no removal path and non-idempotent add — are confirmed by exhaustive grep and by reading amdgpu_device_fini end-to-end. The fix compiles cleanly under -Werror.