Re: [PATCH] drm/xe/pt: Fix NULL pointer dereference in xe_pt_zap_ptes_entry()

[Francois Dugast <francois.dugast@intel.com>]

On Mon, Jun 15, 2026 at 11:07:31AM -0700, Matthew Brost wrote:
> On Mon, Jun 15, 2026 at 11:19:04AM +0200, Francois Dugast wrote:
> > The page-table walk framework may pass a NULL *child pointer for
> > unpopulated entries. xe_pt_zap_ptes_entry() called container_of(*child)
> > before checking for NULL, then dereferenced the result, causing a crash.
> > 
> 
> Can you explain when this occurs?

This occurs when running specific workloads for extended periods of time
but I have not been able to identify the root cause yet. But in any case
it seems like a bug to first dereference then check for NULL, it should
be the other way around, which is what this patch is attempting to do.

> 
> > Move the container_of() call after a NULL guard, so the function
> > returns early instead of proceeding with an invalid pointer. Return 0
> > rather than an error code so that the walk continues to process
> > remaining entries.
> > 
> > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pt.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index 15ce77ce7793..123f97b9ee42 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -885,11 +885,14 @@ static int xe_pt_zap_ptes_entry(struct xe_ptw *parent, pgoff_t offset,
> >  {
> >  	struct xe_pt_zap_ptes_walk *xe_walk =
> >  		container_of(walk, typeof(*xe_walk), base);
> > -	struct xe_pt *xe_child = container_of(*child, typeof(*xe_child), base);
> > +	struct xe_pt *xe_child;
> >  	pgoff_t end_offset;
> >  
> > -	XE_WARN_ON(!*child);
> >  	XE_WARN_ON(!level);
> > +	if (XE_WARN_ON(!*child))
> 
> If this can happen in regular operation then likely drop the warn on.
> 
> Typcially warn_on means this shouldn't be possible unless somewhere else
> in the stack has a bug.

As per my comment above, it is not clear to me at this point if this counts
regular operation but exiting more gracefully when *child is NULL seems like
an improvement.

If you think this is preferable, we could also return -EINVAL to interrupt
the walk instead of potentially hiding a deeper issue, while still improve
robustness by handling NULL.

Francois

> 
> Matt
> 
> > +		return 0;
> > +
> > +	xe_child = container_of(*child, typeof(*xe_child), base);
> >  
> >  	/*
> >  	 * Note that we're called from an entry callback, and we're dealing
> > -- 
> > 2.43.0
> >