[PATCH 1/4] drm/xe/bo: Fix bo leak on unaligned size validation in xe_bo_init_locked()

[PATCH 1/4] drm/xe/bo: Fix bo leak on unaligned size validation in xe_bo_init_locked()

[Shuicheng Lin]

When type is ttm_bo_type_device and aligned_size != size, the function
returns an error without freeing a caller-provided bo, violating the
documented contract that bo is freed on failure.

Add xe_bo_free(bo) before returning the error.

Fixes: 4e03b584143e ("drm/xe/uapi: Reject bo creation of unaligned size")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
---
 drivers/gpu/drm/xe/xe_bo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index b70e8396e56f..6e4ebbe72952 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -2342,8 +2342,10 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo,
 		alignment = SZ_4K >> PAGE_SHIFT;
 	}
 
-	if (type == ttm_bo_type_device && aligned_size != size)
+	if (type == ttm_bo_type_device && aligned_size != size) {
+		xe_bo_free(bo);
 		return ERR_PTR(-EINVAL);
+	}
 
 	if (!bo) {
 		bo = xe_bo_alloc();
-- 
2.43.0

[PATCH 2/4] drm/xe/bo: Fix bo leak on GGTT flag validation in xe_bo_init_locked()

[Shuicheng Lin]

When XE_BO_FLAG_GGTT_ALL is set without XE_BO_FLAG_GGTT, the function
returns an error without freeing a caller-provided bo, violating the
documented contract that bo is freed on failure.

Add xe_bo_free(bo) before returning the error.

Fixes: 5a3b0df25d6a ("drm/xe: Allow bo mapping on multiple ggtts")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
---
 drivers/gpu/drm/xe/xe_bo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index 6e4ebbe72952..d09e96b996b9 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -2322,8 +2322,10 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo,
 	}
 
 	/* XE_BO_FLAG_GGTTx requires XE_BO_FLAG_GGTT also be set */
