Re: [PATCH v2] drm/i915/userptr: Fix user pages leak on get_pages failure

[Krzysztof Karas]

Hi ZhaoJinming,

On 2026-07-28 at 13:41:52 +0800, ZhaoJinming wrote:
> When ____i915_gem_object_get_pages() fails inside
> i915_gem_object_userptr_submit_init(), the pvec containing pages
> pinned by pin_user_pages_fast() was never freed:
> 
> 1. obj->userptr.pvec was set to the pinned pages
> 2. The local pvec variable was NULLed
> 3. ____i915_gem_object_get_pages() failed, but its internal
>    i915_gem_object_userptr_drop_ref() only decremented page_ref
>    from 2 to 1, not triggering the pvec cleanup
> 4. The unconditional obj->userptr.page_ref-- brought page_ref
>    to 0, but the pinned pages remained referenced only by
>    obj->userptr.pvec with no path to reclaim them
> 
> Additionally, the cache hit path could return success on a
> subsequent call despite page_ref being 0, leading to a
> GEM_BUG_ON(obj->userptr.page_ref < 0) crash in drop_ref when
> the pages were eventually invalidated.
> 
> Fix by calling i915_gem_object_userptr_drop_ref() on the
> get_pages failure path, which properly decrements page_ref
> from 1 to 0, triggering the pvec cleanup. Make the page_ref--
> after the if block conditional on success, since drop_ref
> already handles the refcount on failure.
Huh, so there was no cleanup on failure of
____i915_gem_object_get_pages, but the page_ref still got
decremented. I think the changes below should be enough to
resolve that issue, but we still need to run this through CI.

> 
> Signed-off-by: ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> index 043095f93ac6..7d2750528485 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> @@ -292,9 +292,12 @@ int i915_gem_object_userptr_submit_init(struct drm_i915_gem_object *obj)
>  		obj->userptr.notifier_seq = notifier_seq;
>  		pvec = NULL;
>  		ret = ____i915_gem_object_get_pages(obj);
> +		if (ret)
> +			i915_gem_object_userptr_drop_ref(obj);
>  	}
>  
> -	obj->userptr.page_ref--;
> +	if (!ret)
> +		obj->userptr.page_ref--;
>  
>  out_unlock:
>  	i915_gem_object_unlock(obj);
> -- 
> 2.20.1
> 

-- 
Best Regards,
Krzysztof