Skip to content

Commit 39fdac6

Browse files
matt-auldgregkh
authored andcommitted
drm/xe/dma-buf: fix UAF with retry loop
commit 155a372a1cc50fa93387c5d3cdfd614a61e1afd1 upstream. Retry doesn't work here, since bo will be freed on error, leading to UAF. However, now that we do the alloc & init before the attach, we can now combine this as one unit and have the init do the alloc for us. This should make the retry safe. Reported by Sashiko. v2: Fix up the error unwind (CI) Closes: https://sashiko.dev/#/patchset/20260506184332.86743-2-matthew.auld%40intel.com Fixes: eb289a5 ("drm/xe: Convert xe_dma_buf.c for exhaustive eviction") Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: <stable@vger.kernel.org> # v6.18+ Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Link: https://patch.msgid.link/20260508102635.149172-4-matthew.auld@intel.com (cherry picked from commit 479669418253e0f27f8cf5db01a731352ea592e7) Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 20a99ea commit 39fdac6

1 file changed

Lines changed: 12 additions & 37 deletions

File tree

drivers/gpu/drm/xe/xe_dma_buf.c

Lines changed: 12 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -227,16 +227,8 @@ struct dma_buf *xe_gem_prime_export(struct drm_gem_object *obj, int flags)
227227
return buf;
228228
}
229229

230-
/*
231-
* Takes ownership of @storage: on success it is transferred to the returned
232-
* drm_gem_object; on failure it is freed before returning the error.
233-
* This matches the contract of xe_bo_init_locked() which frees @storage on
234-
* its error paths, so callers need not (and must not) free @storage after
235-
* this call.
236-
*/
237230
static struct drm_gem_object *
238-
xe_dma_buf_init_obj(struct drm_device *dev, struct xe_bo *storage,
239-
struct dma_buf *dma_buf)
231+
xe_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf)
240232
{
241233
struct dma_resv *resv = dma_buf->resv;
242234
struct xe_device *xe = to_xe_device(dev);
@@ -247,10 +239,8 @@ xe_dma_buf_init_obj(struct drm_device *dev, struct xe_bo *storage,
247239
int ret = 0;
248240

249241
dummy_obj = drm_gpuvm_resv_object_alloc(&xe->drm);
250-
if (!dummy_obj) {
251-
xe_bo_free(storage);
242+
if (!dummy_obj)
252243
return ERR_PTR(-ENOMEM);
253-
}
254244

255245
dummy_obj->resv = resv;
256246
xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {}, ret) {
@@ -259,8 +249,7 @@ xe_dma_buf_init_obj(struct drm_device *dev, struct xe_bo *storage,
259249
if (ret)
260250
break;
261251

262-
/* xe_bo_init_locked() frees storage on error */
263-
bo = xe_bo_init_locked(xe, storage, NULL, resv, NULL, dma_buf->size,
252+
bo = xe_bo_init_locked(xe, NULL, NULL, resv, NULL, dma_buf->size,
264253
0, /* Will require 1way or 2way for vm_bind */
265254
ttm_bo_type_sg, XE_BO_FLAG_SYSTEM, &exec);
266255
drm_exec_retry_on_contention(&exec);
@@ -311,7 +300,6 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
311300
const struct dma_buf_attach_ops *attach_ops;
312301
struct dma_buf_attachment *attach;
313302
struct drm_gem_object *obj;
314-
struct xe_bo *bo;
315303

316304
if (dma_buf->ops == &xe_dmabuf_ops) {
317305
obj = dma_buf->priv;
@@ -326,22 +314,14 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
326314
}
327315
}
328316

329-
bo = xe_bo_alloc();
330-
if (IS_ERR(bo))
331-
return ERR_CAST(bo);
332-
333317
/*
334-
* xe_dma_buf_init_obj() takes ownership of the raw bo, so do not touch
335-
* on fail, since it will already take care of cleanup. On success we
336-
* still need to drop the ref, if something later fails.
337-
*
338-
* In addition this needs to happen before the attach, since
339-
* it will create a new attachment for this, and add it to the list of
340-
* attachments, at which point it is globally visible, and at any point
341-
* the export side can call into on invalidate_mappings callback, which
342-
* require a working object.
318+
* This needs to happen before the attach, since it will create a new
319+
* attachment for this, and add it to the list of attachments, at which
320+
* point it is globally visible, and at any point the export side can
321+
* call into on invalidate_mappings callback, which require a working
322+
* object.
343323
*/
344-
obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
324+
obj = xe_dma_buf_create_obj(dev, dma_buf);
345325
if (IS_ERR(obj))
346326
return obj;
347327

@@ -351,20 +331,15 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
351331
attach_ops = test->attach_ops;
352332
#endif
353333

354-
attach = dma_buf_dynamic_attach(dma_buf, dev->dev, attach_ops, &bo->ttm.base);
334+
attach = dma_buf_dynamic_attach(dma_buf, dev->dev, attach_ops, obj);
355335
if (IS_ERR(attach)) {
356-
obj = ERR_CAST(attach);
357-
goto out_err;
336+
xe_bo_put(gem_to_xe_bo(obj));
337+
return ERR_CAST(attach);
358338
}
359339

360340
get_dma_buf(dma_buf);
361341
obj->import_attach = attach;
362342
return obj;
363-
364-
out_err:
365-
xe_bo_put(bo);
366-
367-
return obj;
368343
}
369344

370345
#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)

0 commit comments

Comments
 (0)