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