Re: [PATCH] drm/vmwgfx: Erase the detached resource entry on resource release

On Sun, Jul 26, 2026 at 11:21 AM Zack Rusin <[email protected]> wrote:
> On Sat, Jul 25, 2026 at 10:39 AM Jaeyeong Lee <[email protected]> wrote:
> >
> > vmw_bo_add_detached_resource() stores a raw struct vmw_resource pointer in
> > bo->detached_resources without taking a reference on the resource, and the
> > only place that erases the entry again is vmw_resource_mob_attach(). A
> > resource that is never validated, and therefore never attached to its
> > backing mob, keeps its entry until the buffer object itself is destroyed.
> >
> > vmw_resource_release() detaches the resource from the mob RB tree and drops
> > its reference on the backing buffer object, but leaves the
> > detached_resources entry in place. Userspace can hold an independent GEM
> > handle on that buffer object, either because DRM_VMW_GB_SURFACE_CREATE
> > returned one in rep.buffer_handle, or because DRM_VMW_GB_SURFACE_REF minted
> > one with drm_gem_handle_create(). The buffer object then outlives the
> > surface while still holding a pointer to freed memory.
> >
> > Any later surface lookup for that buffer object dereferences it:
> >
> >   vmw_prime_handle_to_fd()
> >     vmw_lookup_surface_handle_for_buffer()
> >       vmw_lookup_user_surface_for_buffer()
> >         vmw_bo_surface()
> >           res->func->res_type           <-- use-after-free read
> >
> > vmw_bo_surface() walks detached_resources before the mob RB tree, so the
> > stale entry is matched first. The kref_get_unless_zero() in
> > vmw_lookup_user_surface_for_buffer() does not help here: it runs after
> > the dereference, and struct vmw_user_surface is freed with kfree_rcu()
> > on a slab that is not SLAB_TYPESAFE_BY_RCU, so the memory is handed out
> > to unrelated allocations once the grace period elapses.
> >
> > A sequence that reaches this from userspace is to create a guest-backed
> > surface with drm_vmw_surface_flag_create_buffer, never validate it,
> > release it with DRM_VMW_UNREF_SURFACE, and then call
> > DRM_IOCTL_PRIME_HANDLE_TO_FD on the buffer handle returned by the create
> > ioctl. All of the ioctls involved are DRM_RENDER_ALLOW. The code was
> > analysed on v7.2-rc4; the erase call site has been the same since the
> > commit below. Found by code inspection, not by a runtime reproducer.
> >
> > Erase the entry next to the existing vmw_resource_mob_detach() call, so
> > that a released resource is removed from both containers, mirroring what
> > vmw_resource_mob_attach() does. The dumb buffer teardown path in
> > vmw_bo_free() clears res->guest_memory_bo before dropping its surface
> > reference, so it does not reach this code and is unaffected.
> >
> > Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
> > Cc: [email protected]
> > Signed-off-by: Jaeyeong Lee <[email protected]>
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > 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
> > @@ -138,6 +138,7 @@ static void vmw_resource_release(struct kref *kref)
> >                 }
> >                 res->guest_memory_size = false;
> >                 vmw_resource_mob_detach(res);
> > +               vmw_bo_del_detached_resource(res->guest_memory_bo, res);
> >                 if (res->dirty)
> >                         res->func->dirty_free(res);
> >                 if (res->coherent)
> > --
> > 2.43.0
> >
> >
>
> Thanks. From the description I'm guessing you forgot to disclose the
> exact LLM you used to detect and fix this. The patch itself wasn't
> generated by git format-patch/send-email . Please respin this fixing
> both issues. Also please improve the commit message to make it more
> human-readable.

Are you still interested in upstreaming this change? I'd need a v2
that addresses the issues mentioned above.

z