Re: [PATCH v2] [PATCH v2] drm/i915/gem: Fix UAF race in eb_relocate_vma

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Mar 24, 2026 at 11:17:41AM -0400, Yassine Mounir wrote:
> Fix a race condition in Linux 7.0-rc2 where a GEM object could be freed
> during relocation if userspace closes the handle concurrently.
> 
> The fix involves pinning the object lifetime using i915_gem_object_get()
> before the relocation loop and releasing it via i915_gem_object_put()
> in the common exit path (out label), ensuring symmetry in both success
> and error paths.
> 
> This v2 rebases the change to the new 'gem/' directory structure in
> the current mainline tree and addresses potential memory leaks in
> early error returns.
> 
> Signed-off-by: Yassine Mounir <sosohero200@gmail.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index e7918f896..0468c0551 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1528,6 +1528,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
>  	if (unlikely(!access_ok(urelocs, remain * sizeof(*urelocs))))
>  		return -EFAULT;
>  
> +	i915_gem_object_get(ev->vma->obj);
>  	do {
>  		struct drm_i915_gem_relocation_entry *r = stack;
>  		unsigned int count =
> @@ -1588,6 +1589,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
>  		urelocs += ARRAY_SIZE(stack);
>  	} while (remain);
>  out:
> +	i915_gem_object_put(ev->vma->obj);
>  	reloc_cache_reset(&eb->reloc_cache, eb);
>  	return remain;


Ignoring the AI slop aspect, I did have a quick look at the code a bit
and noticed this:

eb_lookup_vma() {
	...
	rcu_read_lock();
	vma = radix_tree_lookup(...);
	if (likely(vma && vma->vm == vm))
		vma = i915_vma_tryget(vma);
	rcu_read_unlock();
	if (likely(vma))
		return vma;
	...
}

So if we somehow get a vma with the wrong vm there then we
return the vma without grabbing a reference to it.


Should we not do something like this?

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

-- 
Ville Syrjälä
Intel