Re: [PATCH v2 2/3] drm/vmwgfx: Release PRIME import in the BO destroy path

[sashiko-bot@kernel.org]

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 <michaltoma@sicoop.com>

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/20260910165221.7558-1-michaltoma@sicoop.com?part=2