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

Lin, Shuicheng
2026-04-08T15:58:35Z

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
> >