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 <[email protected]> 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/[email protected]?part=1
