[PATCH] drm/vmwgfx: fix ttm_base_object refcount leak in vmw_buffer_prime_to_surface_base()

[PATCH] drm/vmwgfx: fix ttm_base_object refcount leak in vmw_buffer_prime_to_surface_base()

[Wentao Liang]

vmw_lookup_user_surface_for_buffer() returns a pointer with a
temporary reference taken via kref_get_unless_zero(). The other
two callers (vmw_lookup_surface_for_buffer and
vmw_lookup_surface_handle_for_buffer) correctly release it with
ttm_base_object_unref(). vmw_buffer_prime_to_surface_base() does
not, leaking the reference on both the success and
ttm_ref_object_add() failure paths.

Add the missing ttm_base_object_unref() before vmw_user_bo_unref()
at the out label.

Cc: stable@vger.kernel.org
Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index b2d3927b5567..9e63846fd663 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -972,6 +972,7 @@ static int vmw_buffer_prime_to_surface_base(struct vmw_private *dev_priv,
 
 	*base_p = base;
 out:
+	ttm_base_object_unref(&base);
 	vmw_user_bo_unref(&bo);
 
 	return ret;
-- 
2.34.1

Re: [PATCH] drm/vmwgfx: fix ttm_base_object refcount leak in vmw_buffer_prime_to_surface_base()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Uninitialized variable dereference leading to kernel panic on the error path.
- [Critical] Double-put / Use-After-Free on the success path due to dropping a reference expected by the caller.
--

commit 6c286ae1f64531ad67606db48092510b8d2a64d9
Author: Wentao Liang <vulab@iscas.ac.cn>

drm/vmwgfx: fix ttm_base_object refcount leak in vmw_buffer_prime_to_surface_base()

This patch attempts to fix a reference leak where the reference returned by
vmw_lookup_user_surface_for_buffer() was not properly dropped. It adds a call
to ttm_base_object_unref() at the error handling out label.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index b2d3927b55673..9e63846fd6630 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -972,6 +972,7 @@ static int vmw_buffer_prime_to_surface_base(struct vmw_private *dev_priv,
>  
>  	*base_p = base;
>  out:
> +	ttm_base_object_unref(&base);

[Severity: Critical]
Does this code dereference an uninitialized pointer when
vmw_lookup_user_surface_for_buffer() fails?

In vmw_buffer_prime_to_surface_base(), base is declared but not initialized.
If vmw_lookup_user_surface_for_buffer() returns NULL, the code jumps to the
out label:

	user_srf = vmw_lookup_user_surface_for_buffer(dev_priv, bo, *handle);
	if (WARN_ON(!user_srf)) {
		drm_warn(&dev_priv->drm,
			 "User surface fd %d (handle %d) is null.\n", fd, *handle);
		ret = -EINVAL;
		goto out;
	}

Because base has not been assigned a value before this jump, calling
ttm_base_object_unref(&base) at the out label will dereference an uninitialized
pointer, which could cause a kernel panic.

[Severity: Critical]
Will dropping the reference here cause a double-put or use-after-free on the
success path?

On the success path, the resolved base object is assigned to *base_p:

	*base_p = base;

If the reference is unconditionally unreferenced at the out label, the object
returned to the caller (e.g. vmw_surface_reference_ioctl) will have already
lost the reference.

Since the caller expects to own this temporary reference and drop it
themselves, wouldn't dropping it here result in a double-put when the caller
eventually calls ttm_base_object_unref() on the returned object?

>  	vmw_user_bo_unref(&bo);
>  
>  	return ret;

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