Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] NULL pointer dereference when dma_buf import fails. -- commit 200f36633d8d14d0ae43912ea8f089cba49bace3 Author: Michal TOMA <[email protected]> drm/vmwgfx: Release PRIME import in the BO destroy path This patch fixes a resource leak in the vmwgfx driver where PRIME imported dma-bufs are not properly torn down when the buffer object is destroyed. It adds a call to drm_prime_gem_destroy() in vmw_bo_free() to ensure the attachment and sg mapping are released. > drm_gem_prime_import_dev() attaches to a foreign dma-buf, takes a > reference with get_dma_buf(), maps the attachment and, once > vmw_prime_import_sg_table() has created the TTM buffer object, stores > the attachment in obj->import_attach. Drivers that import this way have > to undo it by calling drm_prime_gem_destroy() when the object is freed. [Severity: High] This is a pre-existing issue, but since we are looking at the dma_buf import path, could a failed import trigger a NULL pointer dereference here? When a process imports a dma_buf via DRM_IOCTL_PRIME_FD_TO_HANDLE on the vmwgfx device, the DRM core calls vmw_prime_import_sg_table(). If BO creation fails, the function returns NULL instead of an ERR_PTR: drivers/gpu/drm/vmwgfx/vmwgfx_gem.c:vmw_prime_import_sg_table() { ... out_no_bo: dma_resv_unlock(params.resv); return gem; } In the DRM core, drm_gem_prime_import_dev() only checks for IS_ERR(), which is false for NULL: drivers/gpu/drm/drm_prime.c:drm_gem_prime_import_dev() { ... obj = dev->driver->gem_prime_import_sg_table(dev, attach, sgt); if (IS_ERR(obj)) { ret = PTR_ERR(obj); goto fail_unmap; } obj->import_attach = attach; ... } Does this mean an unprivileged user could crash the kernel by forcing an import failure (e.g. by exhausting memory or providing an excessive size)? Should vmw_prime_import_sg_table() return ERR_PTR(-ENOMEM) instead of NULL on failure? [ ... ] > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 9c7a73c0b0dc3..56bc94edc30b7 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c [ ... ] > @@ -69,6 +70,8 @@ static void vmw_bo_free(struct ttm_buffer_object *bo) > vmw_surface_unreference(&vbo->dumb_surface); > } > WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree)); > + if (drm_gem_is_imported(&vbo->tbo.base)) > + drm_prime_gem_destroy(&vbo->tbo.base, vbo->tbo.sg); > drm_gem_object_release(&vbo->tbo.base); > WARN_ON(vbo->dirty); > kfree(vbo); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
