Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/vmwgfx: Fix dumb buffer leak

Dumb buffers were not being freed because the GEM reference that was
acquired in gb_surface_define was not dropped like it is in the 2D case.
Dropping this ref uncovered a few additional issues with freeing the
resources associated with dirty tracking in vmw_bo_free/release.

Additionally the TTM object associated with the surface were also leaking
which meant that when the ttm_object_file was closed at process exit the
destructor unreferenced an already destroyed surface.

The solution is to remove the destructor from the vmw_user_surface
associated with the dumb_buffer and immediately unreferencing the TTM
object which his removes it from the ttm_object_file.

This also allows the early return in vmw_user_surface_base_release for the
dumb buffer case to be removed as it should no longer occur.

The chain of references now has the GEM handle(s) owning the dumb buffer.
The GEM handles have a singular GEM reference to the vmw_bo which is
dropped when all handles are closed. When the GEM reference count hits
zero the vmw_bo is freed which then unreferences the surface via
vmw_resource_release in vmw_bo_release.

Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
Reviewed-by: Zack Rusin <zack.rusin@broadcom.com>
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250123204424.836896-1-ian.forbes@broadcom.com

authored by

Ian Forbes and committed by
Zack Rusin
f42c09e6 96c85e42

+17 -9
+4 -2
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
··· 51 51 mutex_lock(&res->dev_priv->cmdbuf_mutex); 52 52 (void)vmw_resource_reserve(res, false, true); 53 53 vmw_resource_mob_detach(res); 54 + if (res->dirty) 55 + res->func->dirty_free(res); 54 56 if (res->coherent) 55 57 vmw_bo_dirty_release(res->guest_memory_bo); 56 58 res->guest_memory_bo = NULL; 57 59 res->guest_memory_offset = 0; 58 - vmw_resource_unreserve(res, false, false, false, NULL, 60 + vmw_resource_unreserve(res, true, false, false, NULL, 59 61 0); 60 62 mutex_unlock(&res->dev_priv->cmdbuf_mutex); 61 63 } ··· 75 73 { 76 74 struct vmw_bo *vbo = to_vmw_bo(&bo->base); 77 75 78 - WARN_ON(vbo->dirty); 79 76 WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree)); 80 77 vmw_bo_release(vbo); 78 + WARN_ON(vbo->dirty); 81 79 kfree(vbo); 82 80 } 83 81
+1 -1
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
··· 273 273 goto out_bad_resource; 274 274 275 275 res = converter->base_obj_to_res(base); 276 - kref_get(&res->kref); 276 + vmw_resource_reference(res); 277 277 278 278 *p_res = res; 279 279 ret = 0;
+12 -6
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
··· 639 639 struct vmw_user_surface *user_srf = 640 640 container_of(srf, struct vmw_user_surface, srf); 641 641 642 - WARN_ON_ONCE(res->dirty); 642 + WARN_ON(res->dirty); 643 643 if (user_srf->master) 644 644 drm_master_put(&user_srf->master); 645 645 kfree(srf->offsets); ··· 670 670 * Dumb buffers own the resource and they'll unref the 671 671 * resource themselves 672 672 */ 673 - if (res && res->guest_memory_bo && res->guest_memory_bo->is_dumb) 674 - return; 673 + WARN_ON(res && res->guest_memory_bo && res->guest_memory_bo->is_dumb); 675 674 676 675 vmw_resource_unreference(&res); 677 676 } ··· 2336 2337 vbo = res->guest_memory_bo; 2337 2338 vbo->is_dumb = true; 2338 2339 vbo->dumb_surface = vmw_res_to_srf(res); 2339 - 2340 + drm_gem_object_put(&vbo->tbo.base); 2341 + /* 2342 + * Unset the user surface dtor since this in not actually exposed 2343 + * to userspace. The suface is owned via the dumb_buffer's GEM handle 2344 + */ 2345 + struct vmw_user_surface *usurf = container_of(vbo->dumb_surface, 2346 + struct vmw_user_surface, srf); 2347 + usurf->prime.base.refcount_release = NULL; 2340 2348 err: 2341 2349 if (res) 2342 2350 vmw_resource_unreference(&res); 2343 - if (ret) 2344 - ttm_ref_object_base_unref(tfile, arg.rep.handle); 2351 + 2352 + ttm_ref_object_base_unref(tfile, arg.rep.handle); 2345 2353 2346 2354 return ret; 2347 2355 }