DF-4522 — amdgpu_vm_fini's DragonFly cleanup loop is dead code (rb_leftmost never maintained) → leaked VM mappings

File: sys/dev/drm/amd/amdgpu/amdgpu_vm.c:3319-3334 Class: kernel memory leak (driver-local, M_DRM) / porting defect — CWE-401 Severity: Low (requires AMD GPU; abnormal-but-real teardown paths) Confidence: certain (static proof + on-kernel semantic demo) Status: runtime not testable on the audit guest (no AMD GPU); mechanism proven on the running kernel; fix compile-validated (module builds).

What is broken

The DragonFly branch of amdgpu_vm_fini() drains leftover mappings with:

while (vm->va.rb_leftmost) {                       /* amdgpu_vm.c:3325 */
        mapping = container_of((void *)vm->va.rb_leftmost, ...);
        list_del(&mapping->list);
        amdgpu_vm_it_remove(mapping, &vm->va);
        kfree(mapping);
}

But on this port struct rb_root_cached::rb_leftmost (sys/dev/drm/include/linux/rbtree.h:64-67) is never written by anyone:

• vm->va is zero-initialized (vm->va = LINUX_RB_ROOT_CACHED, amdgpu_vm.c:3055; LINUX_RB_ROOT_CACHED = { {NULL,}, NULL }, rbtree.h:175);
• the insert path amdgpu_vm_it_insert() (amdgpu_vm.c:107-127) calls rb_insert_color_cached(&node->rb, root, false), whose shim macro (rbtree.h:100-101) ignores the leftmost argument and only rebalances root->rb_root;
• amdgpu_vm_it_remove() → rb_erase_cached (rbtree.h:102-103) likewise;
• grep -rn rb_leftmost sys/ → the ONLY references in the whole kernel tree are the two read sites of this loop (amdgpu_vm.c:3325-3326).

So vm->va.rb_leftmost is deterministically NULL for the VM's entire lifetime, the while body never executes, and every mapping still in vm->va when amdgpu_vm_fini() runs is leaked (kfree at :3333 never runs; dev_err("still active bo inside vm") at :3313-3315 fires first).

