Re: [PATCH] drm/xe: Fix UAF in xe_gem_prime_import() on attach failure

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Thu, 2026-06-04 at 12:32 +0100, Matthew Auld wrote:
> On 04/06/2026 12:14, Thomas Hellström wrote:
> > Hi,
> > 
> > On Thu, 2026-06-04 at 04:54 +0000, Gote, Nitin R wrote:
> > > Hi,
> > > 
> > > > -----Original Message-----
> > > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On
> > > > Behalf
> > > > Of Gote, Nitin
> > > > R
> > > > Sent: Monday, June 1, 2026 8:57 PM
> > > > To: Christian König <christian.koenig@amd.com>; Auld, Matthew
> > > > <matthew.auld@intel.com>; intel-xe@lists.freedesktop.org;
> > > > Christian
> > > > König
> > > > <ckoenig.leichtzumerken@gmail.com>
> > > > Cc: stable@vger.kernel.org; Thomas Hellstrom
> > > > <thomas.hellstrom@linux.intel.com>; Brost, Matthew
> > > > <matthew.brost@intel.com>; Prosyak, Vitaly
> > > > <Vitaly.Prosyak@amd.com>
> > > > Subject: RE: [PATCH] drm/xe: Fix UAF in xe_gem_prime_import()
> > > > on
> > > > attach failure
> > > > 
> > > > Hi Christian,
> > > > 
> > > > > -----Original Message-----
> > > > > From: Christian König <christian.koenig@amd.com>
> > > > > Sent: Monday, June 1, 2026 5:47 PM
> > > > > To: Auld, Matthew <matthew.auld@intel.com>; Gote, Nitin R
> > > > > <nitin.r.gote@intel.com>; intel-xe@lists.freedesktop.org;
> > > > > Christian
> > > > > König <ckoenig.leichtzumerken@gmail.com>
> > > > > Cc: stable@vger.kernel.org; Thomas Hellstrom
> > > > > <thomas.hellstrom@linux.intel.com>; Brost, Matthew
> > > > > <matthew.brost@intel.com>; Prosyak, Vitaly
> > > > > <Vitaly.Prosyak@amd.com>
> > > > > Subject: Re: [PATCH] drm/xe: Fix UAF in xe_gem_prime_import()
> > > > > on
> > > > > attach failure
> > > > > 
> > > > > On 6/1/26 14:01, Matthew Auld wrote:
> > > > > > On 01/06/2026 12:39, Christian König wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On 6/1/26 12:46, Matthew Auld wrote:
> > > > > > > > On 01/06/2026 11:15, Nitin Gote wrote:
> > > > > > > > > xe_dma_buf_create_obj() creates the importer BO with
> > > > > > > > > obj-
> > > > > > > > > > resv
> > > > > > > > > pointing at the exporter's dma_buf->resv. When
> > > > > > > > > dma_buf_dynamic_attach() fails, no dma_buf reference
> > > > > > > > > is
> > > > > > > > > held so
> > > > > > > > > the exporter can be freed immediately. Since
> > > > > > > > > ttm_bo_release() now
> > > > > > > > > always defers cleanup for ttm_bo_type_sg BOs to the
> > > > > > > > > TTM
> > > > > > > > > workqueue, the worker later calls
> > > > > > > > > dma_resv_lock() on the already-freed exporter resv,
> > > > > > > > > causing a UAF.
> > > > > > > > > 
> > > > > > > > > Reset obj->resv to the BO's private _resv before
> > > > > > > > > calling
> > > > > > > > > xe_bo_put() in the error path. The BO is not yet
> > > > > > > > > published
> > > > > > > > > (attach
> > > > > > > > > failed) and carries no fences, so the switch is safe.
> > > > > > > > > 
> > > > > > > > > Observed with igt@xe_live_ktest@xe_dma_buf_kunit on
> > > > > > > > > BMG
> > > > > > > > > (QEMU):
> > > > > > > > > 
> > > > > > > > >      Oops: general protection fault, probably for
> > > > > > > > > non-
> > > > > > > > > canonical
> > > > > > > > > address 0x6b6b6b6b6b6b6b9c
> > > > > > > > >      Workqueue: ttm ttm_bo_delayed_delete [ttm]
> > > > > > > > >      RIP: 0010:mutex_can_spin_on_owner+0x3f/0xc0
> > > > > > > > >      Call Trace:
> > > > > > > > >       <TASK>
> > > > > > > > >       ? __ww_mutex_lock.constprop.0+0x2dd/0x18e0
> > > > > > > > >       ? ttm_bo_delayed_delete+0x41/0xc0 [ttm]
> > > > > > > > >       ww_mutex_lock+0x3c/0xb0
> > > > > > > > >       ttm_bo_delayed_delete+0x41/0xc0 [ttm]
> > > > > > > > >       process_one_work+0x239/0x740
> > > > > > > > >       worker_thread+0x200/0x3f0
> > > > > > > > >       kthread+0x10d/0x150
> > > > > > > > >       ret_from_fork+0x3bd/0x470
> > > > > > > > >       ret_from_fork_asm+0x1a/0x30
> > > > > > > > >       </TASK>
> > > > > > > > > 
> > > > > > > > > Closes:
> > > > > > > > > https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8023
> > > > > > > > > Fixes: d99fbd9aab62 ("drm/ttm: Always take the bo
> > > > > > > > > delayed
> > > > > > > > > cleanup
> > > > > > > > > path for imported bos")
> > > > > > > > > Cc: stable@vger.kernel.org # v6.8+
> > > > > > > > > Cc: Thomas Hellstrom
> > > > > > > > > <thomas.hellstrom@linux.intel.com>
> > > > > > > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > > > > > > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > > > > > > > Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>
> > > > > > > > > ---
> > > > > > > > >     drivers/gpu/drm/xe/xe_dma_buf.c | 8 ++++++++
> > > > > > > > >     1 file changed, 8 insertions(+)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > > > > > > > b/drivers/gpu/drm/xe/xe_dma_buf.c index
> > > > > > > > > 8a920e58245c..6d944bd4065c
> > > > > > > > > 100644
> > > > > > > > > --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > > > > > > > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> > > > > > > > > @@ -384,6 +384,14 @@ struct drm_gem_object
> > > > > > > > > *xe_gem_prime_import(struct drm_device *dev,
> > > > > > > > >           attach = dma_buf_dynamic_attach(dma_buf,
> > > > > > > > > dev-
> > > > > > > > > > dev,
> > > > > > > > > attach_ops, obj);
> > > > > > > > >         if (IS_ERR(attach)) {
> > > > > > > > > +        /*
> > > > > > > > > +         * The BO was created with resv = dma_buf-
> > > > > > > > > >resv
> > > > > > > > > +(exporter's
> > > > > > > > > +         * resv). Since attach failed, no dma_buf
> > > > > > > > > reference is
> > > > > > > > > +held and
> > > > > > > > > +         * the exporter may be freed before TTM's
> > > > > > > > > delayed_delete
> > > > > > > > > +worker
> > > > > > > > > +         * runs. Switch to the BO's own resv to
> > > > > > > > > prevent
> > > > > > > > > a UAF
> > > > > > > > > +when
> > > > > > > > > +         * ttm_bo_delayed_delete() tries to lock the
> > > > > > > > > stale pointer.
> > > > > > > > > +         */
> > > > > > > > > +        obj->resv = &obj->_resv;
> > > > > > > > 
> > > > > > > > +Christian, does amdgpu not have the type of same issue
> > > > > > > > here? Also
> > > > > > > > +any
> > > > > thoughts here?
> > > > > > > 
> > > > > > > Oh, good catch. Yeah I think we have the same problem on
> > > > > > > amdgpu as well.
> > > > > > 
> > > > > > Maybe dumb question, but why does the
> > > > > > ttm_bo_individualize_resv()
> > > > > > skip the
> > > > > final switch of the resv for type_sg?
> > > > > 
> > > > > Because we need the original resv object for cleaning up the
> > > > > mapping
> > > > > should the initial attach and then map have succeed.
> > > > > 
> > > > > > It goes through the trouble of copying the fences across?
> > > > > 
> > > > > Because we need to know when the import can be cleaned up.
> > > > > 
> > > > > In other words TTM takes a copy of the current fences and
> > > > > only
> > > > > unmap,
> > > > > detach and then do the final cleanup after we are sure that
> > > > > the
> > > > > set of
> > > > > fences which was active on destruction is now signaled.
> > > > > 
> > > > > If new fences are added to the resv object (maybe by the
> > > > > exporter
> > > > > itself or other
> > > > > importers) after our reference count got down to zero then we
> > > > > don't
> > > > > care about that.
> > > > > > If we do need to handle this here, do we also need to grab
> > > > > > the
> > > > > > lru
> > > > > > lock, like we
> > > > > do in ttm_bo_individualize_resv() when doing the swap?
> > > > > 
> > > > > Good question, of hand I would say yes but I clearly need to
> > > > > check the
> > > > > source code as well.
> > > > > 
> > > > > Might be better to switch the type of the BO on error so that
> > > > > the
> > > > > normal cleanup will just switch over to the local dma_resv
> > > > > object.
> > > > > 
> > > > 
> > > > -               obj->resv = &obj->_resv;
> > > > +               gem_to_xe_bo(obj)->ttm.type =
> > > > ttm_bo_type_kernel;
> > > > 
> > > > Switching the type to ttm_bo_type_kernel lets
> > > > ttm_bo_individualize_resv() swap
> > > > resv to the BO's private _resv under lru_lock, which prevents
> > > > UAF
> > > > without
> > > > needing any manual locking.
> > 
> > The lru lock is IIRC only needed and safe when the ttm refcount is
> > zero
> > (in the TTM destruction path) to protect against a racing LRU walk
> > trylock succeeds against the incorrect resv.
> > 
> > I wonder whether this was actually why xe code initially took care
> > not
> > to publish the bo on the LRUs until the attachment succeeded.
> > 
> > A TTM LRU walker may pick up the exporting resv as soon as the
> > resource
> > is published on the LRU, and then try to lock it using
> > ttm_lru_walk_ticketlock(). The lru lock doesn't protect against
> > that.
> >   
> > So we have a sort of moment22, since with that approach
> > move_notify()
> > could be called without the bo being fully initialized.
> > 
> > One way to move forward would perhaps be to, for now, reinstate
> > that
> > and have move_notify check if the bo is a stub or fully initialized
> > before doing anything.
> > 
> > Also perhaps we should in the future consider allowing dma-buf
> > attachment removal under a separate lower-level lock than the resv.
> 
> Is it plausible to check for drm_gem_is_imported() in 
> ttm_bo_individualize_resv()? If sg && !imported then it should be
> safe 
> to swap out the resv?

That'd require we never dropped the exporter refcount until the
individualization happens. Hard to assert, and would be fragile. Also
not completely sure on the stance currently WRT gem and ttm layer code
separation, but that OTH is fixable.

/Thomas


> 
> > 
> > Thanks,
> > Thomas
> > 
> > 
> > > 
> > > Checked all bo->type readers (xe_evict_flags(), xe_bo_move(),
> > > xe_bo_can_migrate()) and found they can be called concurrently by
> > > the
> > > shrinker or eviction paths without any synchronization, making
> > > the
> > > bo->type change unsafe.
> > > 
> > > Switching resv to &obj->_resv under lru_lock, mirroring
> > > ttm_bo_individualize_resv(), is the more reasonable.
> > > I'll send this as v2, along with a separate patch fixing the same
> > > issue in amdgpu.
> > > 
> > > - Nitin
> > > 
> > > > > Since we don't need the original dma_resv for the cleanup
> > > > > that
> > > > > should work
> > > > fine.
> > > > > 
> > > > > > Ideally xe and amdgpu can just have identical solutions
> > > > > > here.
> > > > > 
> > > > > Yeah completely agree.
> > > > > 
> > > > > Regards,
> > > > > Christian.
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > How the heck did you found that? Do we have a dummy
> > > > > > > driver
> > > > > > > (VGEM?)
> > > > > > > which
> > > > > could be made to always fail attachment for a test case?
> > > > 
> > > > The bug was found via the existing KUnit test
> > > > (xe_dma_buf_kunit),
> > > > which was
> > > > failing on a BMG VM device. The test runs 20 parameter
> > > > combinations.
> > > > the failing ones use force_different_devices=true +
> > > > mem_mask=XE_BO_FLAG_VRAM0 + nop2p_attach_ops, where
> > > > dma_buf_dynamic_attach() returns -EOPNOTSUPP, hitting the error
> > > > path.
> > > > 
> > > > On bare metal BMG the race window is too narrow to hit the
> > > > issue.
> > > > To make it
> > > > more deterministic, added a small msleep(100) in
> > > > ttm_bo_delayed_delete() just
> > > > before the dma_resv_lock() call, which widened the race window.
> > > > With KASAN enabled, that gave a clear slab-use-after-free in
> > > > __ww_mutex_lock
> > > > — the 0x6b6b6b6b SLUB poison pattern in the faulting address
> > > > confirmed the
> > > > UAF.
> > > > 
> > > > Thanks,
> > > > Nitin
> > > > 
> > > > > > > 
> > > > > > > @Vitaly can you take a look and try to come up with a
> > > > > > > test
> > > > > > > case for that?
> > > > > Thanks in advance.
> > > > > > > 
> > > > > > > Thanks for the notice,
> > > > > > > Christian.
> > > > > > > 
> > > > > > > > 
> > > > > > > > >             xe_bo_put(gem_to_xe_bo(obj));
> > > > > > > > >             return ERR_CAST(attach);
> > > > > > > > >         }
> > > > > > > > 
> > > > > > > 
> > > > > >