Re: [PATCH] fix: drm/vmwgfx: vmw_user_shader_alloc: fix base object refcount leak on ttm_base_object_init failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Passing `&ushader->base` to `ttm_base_object_unref` causes a type mismatch that leads to a guaranteed NULL pointer dereference.
- [Critical] Calling `ttm_base_object_unref` in the error path drops the resource reference prematurely, leading to a use-after-free and double-free.
- [High] Calling `ttm_base_object_unref` on a failed initialization causes a double `idr_remove`, leading to IDR corruption.
--

commit 126458e87c7593f5bdf4d05a97e545b741bd41aa
Author: WenTao Liang <vulab@iscas.ac.cn>

fix: drm/vmwgfx: vmw_user_shader_alloc: fix base object refcount leak on ttm_base_object_init failure

ttm_base_object_init unconditionally acquires a base object reference via
  kref_init. When it fails, the error path in vmw_user_shader_alloc only
  calls vmw_resource_unreference but never calls ttm_base_object_unref to
  release the base object reference, causing a refcount leak.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> index eca4e3e97eb4e..d82ec43c8901a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> @@ -724,6 +724,7 @@ static int vmw_user_shader_alloc(struct vmw_private *dev_priv,
>  
>  	if (unlikely(ret != 0)) {
>  		vmw_resource_unreference(&tmp);
> +		ttm_base_object_unref(&ushader->base);

[Severity: Critical]
Does passing &ushader->base to ttm_base_object_unref() cause a type mismatch 
that leads to a null pointer dereference?

The function ttm_base_object_unref() expects a double pointer:

void ttm_base_object_unref(struct ttm_base_object **p_base)
{
	struct ttm_base_object *base = *p_base;

	*p_base = NULL;
...

By passing the single pointer &ushader->base, the function will interpret the 
first 8 bytes of the struct as a pointer. It reads the zeroed field, resulting 
in base being null, and then dereferences it when calling kref_put().

[Severity: Critical]
Does calling ttm_base_object_unref() here lead to a use-after-free and
double-free later in the error path?

If ttm_base_object_unref() drops the last reference, it will call the 
release callback vmw_user_shader_base_release() which frees the ushader 
object. The execution then falls through to the out_err label:

out_err:
	vmw_resource_unreference(&res);

Because res was initialized as &ushader->shader.res, this will attempt to 
use and unreference a field inside the already freed ushader object.

[Severity: High]
Can this unref call cause an issue due to a double idr_remove()?

If ttm_base_object_init() fails, its internal error path already calls 
idr_remove(&tdev->idr, base->handle) to clean up. Because base->handle 
remains populated, calling ttm_base_object_unref() here will invoke 
ttm_release_base(), which executes idr_remove() a second time for the 
same handle.

>  		goto out_err;
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626150142.49732-1-vulab@iscas.ac.cn?part=1