Re: [PATCH 1/2] i915/gt: tolerate ERR_PTR pd in gen6_ppgtt teardown

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

On Wed, Nov 04, 2026 at 03:33:10PM +0800, Chengyu Liu wrote:
> gen6_ppgtt_create() stores the result of gen6_alloc_top_pd() into
> ppgtt->base.pd and, on failure, jumps to err_put with the ERR_PTR still
> in the field.  i915_vm_put() runs vm->cleanup = gen6_ppgtt_cleanup(),
> which calls gen6_ppgtt_free_pd(), where gen6_for_all_pdes() immediately
> dereferences pd - with an ERR_PTR that is an invalid dereference and a
> crash (in the async release worker).  The free_pd() right after is only
> guarded against NULL, not ERR_PTR.
> 
> Bail out of gen6_ppgtt_free_pd() for an invalid pd and skip free_pd()
> likewise, so an allocation failure under memory pressure stays a plain
> -ENOMEM instead of an oops.

The correct fix would be to not assign the error pointer
into ppgtt->base.pd in the first place.

Looks like a somwhat common bad pattern in this code that
received pointers are assigned directly into their final
location, and if the pointer is an error then the pointer is
cleared. IMO one should never do that because it's very easy
to forget the clearing part. Instead one should always use 
local variable first, and only assign the final pointer after
having checked that the pointer is valid.

> 
> Found by static analysis.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
> ---
>  drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> index 438cd4724a..72a9ad4642 100644
> --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
> @@ -259,6 +259,9 @@ static void gen6_ppgtt_free_pd(struct gen6_ppgtt *ppgtt)
>  	struct i915_page_table *pt;
>  	u32 pde;
>  
> +	if (IS_ERR_OR_NULL(pd))
> +		return;
> +
>  	gen6_for_all_pdes(pt, pd, pde)
>  		if (pt)
>  			free_pt(&ppgtt->base.vm, pt);
> @@ -271,7 +274,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>  	gen6_ppgtt_free_pd(ppgtt);
>  	free_scratch(vm);
>  
> -	if (ppgtt->base.pd)
> +	if (!IS_ERR_OR_NULL(ppgtt->base.pd))
>  		free_pd(&ppgtt->base.vm, ppgtt->base.pd);
>  
>  	mutex_destroy(&ppgtt->flush);
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel