Re: [PATCH] drm/xe: Hold a dma-buf reference for imported BOs

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

On Fri, 2026-07-10 at 15:07 +0530, Nitin Gote wrote:
> An imported dma-buf BO is created as a ttm_bo_type_sg BO whose
> reservation object is the exporter's dma_buf->resv. The importer,
> however, only takes a dma-buf reference after a successful
> dma_buf_dynamic_attach(). Until then nothing keeps the exporter
> alive,
> so if the exporter is freed while the BO still references its resv, a
> later access to that resv is a use-after-free:
> 
>   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
> 
> This can be reached on two paths:
> 
>  - dma_buf_dynamic_attach() fails, or
>  - ttm_bo_init_reserved() fails during BO creation.
> 
> In both cases the BO already has bo->base.resv pointing at the
> exporter
> resv, and sg BOs are always torn down via ttm_bo_delayed_delete(),
> which
> locks bo->base.resv asynchronously - potentially after the exporter
> has
> been freed.
> 
> Take the dma-buf reference in xe_bo_init_locked(), before
> ttm_bo_init_reserved(), so it also covers a creation failure there,
> and
> release it in xe_ttm_bo_destroy(). The reference is held for the
> whole
> BO lifetime, keeping the shared resv alive on every path.
> 
> Closes:
> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8023
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
> GPUs")
> Cc: stable@vger.kernel.org
> Cc: Thomas Hellstrom <thomas.hellstrom@linux.intel.com>
> Cc: Christian Konig <christian.koenig@amd.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Suggested-by: Thomas Hellstrom <thomas.hellstrom@linux.intel.com>
> Assisted-by: GitHub_Copilot:claude-sonnet-4.6
> Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>
> ---
> Thank you Thomas for suggesting this simpler approach over the
> earlier
> TTM/LRU handling.
> 
> v6:
>  - Reworked the fix based on Thomas' suggestion. Instead of the TTM
> resv
>    individualization (v1-v5) plus the xe off-LRU/placement handling
> (v5),
>    just hold a dma-buf reference for the imported BO lifetime so the
>    shared resv can never be freed while the BO still references it.
>    Single xe patch, no TTM change. (Thomas)
>  - Take the reference in xe_bo_init_locked() before
> ttm_bo_init_reserved()
>    so a TTM creation failure is covered too (Thomas).
>  - Dropped the v5 series (drm/ttm + drm/xe off-LRU); the off-LRU
> approach
>    also regressed in CI BAT via ttm_bo_pipeline_gutting() creating a
> ghost
>    BO that outlived the exporter.
>    Link to v5: https://patchwork.freedesktop.org/series/169984/
>    
> v5:
>  - Add drm/xe patch to keep imported sg BOs off the LRU before attach
>    succeeds; the TTM fix alone is not sufficient for xe if the BO is
>    already LRU-visible. (Thomas)
>    v4 patch:
>   
> https://patchwork.freedesktop.org/patch/736663/?series=169129&rev=2
>   - Patch 1 (drm/ttm) carries Christian's Reviewed-by from v4.
> 
> v4:
>  - Moved import_attach check to after dma_resv_copy_fences() so
> fences
>    are copied before returning for successful imports (Thomas).
>  - Removed exporter-alive claim from commit message (Thomas).
> 
> v3:
>  - Dropped the xe-side reordering approach since importer_priv must
> be
>    valid when dma_buf_dynamic_attach() publishes the attachment.
>  - Per Christian's suggestion on the v1 thread, keyed the check on
>    import_attach rather than removing the sg guard entirely.
>  - Fixes both xe and amdgpu in a single TTM patch.

We typically place the changelog above the ___ for xe, so it is kept in
the commit message. And in the reverse order so v3 comes first.

