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

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

On Wed, Dec 17, 2025 at 02:22:13PM +0200, Tapani Pälli wrote:
> 
> On 12/17/25 13:51, Matthew Auld wrote:
> > On 17/12/2025 10:51, Tapani Pälli wrote:
> > > 
> > > On 12/17/25 12:31, Matthew Auld wrote:
> > > > On 17/12/2025 10:27, Matthew Auld wrote:
> > > > > On 17/12/2025 06:17, 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.
> > > > > > 
> > > > > > 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>
> > > > > > ---
> > > > > >   drivers/gpu/drm/xe/xe_exec.c | 3 ++-
> > > > > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/xe/xe_exec.c
> > > > > > b/drivers/gpu/drm/xe/ xe_exec.c
> > > > > > index 730a5c9c2637..ea368f02cb9f 100644
> > > > > > --- a/drivers/gpu/drm/xe/xe_exec.c
> > > > > > +++ b/drivers/gpu/drm/xe/xe_exec.c
> > > > > > @@ -184,7 +184,8 @@ int xe_exec_ioctl(struct drm_device
> > > > > > *dev, void *data, struct drm_file *file)
> > > > > >           if (xe_sync_is_ufence(&syncs[num_syncs]))
> > > > > >               num_ufence++;
> > > > > > -        if (!num_in_sync && xe_sync_needs_wait(&syncs[num_syncs]))
> > > > > > +        if (!num_in_sync && syncs[num_syncs].fence &&
> > > > > > +            xe_sync_needs_wait(&syncs[num_syncs]))
> > > > > 
> > > > > In xe_sync_entry_parse() it looks like it will always
> > > > > populate the fence for the !signal case, otherwise throwing
> > > > > an error if that is not possible. And xe_sync_needs_wait()
> > > > > will only touch the fence if it's the signal case? So it
> > > > > seems like this should not be possible?
> > > > 
> > > > Sorry meant to type:
> > > > 
> > > > s/touch the fence if it's the signal/touch the fence if it's the
> > > > !signal/
> > > > 
> > > I'm not sure of the complete flow but it is quite easy to reproduce.
> > 
> > Oh, I think I see it now.
> > 
> > I assume the below also fixes it? There is also the case in
> > xe_sync_entry_wait() which has the same bug, I think, so would also need
> > to handle that.
> > 
> > @@ -178,6 +179,9 @@ int xe_sync_entry_parse(struct xe_device *xe, struct
> > xe_file *xef,
> > 
> > sync_in.timeline_value);
> >                         if (err)
> >                                 return err;
> > +
> > +                       if (!sync->fence)
> > +                               sync->fence = dma_fence_get_stub();
> > 
> I will try this one.
> 
> 
> > So it looks like dma_fence_chain_find_seqno() can return NULL if the

Yes, I reasoned this after a bit of digging too.

> > seqno has already signalled. Also means we are missing some IGT coverage
> > for this.

Indeed. I don't think any IGTs use syncobj timelines which is a pretty
big gap as this is basically all VK uses. IIRC this isn't first time
we'd made a change which broke syncobj timelines only for VK to
catch. I can open a Jira for a syncobj timelines IGT to be written.

Matt

> > 
> > Can you resend your patch but perhaps we move the check into
> > needs_wait() and then add a similar check in xe_sync_entry_wait() also?
> > Or perhaps might be cleaner to have xe_sync_entry_wait() first check
> > needs_wait(), which will now also check for a NULL fence?
> > 
> Yes, that sounds fine to me, I'll add null check to needs_wait().
> 
> 
> > > 
> > > Here is the stacktrace snippet:
> > > 
> > > [  118.410401] RIP: 0010:xe_sync_needs_wait+0x27/0x50 [xe]
> > > [  118.410940] Code: 90 90 90 0f 1f 44 00 00 55 48 89 f1 48 89 e5 48
> > > 83 ec 08 48 83 7e 08 00 0f 84 79 be 1c 00 31 c0 f6 41 4c 01 75 11 48
> > > 8b 41 08 <48> 8b 40 30 48 d1 e8 83 e0 01 83 f0 01 c9 31 d2 31 c9 31
> > > f6 31 ff
> > > [  118.410949] RSP: 0018:ffffccec8ea7bb18 EFLAGS: 00010246
> > > [  118.410957] RAX: 0000000000000000 RBX: 00007ff23812f460 RCX:
> > > ffff8c1790db4800
> > > [  118.410964] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
> > > 0000000000000000
> > > [  118.410968] RBP: ffffccec8ea7bb20 R08: 0000000000000000 R09:
> > > 0000000000000000
> > > [  118.410973] R10: 0000000000000000 R11: 0000000000000000 R12:
> > > ffff8c1790db4800
> > > [  118.410978] R13: 0000000000000000 R14: ffff8c178b67e000 R15:
> > > 0000000000000000
> > > [  118.410984] FS:  0000000101aff6c0(0000) GS:ffff8c27350d1000(0000)
> > > knlGS:000000007fe20000
> > > [  118.410992] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [  118.410998] CR2: 0000000000000030 CR3: 00000001037ef005 CR4:
> > > 0000000000772ef0
> > > [  118.411004] PKRU: 55555554
> > > [  118.411008] Call Trace:
> > > [  118.411013]  <TASK>
> > > [  118.411022]  xe_exec_ioctl+0x375/0xea0 [xe]
> > > [  118.411377]  ? __x64_sys_ioctl+0xbd/0x100
> > > [  118.411392]  ? do_syscall_64+0xa7/0x580
> > > [  118.411403]  ? dma_fence_free+0x1a/0x30
> > > [  118.411419]  ? __pfx_xe_exec_fn+0x10/0x10 [xe]
> > > [  118.411752]  ? drm_syncobj_array_free+0x56/0x80 [drm]
> > > [  118.411915]  ? drm_syncobj_query_ioctl+0x20f/0x460 [drm]
> > > [  118.412035]  ? __pfx_xe_exec_ioctl+0x10/0x10 [xe]
> > > [  118.412372]  drm_ioctl_kernel+0xae/0x110 [drm]
> > > [  118.412524]  drm_ioctl+0x2ee/0x5d0 [drm]
> > > [  118.412649]  ? __pfx_xe_exec_ioctl+0x10/0x10 [xe]
> > > [  118.412989]  ? __pm_runtime_resume+0x5f/0x90
> > > [  118.413002]  xe_drm_ioctl+0x61/0xb0 [xe]
> > > [  118.413329]  __x64_sys_ioctl+0xa3/0x100
> > > [  118.413338]  x64_sys_call+0x1060/0x2360
> > > [  118.413347]  do_syscall_64+0x74/0x580
> > > [  118.413355]  ? x64_sys_call+0x1060/0x2360
> > > [  118.413360]  ? do_syscall_64+0xa7/0x580
> > > [  118.413368]  ? __do_sys_getpid+0x1d/0x30
> > > [  118.413377]  ? x64_sys_call+0xf9d/0x2360
> > > [  118.413383]  ? do_syscall_64+0xa7/0x580
> > > [  118.413389]  ? x64_sys_call+0x1060/0x2360
> > > [  118.413395]  ? do_syscall_64+0xa7/0x580
> > > [  118.413401]  ? sysvec_apic_timer_interrupt+0x54/0xd0
> > > [  118.413410]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > > [  118.413418] RIP: 0033:0x7ff29c93287d
> > > 
> > > 
> > > 
> > > > > 
> > > > > >               num_in_sync++;
> > > > > >       }
> > > > > 
> > > > 
> >