From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
Date: Sun, 12 Apr 2026 12:54:46 +1000
Message-ID: <review-patch1-20260408082859.69823-1-joonas.lahtinen@linux.intel.com>
In-Reply-To: <20260408082859.69823-1-joonas.lahtinen@linux.intel.com>
References: <20260408082859.69823-1-joonas.lahtinen@linux.intel.com> <20260408082859.69823-1-joonas.lahtinen@linux.intel.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**The Bug:**

In `eb_lookup_vma()`, the fast path does a radix tree lookup and then checks if the VMA belongs to the correct VM (ppGTT):

```c
vma = radix_tree_lookup(&eb->gem_context->handles_vma, handle);
if (likely(vma && vma->vm == vm))
    vma = i915_vma_tryget(vma);
// BUG: if the condition is false, vma still holds the stale pointer
rcu_read_unlock();
if (likely(vma))
    return vma;  // returns VMA from wrong VM, without refcount!
```

Without the `else vma = NULL;`, when a VMA is found in the radix tree but belongs to a **different** VM (`vma->vm != vm`), the code falls through with `vma` still pointing to the looked-up (wrong-VM) VMA. The subsequent `if (likely(vma))` check sees a non-NULL pointer and **returns it**.

This causes two distinct problems:

1. **Wrong VM VMA returned**: The caller gets a VMA bound to a different address space than intended. If the same BO is used across EXECBUF calls with alternating VMs on the same context, the wrong VMA could be selected, leading to incorrect GPU address translations.

2. **Missing refcount (potential UAF)**: `i915_vma_tryget()` is only called when the VM matches, so the returned VMA has no reference taken on it. Once the RCU read lock is released, nothing prevents the VMA from being freed, creating a use-after-free condition.

**The Fix:**

```c
if (likely(vma && vma->vm == vm))
    vma = i915_vma_tryget(vma);
else
    vma = NULL;
```

Setting `vma = NULL` on the else branch ensures the fast path is skipped when the VM doesn't match, correctly falling through to the slow path which calls `i915_vma_instance(obj, vm, NULL)` to find or create the proper VMA for the correct VM, with proper refcounting.

**Assessment**: The fix is correct and minimal. The `else` branch also covers the case where `vma` was NULL from the radix tree lookup (a no-op assignment but harmless). The patch is well-described, references the original report, and has appropriate tags. No concerns.

---
Generated by Claude Code Patch Reviewer