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
smime.p7s
Description: S/MIME Cryptographic Signature