With that fixed,
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> 
> 
>  drivers/gpu/drm/xe/xe_bo.c       | 24 ++++++++++++++++++++----
>  drivers/gpu/drm/xe/xe_bo.h       |  3 ++-
>  drivers/gpu/drm/xe/xe_bo_types.h |  2 ++
>  drivers/gpu/drm/xe/xe_dma_buf.c  |  2 +-
>  4 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 85e6d9a0f575..ae730bd6f4b2 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1349,7 +1349,7 @@ int xe_bo_notifier_prepare_pinned(struct xe_bo
> *bo)
>  		backup = xe_bo_init_locked(xe, NULL, NULL, bo-
> >ttm.base.resv, NULL, xe_bo_size(bo),
>  					  
> DRM_XE_GEM_CPU_CACHING_WB, ttm_bo_type_kernel,
>  					   XE_BO_FLAG_SYSTEM |
> XE_BO_FLAG_NEEDS_CPU_ACCESS |
> -					   XE_BO_FLAG_PINNED,
> &exec);
> +					   XE_BO_FLAG_PINNED, NULL,
> &exec);
>  		if (IS_ERR(backup)) {
>  			drm_exec_retry_on_contention(&exec);
>  			ret = PTR_ERR(backup);
> @@ -1490,7 +1490,7 @@ int xe_bo_evict_pinned(struct xe_bo *bo)
>  						   xe_bo_size(bo),
>  						  
> DRM_XE_GEM_CPU_CACHING_WB, ttm_bo_type_kernel,
>  						   XE_BO_FLAG_SYSTEM
> | XE_BO_FLAG_NEEDS_CPU_ACCESS |
> -						  
> XE_BO_FLAG_PINNED, &exec);
> +						  
> XE_BO_FLAG_PINNED, NULL, &exec);
>  			if (IS_ERR(backup)) {
>  				drm_exec_retry_on_contention(&exec);
>  				ret = PTR_ERR(backup);
> @@ -1826,6 +1826,8 @@ static void xe_ttm_bo_destroy(struct
> ttm_buffer_object *ttm_bo)
>  
>  	if (bo->ttm.base.import_attach)
>  		drm_prime_gem_destroy(&bo->ttm.base, NULL);
> +	if (bo->dma_buf)
> +		dma_buf_put(bo->dma_buf);
>  	drm_gem_object_release(&bo->ttm.base);
>  
>  	xe_assert(xe, list_empty(&ttm_bo->base.gpuva.list));
> @@ -2283,6 +2285,8 @@ void xe_bo_free(struct xe_bo *bo)
>   * @cpu_caching: The cpu caching used for system memory backing
> store.
>   * @type: The TTM buffer object type.
>   * @flags: XE_BO_FLAG_ flags.
> + * @dma_buf: The dma-buf to reference for the BO lifetime (imported
> BOs),
> + * or NULL.
>   * @exec: The drm_exec transaction to use for exhaustive eviction.
>   *
>   * Initialize or create an xe buffer object. On failure, any
> allocated buffer
> @@ -2294,7 +2298,8 @@ struct xe_bo *xe_bo_init_locked(struct
> xe_device *xe, struct xe_bo *bo,
>  				struct xe_tile *tile, struct
> dma_resv *resv,
>  				struct ttm_lru_bulk_move *bulk,
> size_t size,
>  				u16 cpu_caching, enum ttm_bo_type
> type,
> -				u32 flags, struct drm_exec *exec)
> +				u32 flags, struct dma_buf *dma_buf,
> +				struct drm_exec *exec)
>  {
>  	struct ttm_operation_ctx ctx = {
>  		.interruptible = true,
> @@ -2383,6 +2388,17 @@ struct xe_bo *xe_bo_init_locked(struct
> xe_device *xe, struct xe_bo *bo,
>  	placement = (type == ttm_bo_type_sg ||
>  		     bo->flags & XE_BO_FLAG_DEFER_BACKING) ?
> &sys_placement :
>  		&bo->placement;
> +
> +	/*
> +	 * For imported BOs, keep the exporter dma-buf alive for the
> BO
> +	 * lifetime. Taken before ttm_bo_init_reserved() to also
> cover a
> +	 * creation failure there. Released in xe_ttm_bo_destroy().
> +	 */
> +	if (dma_buf) {
> +		get_dma_buf(dma_buf);
> +		bo->dma_buf = dma_buf;
> +	}
> +
>  	err = ttm_bo_init_reserved(&xe->ttm, &bo->ttm, type,
>  				   placement, alignment,
>  				   &ctx, NULL, resv,
> xe_ttm_bo_destroy);
> @@ -2500,7 +2516,7 @@ __xe_bo_create_locked(struct xe_device *xe,
>  			       vm && !xe_vm_in_fault_mode(vm) &&
>  			       flags & XE_BO_FLAG_USER ?
>  			       &vm->lru_bulk_move : NULL, size,
> -			       cpu_caching, type, flags, exec);
> +			       cpu_caching, type, flags, NULL,
> exec);
>  	if (IS_ERR(bo))
>  		return bo;
>  
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 6340317f7d2e..7ae1d9ac0574 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -118,7 +118,8 @@ struct xe_bo *xe_bo_init_locked(struct xe_device
> *xe, struct xe_bo *bo,
>  				struct xe_tile *tile, struct
> dma_resv *resv,
>  				struct ttm_lru_bulk_move *bulk,
> size_t size,
>  				u16 cpu_caching, enum ttm_bo_type
> type,
> -				u32 flags, struct drm_exec *exec);
> +				u32 flags, struct dma_buf *dma_buf,
> +				struct drm_exec *exec);
>  struct xe_bo *xe_bo_create_locked(struct xe_device *xe, struct
> xe_tile *tile,
>  				  struct xe_vm *vm, size_t size,
>  				  enum ttm_bo_type type, u32 flags,
> diff --git a/drivers/gpu/drm/xe/xe_bo_types.h
> b/drivers/gpu/drm/xe/xe_bo_types.h
> index fcc63ae3f455..e45f24301050 100644
> --- a/drivers/gpu/drm/xe/xe_bo_types.h
> +++ b/drivers/gpu/drm/xe/xe_bo_types.h
> @@ -36,6 +36,8 @@ struct xe_bo {
>  	struct xe_bo *backup_obj;
>  	/** @parent_obj: Ref to parent bo if this a backup_obj */
>  	struct xe_bo *parent_obj;
> +	/** @dma_buf: Imported dma-buf ref to keep its resv alive.
> */
> +	struct dma_buf *dma_buf;
>  	/** @flags: flags for this buffer object */
>  	u32 flags;
>  	/** @vm: VM this BO is attached to, for extobj this will be
> NULL */
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> b/drivers/gpu/drm/xe/xe_dma_buf.c
> index 8a920e58245c..bf0728838ead 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -302,7 +302,7 @@ xe_dma_buf_create_obj(struct drm_device *dev,
> struct dma_buf *dma_buf)
>  
>  		bo = xe_bo_init_locked(xe, NULL, NULL, resv, NULL,
> dma_buf->size,
>  				       0, /* Will require 1way or
> 2way for vm_bind */
> -				       ttm_bo_type_sg,
> XE_BO_FLAG_SYSTEM, &exec);
> +				       ttm_bo_type_sg,
> XE_BO_FLAG_SYSTEM, dma_buf, &exec);
>  		drm_exec_retry_on_contention(&exec);
>  		if (IS_ERR(bo)) {
>  			ret = PTR_ERR(bo);