Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Jumping to out_bind_failed on dirty_alloc failure destroys the 
hardware resource without detaching the guest memory (MOB), leading to state 
desynchronization and invalid hardware commands (-1 ID) being sent to the 
hypervisor.
--

commit 176a58de892da45c4d3d22e924894efcb6a1de37
Author: Wentao Liang <[email protected]>

drm/vmwgfx: fix refcount leak in vmw_resource_do_validate()

This patch attempts to fix a refcount leak in vmw_resource_do_validate()
by jumping to out_bind_failed when func->dirty_alloc() fails, which calls
func->destroy() to balance the resource reference.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -402,7 +402,7 @@ static int vmw_resource_do_validate(struct vmw_resource 
> *res,
>               if (res->guest_memory_bo->dirty && !res->dirty) {
>                       ret = func->dirty_alloc(res);
>                       if (ret)
> -                             return ret;
> +                             goto out_bind_failed;

[Severity: High]
Does jumping to out_bind_failed here destroy the hardware resource without
detaching the guest memory (MOB)?

If func->dirty_alloc() fails, jumping to out_bind_failed calls
func->destroy(res), which destroys the hardware resource and sets
res->id = -1. However, since the MOB was already attached earlier in the
function, skipping func->unbind() and vmw_resource_mob_detach() leaves the
software state out of sync, as vmw_resource_mob_attached(res) remains true.

When the resource is later evicted or released, routines like
vmw_resource_release() or vmw_resource_do_evict() will see the MOB is still
attached and call func->unbind(res).

For example, vmw_gb_surface_unbind() will then use the destroyed res->id
(-1), sending an invalid command (sid = 0xffffffff) to the hypervisor:

drivers/gpu/drm/vmwgfx/vmwgfx_surface.c:vmw_gb_surface_unbind() {
    ...
    cmd3->header.id = SVGA_3D_CMD_BIND_GB_SURFACE;
    cmd3->header.size = sizeof(cmd3->body);
    cmd3->body.sid = res->id;
    cmd3->body.mobid = SVGA3D_INVALID_ID;
    ...
}

>               } else if (!res->guest_memory_bo->dirty && res->dirty) {
>                       func->dirty_free(res);
>               }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to