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

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Jun 15, 2026 at 09:11:13PM +0200, Francois Dugast wrote:
> 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.

Yes, it doess seem to be an improvement to bail instead of crashing the
driver. I guess let's keep the WARN_ON so we can chase this and
hopefully root cause.

Can you add comment that we think that triggering the WARN_ON is
unexpected, thus it is in the code base, but it happens under some
conditions and better to WARN + bail that crash the driver?

Matt

> 
> 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
> > >