-	if ((flags & XE_BO_FLAG_GGTT_ALL) && !(flags & XE_BO_FLAG_GGTT))
+	if ((flags & XE_BO_FLAG_GGTT_ALL) && !(flags & XE_BO_FLAG_GGTT)) {
+		xe_bo_free(bo);
 		return ERR_PTR(-EINVAL);
+	}
 
 	if (flags & (XE_BO_FLAG_VRAM_MASK | XE_BO_FLAG_STOLEN) &&
 	    !(flags & XE_BO_FLAG_IGNORE_MIN_PAGE_SIZE) &&
-- 
2.43.0

[PATCH 3/4] drm/xe: Fix bo leak in xe_dma_buf_init_obj() on allocation failure

[Shuicheng Lin]

When drm_gpuvm_resv_object_alloc() fails, the pre-allocated storage bo
is not freed. Add xe_bo_free(storage) before returning the error.

Fixes: eb289a5f6cc6 ("drm/xe: Convert xe_dma_buf.c for exhaustive eviction")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
---
 drivers/gpu/drm/xe/xe_dma_buf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
index 7f9602b3363d..24d9d82426b9 100644
--- a/drivers/gpu/drm/xe/xe_dma_buf.c
+++ b/drivers/gpu/drm/xe/xe_dma_buf.c
@@ -271,8 +271,10 @@ xe_dma_buf_init_obj(struct drm_device *dev, struct xe_bo *storage,
 	int ret = 0;
 
 	dummy_obj = drm_gpuvm_resv_object_alloc(&xe->drm);
-	if (!dummy_obj)
+	if (!dummy_obj) {
+		xe_bo_free(storage);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	dummy_obj->resv = resv;
 	xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {}, ret) {
-- 
2.43.0

[PATCH 4/4] drm/xe: Fix dma-buf attachment leak in xe_gem_prime_import()

[Shuicheng Lin]

When xe_dma_buf_init_obj() fails, the attachment from
dma_buf_dynamic_attach() is not detached. Add dma_buf_detach() before
returning the error. Note: we cannot use goto out_err here because
xe_dma_buf_init_obj() already frees bo on failure, and out_err would
double-free it.

Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
---
 drivers/gpu/drm/xe/xe_dma_buf.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
index 24d9d82426b9..7702a6bdaae5 100644
--- a/drivers/gpu/drm/xe/xe_dma_buf.c
+++ b/drivers/gpu/drm/xe/xe_dma_buf.c
@@ -370,12 +370,15 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
 		goto out_err;
 	}
 
-	/* Errors here will take care of freeing the bo. */
+	/*
+	 * xe_dma_buf_init_obj() takes ownership of bo on both success
+	 * and failure, so we must not touch bo after this call.
+	 */
 	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
-	if (IS_ERR(obj))
+	if (IS_ERR(obj)) {
+		dma_buf_detach(dma_buf, attach);
 		return obj;
-
-
+	}
 	get_dma_buf(dma_buf);
 	obj->import_attach = attach;
 	return obj;
-- 
2.43.0

Re: [PATCH 1/4] drm/xe/bo: Fix bo leak on unaligned size validation in xe_bo_init_locked()

[Matthew Brost]

On Tue, Apr 07, 2026 at 08:15:39PM +0000, Shuicheng Lin wrote:
> When type is ttm_bo_type_device and aligned_size != size, the function
> returns an error without freeing a caller-provided bo, violating the
> documented contract that bo is freed on failure.
> 
> Add xe_bo_free(bo) before returning the error.
> 
> Fixes: 4e03b584143e ("drm/xe/uapi: Reject bo creation of unaligned size")
> Cc: stable@vger.kernel.org

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

> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index b70e8396e56f..6e4ebbe72952 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2342,8 +2342,10 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo,
>  		alignment = SZ_4K >> PAGE_SHIFT;
>  	}
>  
> -	if (type == ttm_bo_type_device && aligned_size != size)
> +	if (type == ttm_bo_type_device && aligned_size != size) {
> +		xe_bo_free(bo);
>  		return ERR_PTR(-EINVAL);
> +	}
>  
>  	if (!bo) {
>  		bo = xe_bo_alloc();
> -- 
> 2.43.0
> 

Re: [PATCH 2/4] drm/xe/bo: Fix bo leak on GGTT flag validation in xe_bo_init_locked()

[Matthew Brost]

On Tue, Apr 07, 2026 at 08:15:40PM +0000, Shuicheng Lin wrote:
> When XE_BO_FLAG_GGTT_ALL is set without XE_BO_FLAG_GGTT, the function
> returns an error without freeing a caller-provided bo, violating the
> documented contract that bo is freed on failure.
> 
> Add xe_bo_free(bo) before returning the error.
> 
> Fixes: 5a3b0df25d6a ("drm/xe: Allow bo mapping on multiple ggtts")
> Cc: stable@vger.kernel.org

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

> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 6e4ebbe72952..d09e96b996b9 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2322,8 +2322,10 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo,
>  	}
>  
>  	/* XE_BO_FLAG_GGTTx requires XE_BO_FLAG_GGTT also be set */
> -	if ((flags & XE_BO_FLAG_GGTT_ALL) && !(flags & XE_BO_FLAG_GGTT))
> +	if ((flags & XE_BO_FLAG_GGTT_ALL) && !(flags & XE_BO_FLAG_GGTT)) {
> +		xe_bo_free(bo);
>  		return ERR_PTR(-EINVAL);
> +	}
>  
>  	if (flags & (XE_BO_FLAG_VRAM_MASK | XE_BO_FLAG_STOLEN) &&
>  	    !(flags & XE_BO_FLAG_IGNORE_MIN_PAGE_SIZE) &&
> -- 
> 2.43.0
> 

Re: [PATCH 3/4] drm/xe: Fix bo leak in xe_dma_buf_init_obj() on allocation failure

[Matthew Brost]

On Tue, Apr 07, 2026 at 08:15:41PM +0000, Shuicheng Lin wrote:
> When drm_gpuvm_resv_object_alloc() fails, the pre-allocated storage bo
> is not freed. Add xe_bo_free(storage) before returning the error.
> 
> Fixes: eb289a5f6cc6 ("drm/xe: Convert xe_dma_buf.c for exhaustive eviction")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_dma_buf.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
> index 7f9602b3363d..24d9d82426b9 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -271,8 +271,10 @@ xe_dma_buf_init_obj(struct drm_device *dev, struct xe_bo *storage,
>  	int ret = 0;
>  
>  	dummy_obj = drm_gpuvm_resv_object_alloc(&xe->drm);
> -	if (!dummy_obj)
> +	if (!dummy_obj) {

I know the comment at caller says 'Errors here will take care of freeing the bo.'

But I'm not sure that is right sematic as this patch alone won't free
the BO give this line not seen in this diff:

296         return ret ? ERR_PTR(ret) : &bo->ttm.base;

So IMO we make the caller own the freeing of the BO here.

Matt

> +		xe_bo_free(storage);
>  		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	dummy_obj->resv = resv;
>  	xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {}, ret) {
> -- 
> 2.43.0
> 

Re: [PATCH 4/4] drm/xe: Fix dma-buf attachment leak in xe_gem_prime_import()

[Matthew Brost]

On Tue, Apr 07, 2026 at 08:15:42PM +0000, Shuicheng Lin wrote:
> When xe_dma_buf_init_obj() fails, the attachment from
> dma_buf_dynamic_attach() is not detached. Add dma_buf_detach() before
> returning the error. Note: we cannot use goto out_err here because
> xe_dma_buf_init_obj() already frees bo on failure, and out_err would
> double-free it.
> 
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_dma_buf.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
> index 24d9d82426b9..7702a6bdaae5 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -370,12 +370,15 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
>  		goto out_err;
>  	}
>  
> -	/* Errors here will take care of freeing the bo. */
> +	/*
> +	 * xe_dma_buf_init_obj() takes ownership of bo on both success
> +	 * and failure, so we must not touch bo after this call.
> +	 */
>  	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> -	if (IS_ERR(obj))
> +	if (IS_ERR(obj)) {
> +		dma_buf_detach(dma_buf, attach);

Based on my feedback from the previous patch [1], I think we also want...

		xe_bo_free(bo);

Also unseen in this diff is this code:

365         attach = dma_buf_dynamic_attach(dma_buf, dev->dev, attach_ops, &bo->ttm.base);
366         if (IS_ERR(attach)) {
367                 obj = ERR_CAST(attach);
368                 goto out_err;
369         }

We also need a xe_bo_free(bo) in this failures if statement.

Matt

[1] https://patchwork.freedesktop.org/patch/716820/?series=164476&rev=1#comment_1319810

>  		return obj;
> -
> -
> +	}
>  	get_dma_buf(dma_buf);
>  	obj->import_attach = attach;
>  	return obj;
> -- 
> 2.43.0
> 

RE: [PATCH 3/4] drm/xe: Fix bo leak in xe_dma_buf_init_obj() on allocation failure

[Lin, Shuicheng]

On Tue, Apr 7, 2026 10:01 PM Matthew Brost wrote:
> On Tue, Apr 07, 2026 at 08:15:41PM +0000, Shuicheng Lin wrote:
> > When drm_gpuvm_resv_object_alloc() fails, the pre-allocated storage bo
> > is not freed. Add xe_bo_free(storage) before returning the error.
> >
> > Fixes: eb289a5f6cc6 ("drm/xe: Convert xe_dma_buf.c for exhaustive
> > eviction")
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-4.6
> > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_dma_buf.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> > b/drivers/gpu/drm/xe/xe_dma_buf.c index 7f9602b3363d..24d9d82426b9
> > 100644
> > --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> > @@ -271,8 +271,10 @@ xe_dma_buf_init_obj(struct drm_device *dev, struct
> xe_bo *storage,
> >  	int ret = 0;
> >
> >  	dummy_obj = drm_gpuvm_resv_object_alloc(&xe->drm);
> > -	if (!dummy_obj)
> > +	if (!dummy_obj) {
> 
> I know the comment at caller says 'Errors here will take care of freeing the bo.'
> 
> But I'm not sure that is right sematic as this patch alone won't free the BO give
> this line not seen in this diff:
> 
> 296         return ret ? ERR_PTR(ret) : &bo->ttm.base;
> 
> So IMO we make the caller own the freeing of the BO here.

xe_dma_buf_init_obj() calls xe_bo_init_locked(), which frees the BO on error.
Therefore, xe_dma_buf_init_obj() must also free the BO on its error paths.
Otherwise, since xe_gem_prime_import() cannot distinguish whether the failure originated from xe_dma_buf_init_obj() or from xe_bo_init_locked(), it cannot safely decide whether the BO should be freed.

On success, ownership of the BO is transferred to the drm_gem_object.

How about add some comments in this function like below?

+/*
+ * Takes ownership of @storage: on success it is transferred to the returned
+ * drm_gem_object; on failure it is freed before returning the error.
+ * This matches the contract of xe_bo_init_locked() which frees @storage on
+ * its error paths, so callers need not (and must not) free @storage after
+ * this call.
+ */
 static struct drm_gem_object *
 xe_dma_buf_init_obj(struct drm_device *dev, struct xe_bo *storage,
                    struct dma_buf *dma_buf)

Shuicheng

> 
> Matt
> 
> > +		xe_bo_free(storage);
> >  		return ERR_PTR(-ENOMEM);
> > +	}
> >
> >  	dummy_obj->resv = resv;
> >  	xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {},
> > ret) {
> > --
> > 2.43.0
> >

Re: [PATCH 3/4] drm/xe: Fix bo leak in xe_dma_buf_init_obj() on allocation failure

[Matthew Brost]

On Wed, Apr 08, 2026 at 09:58:06AM -0600, Lin, Shuicheng wrote:
> On Tue, Apr 7, 2026 10:01 PM Matthew Brost wrote:
> > On Tue, Apr 07, 2026 at 08:15:41PM +0000, Shuicheng Lin wrote:
> > > When drm_gpuvm_resv_object_alloc() fails, the pre-allocated storage bo
> > > is not freed. Add xe_bo_free(storage) before returning the error.
> > >
> > > Fixes: eb289a5f6cc6 ("drm/xe: Convert xe_dma_buf.c for exhaustive
> > > eviction")
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: Claude:claude-opus-4.6
> > > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_dma_buf.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > b/drivers/gpu/drm/xe/xe_dma_buf.c index 7f9602b3363d..24d9d82426b9
> > > 100644
> > > --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> > > @@ -271,8 +271,10 @@ xe_dma_buf_init_obj(struct drm_device *dev, struct
> > xe_bo *storage,
> > >  	int ret = 0;
> > >
> > >  	dummy_obj = drm_gpuvm_resv_object_alloc(&xe->drm);
> > > -	if (!dummy_obj)
> > > +	if (!dummy_obj) {
> > 
> > I know the comment at caller says 'Errors here will take care of freeing the bo.'
> > 
> > But I'm not sure that is right sematic as this patch alone won't free the BO give
> > this line not seen in this diff:
> > 
> > 296         return ret ? ERR_PTR(ret) : &bo->ttm.base;
> > 
> > So IMO we make the caller own the freeing of the BO here.
> 
> xe_dma_buf_init_obj() calls xe_bo_init_locked(), which frees the BO on error.
> Therefore, xe_dma_buf_init_obj() must also free the BO on its error paths.

Yes, right. It is easy to forget these consuming interfaces on error.

> Otherwise, since xe_gem_prime_import() cannot distinguish whether the failure originated from xe_dma_buf_init_obj() or from xe_bo_init_locked(), it cannot safely decide whether the BO should be freed.
> 
> On success, ownership of the BO is transferred to the drm_gem_object.
> 
> How about add some comments in this function like below?
> 
> +/*
> + * Takes ownership of @storage: on success it is transferred to the returned
> + * drm_gem_object; on failure it is freed before returning the error.
> + * This matches the contract of xe_bo_init_locked() which frees @storage on
> + * its error paths, so callers need not (and must not) free @storage after
> + * this call.
> + */

Yes, that's good to avoid forgetting.

So this patch looks correct:

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

>  static struct drm_gem_object *
>  xe_dma_buf_init_obj(struct drm_device *dev, struct xe_bo *storage,
>                     struct dma_buf *dma_buf)
> 
> Shuicheng
> 
> > 
> > Matt
> > 
> > > +		xe_bo_free(storage);
> > >  		return ERR_PTR(-ENOMEM);
> > > +	}
> > >
> > >  	dummy_obj->resv = resv;
> > >  	xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {},
> > > ret) {
> > > --
> > > 2.43.0
> > >

RE: [PATCH 4/4] drm/xe: Fix dma-buf attachment leak in xe_gem_prime_import()

[Lin, Shuicheng]

On Tue, Apr 7, 2026 10:05 PM Matthew Brost wrote:
> On Tue, Apr 07, 2026 at 08:15:42PM +0000, Shuicheng Lin wrote:
> > When xe_dma_buf_init_obj() fails, the attachment from
> > dma_buf_dynamic_attach() is not detached. Add dma_buf_detach() before
> > returning the error. Note: we cannot use goto out_err here because
> > xe_dma_buf_init_obj() already frees bo on failure, and out_err would
> > double-free it.
> >
> > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
> > GPUs")
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-4.6
> > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_dma_buf.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> > b/drivers/gpu/drm/xe/xe_dma_buf.c index 24d9d82426b9..7702a6bdaae5
> > 100644
> > --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> > @@ -370,12 +370,15 @@ struct drm_gem_object
> *xe_gem_prime_import(struct drm_device *dev,
> >  		goto out_err;
> >  	}
> >
> > -	/* Errors here will take care of freeing the bo. */
> > +	/*
> > +	 * xe_dma_buf_init_obj() takes ownership of bo on both success
> > +	 * and failure, so we must not touch bo after this call.
> > +	 */
> >  	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> > -	if (IS_ERR(obj))
> > +	if (IS_ERR(obj)) {
> > +		dma_buf_detach(dma_buf, attach);
> 
> Based on my feedback from the previous patch [1], I think we also want...
> 
> 		xe_bo_free(bo);
> 
> Also unseen in this diff is this code:
> 
> 365         attach = dma_buf_dynamic_attach(dma_buf, dev->dev, attach_ops,
> &bo->ttm.base);
> 366         if (IS_ERR(attach)) {
> 367                 obj = ERR_CAST(attach);
> 368                 goto out_err;
> 369         }
> 
> We also need a xe_bo_free(bo) in this failures if statement.
> 
> Matt
> 
> [1]
> https://patchwork.freedesktop.org/patch/716820/?series=164476&rev=1#c
> omment_1319810
> 

As discussed in another email, could you please help me	review this patch again?
Thanks.

Shuicheng

> >  		return obj;
> > -
> > -
> > +	}
> >  	get_dma_buf(dma_buf);
> >  	obj->import_attach = attach;
> >  	return obj;
> > --
> > 2.43.0
> >

Re: [PATCH 4/4] drm/xe: Fix dma-buf attachment leak in xe_gem_prime_import()

[Matthew Brost]

On Wed, Apr 08, 2026 at 11:34:28AM -0600, Lin, Shuicheng wrote:
> On Tue, Apr 7, 2026 10:05 PM Matthew Brost wrote:
> > On Tue, Apr 07, 2026 at 08:15:42PM +0000, Shuicheng Lin wrote:
> > > When xe_dma_buf_init_obj() fails, the attachment from
> > > dma_buf_dynamic_attach() is not detached. Add dma_buf_detach() before
> > > returning the error. Note: we cannot use goto out_err here because
> > > xe_dma_buf_init_obj() already frees bo on failure, and out_err would
> > > double-free it.
> > >
> > > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
> > > GPUs")
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: Claude:claude-opus-4.6
> > > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_dma_buf.c | 11 +++++++----
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > b/drivers/gpu/drm/xe/xe_dma_buf.c index 24d9d82426b9..7702a6bdaae5
> > > 100644
> > > --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> > > @@ -370,12 +370,15 @@ struct drm_gem_object
> > *xe_gem_prime_import(struct drm_device *dev,
> > >  		goto out_err;
> > >  	}
> > >
> > > -	/* Errors here will take care of freeing the bo. */
> > > +	/*
> > > +	 * xe_dma_buf_init_obj() takes ownership of bo on both success
> > > +	 * and failure, so we must not touch bo after this call.
> > > +	 */
> > >  	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> > > -	if (IS_ERR(obj))
> > > +	if (IS_ERR(obj)) {
> > > +		dma_buf_detach(dma_buf, attach);
> > 
> > Based on my feedback from the previous patch [1], I think we also want...
> > 
> > 		xe_bo_free(bo);
> > 
> > Also unseen in this diff is this code:
> > 
> > 365         attach = dma_buf_dynamic_attach(dma_buf, dev->dev, attach_ops,
> > &bo->ttm.base);
> > 366         if (IS_ERR(attach)) {
> > 367                 obj = ERR_CAST(attach);
> > 368                 goto out_err;
> > 369         }
> > 
> > We also need a xe_bo_free(bo) in this failures if statement.
> > 
> > Matt

^^^ Ignore all of this. Missed out_err calls xe_bo_free too.

So patch LGTM:
Reviewed-by: Mattheq Brost <matthew.brost@intel.com>

> > 
> > [1]
> > https://patchwork.freedesktop.org/patch/716820/?series=164476&rev=1#c
> > omment_1319810
> > 
> 
> As discussed in another email, could you please help me	review this patch again?
> Thanks.
> 
> Shuicheng
> 
> > >  		return obj;
> > > -
> > > -
> > > +	}
> > >  	get_dma_buf(dma_buf);
> > >  	obj->import_attach = attach;
> > >  	return obj;
> > > --
> > > 2.43.0
> > >