Re: [PATCH v3] drm/xe: Fix NULL pointer dereference in xe_exec_ioctl

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

On Wed, Dec 17, 2025 at 02:56:44PM +0000, Matthew Auld wrote:
> On 17/12/2025 13:24, Tapani Pälli wrote:
> > Helper function xe_sync_needs_wait expects sync->fence when accessing
> > flags, patch makes sure we call only when sync->fence exists.
> > 
> > v2: move null checking to xe_sync_needs_wait and make
> >      xe_sync_entry_wait utilize this helper (Matthew Auld)
> > v3: further simplify code (Matthew Auld)
> > 
> > Fixes NULL pointer dereference seen with Vulkan workloads:
> > 
> > [  118.410401] RIP: 0010:xe_sync_needs_wait+0x27/0x50 [xe]
> > 
> > Fixes: 4ac9048d0501 ("drm/xe: Wait on in-syncs when swicthing to dma-fence mode")
> > Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
> 
> To me this looks reasonable, but let's wait for Matt B and get his opinion
> here also, since he was the original author.
> 

It took me a minute to figure this one out,
DRM_XE_SYNC_TYPE_TIMELINE_SYNCOBJ can set the sync->fence to NULL if the
timeline point is signaled.

So yes, this looks correct.

Thanks for the fix!

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

Going to push this one now.

> Also thanks a lot for reporting, and fixing the issue,
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> 
> > ---
> >   drivers/gpu/drm/xe/xe_sync.c | 10 ++++------
> >   1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> > index ee1344a880b9..c8fdcdbd6ae7 100644
> > --- a/drivers/gpu/drm/xe/xe_sync.c
> > +++ b/drivers/gpu/drm/xe/xe_sync.c
> > @@ -238,10 +238,8 @@ int xe_sync_entry_add_deps(struct xe_sync_entry *sync, struct xe_sched_job *job)
> >    */
> >   int xe_sync_entry_wait(struct xe_sync_entry *sync)
> >   {
> > -	if (sync->flags & DRM_XE_SYNC_FLAG_SIGNAL)
> > -		return 0;
> > -
> > -	return dma_fence_wait(sync->fence, true);
> > +	return xe_sync_needs_wait(sync) ?
> > +		dma_fence_wait(sync->fence, true) : 0;
> >   }
> >   /**
> > @@ -252,8 +250,8 @@ int xe_sync_entry_wait(struct xe_sync_entry *sync)
> >    */
> >   bool xe_sync_needs_wait(struct xe_sync_entry *sync)
> >   {
> > -	return !(sync->flags & DRM_XE_SYNC_FLAG_SIGNAL) &&
> > -		!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &sync->fence->flags);
> > +	return sync->fence &&
> > +	       !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &sync->fence->flags);
> >   }
> >   void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
>