This is NOT an infinite loop and NOT a wild pointer deref — the field is NULL, not garbage (explicit init + kzalloc'd fpriv, amdgpu_kms.c:979).

When it leaks

Normal GEM handle close removes mappings (amdgpu_gem_object_close → amdgpu_vm_bo_rmv, amdgpu_gem.c:185 → amdgpu_vm.c:2847-2859), so a perfectly clean exit empties the tree. The leak hits the paths upstream Linux deliberately handles in this same loop:

• amdgpu_gem_object_close() reserve failure — amdgpu_gem.c:170-175 prints "leaking bo va because we fail to reserve bo" and returns without rmv, leaving mappings in vm->va for fini to clean up (upstream frees them here; this port leaks them);
• any driver/teardown state that triggers the upstream "still active bo inside vm" condition (amdgpu_vm.c:3313);
• the KFD destroy callback is an empty stub on this port (amdgpu_amdkfd.c:534-537) — no additional cleanup happens there.

Leaked objects are struct amdgpu_bo_va_mapping (~80 bytes, kmalloc M_DRM at amdgpu_vm.c:2526/2580), one per mapping, repeatable per VM open/ close cycle by any user with access to an AMD GPU's /dev/dri/* node.

Pack contents / how to reproduce

• static_proof.sh — airtight source-level proof (runs on the host): 5 assertions, each with path:line. sh static_proof.sh → PASS.
• df4522_demo.c + Makefile — kld that includes the real sys/dev/drm/include/linux/rbtree.h and runs the verbatim amdgpu interval-tree shim (amdgpu_vm.c:100-127) plus the verbatim fini loop (amdgpu_vm.c:3325-3334) against a populated tree on the running kernel, then the fixed loop on an identical tree.
• build.sh / run.sh — exact commands used against the guest.
• build.log — demo kld build (guest, cc 8.3, -Werror, clean).
• run.log — kldload output, the decisive lines: DF4522: inserted 64 mappings per tree; tree_nonempty=1 rb_leftmost=0 rb_first=0xfffff8008d8a20b8 DF4522: BUGGED fini loop: iterations=0 freed=0 LEAKED=64 (tree still nonempty=1) DF4522: FIXED fini loop: iterations=64 freed=64 LEAKED=0 (tree empty=1)
• baseline_build.log — unmodified amdgpu.ko full module build (RC=0).
• fix_build.log — fix.diff applied in guest /usr/src, incremental rebuild (RC=0, only amdgpu_vm.o recompiled, module relinked).
• fix.diff — the fix (loop on RB_EMPTY_ROOT + rb_first(), which is a real RB_MIN walk on this port, rbtree.h:97/104).
• env.txt — guest uname / kldstat / cc version.

The demo cannot and does not claim a runtime hit on amdgpu_vm_fini itself: the guest has no AMD GPU, so amdgpu.ko never attaches. The mechanism (shim never maintains rb_leftmost → verbatim loop idles) is proven on the running kernel with the real header.

Not affected (checked)

• drm_mm.c (DF-4521's file): uses rb_first_cached (drm_mm.c:496) which is a real RB_MIN walk — functionally correct, never reads the cache field directly.
• generic interval_tree_generic.h consumers: lookups go through rb_first_cached → RB_MIN — correct; the un-maintained cache only costs the O(1) optimization.
• i915_gem_userptr.c:181, radeon_mn.c:225: same generic-shim pattern, no direct rb_leftmost reads.
• amdgpu_mn.c:412 uses undefined RB_ROOT_CACHED — dead file, not in the module build (not in sys/dev/drm/amd/amdgpu/Makefile SRCS).

DF-4522 VERDICT — verified real (dead cleanup loop / mapping leak); mechanism proven on-kernel, runtime gated on absent AMD hardware

Verdict: REAL BUG, proven statically (certain) and empirically at shim level on the running DragonFly kernel. The amdgpu_vm_fini() DragonFly cleanup walk is dead code; leftover VM mappings leak. NOT an infinite loop / wild deref (the lead's worst-case hypothesis is refuted: the field is deterministically NULL, never garbage).

1. The lead, and what is actually true

Lead (from the drm_mm.c pass-2, DF-4521 negative notes): "amdgpu_vm.c:3325-3326 loops on vm->va.rb_leftmost, but amdgpu's own DFly interval-tree insert never sets rb_leftmost … the fini/teardown walk may loop forever (if rb_leftmost is NULL/garbage) or leak mappings."

Verified point by point:

1. Nothing sets vm->va.rb_leftmost — CONFIRMED. rg -n 'rb_leftmost' sys/ yields exactly two hits, both the reads inside the fini loop itself (amdgpu_vm.c:3325, amdgpu_vm.c:3326). No writer exists anywhere in the kernel tree. The insert path is amdgpu_vm_it_insert (amdgpu_vm.c:107-127) → rb_insert_color_cached(&node->rb, root, false); the shim (sys/dev/drm/include/linux/rbtree.h:100-101) discards the leftmost parameter and only runs linux_root_RB_INSERT_COLOR on root->rb_root. amdgpu_vm_it_remove → rb_erase_cached (rbtree.h:102-103) likewise never touches the cache field. sys/sys/tree.h (the underlying RB implementation) has no leftmost-cache concept at all (no match for leftmost in the file).

2. NULL or garbage? — Deterministically NULL. vm->va is explicitly initialized: vm->va = LINUX_RB_ROOT_CACHED; (amdgpu_vm.c:3055, inside amdgpu_vm_init), and LINUX_RB_ROOT_CACHED is (struct rb_root_cached) { {NULL,}, NULL } (rbtree.h:175). The containing struct amdgpu_fpriv is kzalloc'd besides (amdgpu_kms.c:979). So the field cannot be stale garbage.

3. Infinite loop? — NO. while (vm->va.rb_leftmost) with the field NULL-forever executes zero iterations, always. The lead's "loop forever" hypothesis is dead: there is no non-NULL value to loop on, and the loop body (which removes nodes) never runs to re-read a stale value.

4. Leak? — YES. The loop body at amdgpu_vm.c:3331-3333 (list_del, amdgpu_vm_it_remove, kfree(mapping)) is the only cleanup for mappings still in vm->va at teardown (upstream Linux frees exactly these with rbtree_postorder_for_each_entry_safe, amdgpu_vm.c:3317-3318 guarded #ifndef __DragonFly__). Zero iterations ⇒ every remaining struct amdgpu_bo_va_mapping (kmalloc'd M_DRM at amdgpu_vm.c:2526 and :2580) leaks permanently. The preceding dev_err(adev->dev, "still active bo inside vm") (amdgpu_vm.c:3313-3315) fires in exactly the leaking cases — the kernel even tells you it is about to leak.

5. Which function? amdgpu_vm_fini() (amdgpu_vm.c:3282). There is no amdgpu_vm_cleanup() in this port. Both fini call sites share the walk: amdgpu_kms.c:1015 (open error path; tree empty there, moot) and amdgpu_kms.c:1073 (amdgpu_driver_postclose_kms, the real teardown).

2. Reachability of the leak

The normal close path empties the tree: GEM handle close → amdgpu_gem_object_close (amdgpu_gem.c:152) → amdgpu_vm_bo_rmv (amdgpu_gem.c:185) → mappings unlinked at amdgpu_vm.c:2847-2859. So the dead loop is harmless on perfectly-clean exits. It leaks on the paths the upstream cleanup exists for:

• Reserve-failure path, by design: amdgpu_gem_object_close on ttm_eu_reserve_buffers() failure prints "leaking bo va because we fail to reserve bo (%d)" and returns without calling amdgpu_vm_bo_rmv (amdgpu_gem.c:170-175). The comment/name says it: the bo_va (and its tree mappings) are deliberately left for amdgpu_vm_fini to reclaim. Linux reclaims them in this very loop; this port leaks every one.
• Any "still active bo inside vm" condition (upstream's dev_err): driver bug or unclean teardown states leave mappings behind.
• KFD does not help: amdgpu_amdkfd_gpuvm_destroy_cb is an empty stub in this port (amdgpu_amdkfd.c:534-537).

Attacker/impact model: any local user with access to an AMD GPU's DRM node can create thousands of mappings per VM and, via the reserve-fail or other abnormal exit paths, leak them all; repeat per open/close cycle. Bounded by mapping count per cycle (~80 bytes each), permanent, unbounded across cycles. No memory-safety consequence (no UAF, no double-free: the leaked nodes are simply never referenced again).

3. Empirical proof on the running kernel (mechanism level)

df4522_demo.c (kld) includes the real shim header (sys/dev/drm/include/linux/rbtree.h), runs the verbatim amdgpu interval-tree shim (amdgpu_vm.c:100-127) and the verbatim fini loop (amdgpu_vm.c:3325-3334) on a 64-node tree, then the fixed loop on an identical tree. Guest: DragonFly 6.5-DEVELOPMENT x86_64 (env.txt). Decisive output (run.log):

DF4522: inserted 64 mappings per tree; tree_nonempty=1 rb_leftmost=0 rb_first=0xfffff8008d8a20b8
DF4522: BUGGED  fini loop: iterations=0 freed=0 LEAKED=64 (tree still nonempty=1)
DF4522: FIXED  fini loop: iterations=64 freed=64 LEAKED=0 (tree empty=1)

Tree populated, rb_first() (real RB_MIN walk) finds nodes, yet rb_leftmost is NULL → the verbatim loop idles and leaks 64/64; the fix loop drains 64/64. kldload/kldunload clean, guest healthy.

4. Why status is not_testable (not reproduced)

The audit guest has no AMD GPU (kldstat: kernel + ehci/xhci only, env.txt); amdgpu.ko never attaches, so the real amdgpu_vm_fini() path cannot be driven at runtime. Per the honest-status table this is not_testable / reproduced=0 / impact=none for the poc_results row — the mechanism-level demo and the airtight static proof carry the certainty instead (confidence: certain for the dead-code property itself).

5. Fix — validated at module-build level

fix.diff replaces the dead loop with a live one that walks the real tree (rb_first() = RB_MIN, rbtree.h:97; both macros already used in this file at runtime: rb_first_cached at amdgpu_vm.c:77 via amdgpu_vm_it_iter_first, RB_EMPTY_ROOT at amdgpu_vm.c:3313):

-   while (vm->va.rb_leftmost) {
-       mapping = container_of((void *)vm->va.rb_leftmost, struct amdgpu_bo_va_mapping, rb);
+   while (!RB_EMPTY_ROOT(&vm->va.rb_root)) {
+       mapping = rb_entry(rb_first(&vm->va.rb_root),
+                  struct amdgpu_bo_va_mapping, rb);

Validation performed on the guest:

• baseline: unmodified amdgpu.ko full build, RC=0 (baseline_build.log);
• git apply --check clean on the host repo;
• fix applied in guest /usr/src, incremental rebuild RC=0 with -Werror (fix_build.log; only amdgpu_vm.o recompiled, amdgpu.ko relinked);
• guest source restored to pristine (sha256 match) after the build;
• fixed-loop semantics proven on-kernel by the demo (64/64 freed).

An equally valid alternative (closer to upstream) is deleting the #ifndef __DragonFly__ special-case entirely and using rbtree_postorder_for_each_entry_safe, which this port's rbtree.h already provides (rbtree.h:135-140) — the porter's comment "the RB iterator can not be used" is stale. The minimal diff above was preferred.

6. Other consumers with the same problem — none

rb_leftmost direct readers in sys/: only amdgpu_vm.c:3325-3326 (proof step [1] in static_proof.sh). All other rb_root_cached users go through rb_first_cached() (rbtree.h:104), which is a real RB_MIN walk and therefore correct even with the cache un-maintained — they merely lose the O(1) fast path:

• drm_mm.c — rb_insert_color_cached (drm_mm.c:227, 271), rb_erase_cached (drm_mm.c:293), rb_first_cached(&mm->holes_size) (drm_mm.c:496): correct. (Consistent with DF-4521's pass-2 negative notes.)
• interval_tree_generic.h (OpenBSD/FreeBSD-derived shim) — insert computes min_entry and passes it to rb_insert_color_cached (interval_tree_generic.h:91), which drops it; lookups use rb_first_cached (interval_tree_generic.h:60) → correct.
• i915_gem_userptr.c:181, radeon_mn.c:225 (LINUX_RB_ROOT_CACHED) and amdgpu_mn.c:412 — the latter references RB_ROOT_CACHED, which is defined nowhere in sys/, but amdgpu_mn.c is not in the module build (absent from sys/dev/drm/amd/amdgpu/Makefile SRCS), so it cannot break compilation. i915_gem_userptr.c/radeon_mn.c use the generic-shim lookup pattern → functionally correct.

Root cause home: shared shim (sys/dev/drm/include/linux/rbtree.h), but the only defect consumer is amdgpu_vm.c's direct field read; filed under the consumer per the tasking, shim noted.

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.