drm/vmwgfx: Fix possible invalid drm gem put calls
authorZack Rusin <zackr@vmware.com>
Fri, 18 Aug 2023 04:13:01 +0000 (00:13 -0400)
committerZack Rusin <zackr@vmware.com>
Wed, 23 Aug 2023 17:20:04 +0000 (13:20 -0400)
vmw_bo_unreference sets the input buffer to null on exit, resulting in
null ptr deref's on the subsequent drm gem put calls.

This went unnoticed because only very old userspace would be exercising
those paths but it wouldn't be hard to hit on old distros with brand
new kernels.

Introduce a new function that abstracts unrefing of user bo's to make
the code cleaner and more explicit.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Reported-by: Ian Forbes <iforbes@vmware.com>
Fixes: 9ef8d83e8e25 ("drm/vmwgfx: Do not drop the reference to the handle too soon")
Cc: <stable@vger.kernel.org> # v6.4+
Reviewed-by: Maaz Mombasawala<mombasawalam@vmware.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230818041301.407636-1-zack@kde.org
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c

index 82094c137855b9b7e6a2c2f6666da6bb58553f36..c43853597776ffb0e6fea21e8cecbb1f6add9a7c 100644 (file)
@@ -497,10 +497,9 @@ static int vmw_user_bo_synccpu_release(struct drm_file *filp,
                if (!(flags & drm_vmw_synccpu_allow_cs)) {
                        atomic_dec(&vmw_bo->cpu_writers);
                }
-               ttm_bo_put(&vmw_bo->tbo);
+               vmw_user_bo_unref(vmw_bo);
        }
 
-       drm_gem_object_put(&vmw_bo->tbo.base);
        return ret;
 }
 
@@ -540,8 +539,7 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device *dev, void *data,
                        return ret;
 
                ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
-               vmw_bo_unreference(&vbo);
-               drm_gem_object_put(&vbo->tbo.base);
+               vmw_user_bo_unref(vbo);
                if (unlikely(ret != 0)) {
                        if (ret == -ERESTARTSYS || ret == -EBUSY)
                                return -EBUSY;
index 50a836e70994934a7dd8dd2c1c2b36a55cc64f4c..1d433fceed3d84117b289534940d5192e362fa98 100644 (file)
@@ -195,6 +195,14 @@ static inline struct vmw_bo *vmw_bo_reference(struct vmw_bo *buf)
        return buf;
 }
 
+static inline void vmw_user_bo_unref(struct vmw_bo *vbo)
+{
+       if (vbo) {
+               ttm_bo_put(&vbo->tbo);
+               drm_gem_object_put(&vbo->tbo.base);
+       }
+}
+
 static inline struct vmw_bo *to_vmw_bo(struct drm_gem_object *gobj)
 {
        return container_of((gobj), struct vmw_bo, tbo.base);
index d30c0e3d3ab7c2d11c3c43a64654a45fb8047e2b..98e0723ca6f5eec96017a39a0372e1eb5c471545 100644 (file)
@@ -1164,8 +1164,7 @@ static int vmw_translate_mob_ptr(struct vmw_private *dev_priv,
        }
        vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_MOB, VMW_BO_DOMAIN_MOB);
        ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
-       ttm_bo_put(&vmw_bo->tbo);
-       drm_gem_object_put(&vmw_bo->tbo.base);
+       vmw_user_bo_unref(vmw_bo);
        if (unlikely(ret != 0))
                return ret;
 
@@ -1221,8 +1220,7 @@ static int vmw_translate_guest_ptr(struct vmw_private *dev_priv,
        vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM,
                             VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM);
        ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
-       ttm_bo_put(&vmw_bo->tbo);
-       drm_gem_object_put(&vmw_bo->tbo.base);
+       vmw_user_bo_unref(vmw_bo);
        if (unlikely(ret != 0))
                return ret;
 
index b62207be3363e7a93a08b103dc6b2ed492083116..1489ad73c103fe3ac5e1a8e34b3ff1704b7253da 100644 (file)
@@ -1665,10 +1665,8 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
 
 err_out:
        /* vmw_user_lookup_handle takes one ref so does new_fb */
-       if (bo) {
-               vmw_bo_unreference(&bo);
-               drm_gem_object_put(&bo->tbo.base);
-       }
+       if (bo)
+               vmw_user_bo_unref(bo);
        if (surface)
                vmw_surface_unreference(&surface);
 
index 7e112319a23ce2f96674dc63316b1b08d745c2f1..fb85f244c3d028022a8a422df6b5022184ef29db 100644 (file)
@@ -451,8 +451,7 @@ int vmw_overlay_ioctl(struct drm_device *dev, void *data,
 
        ret = vmw_overlay_update_stream(dev_priv, buf, arg, true);
 
-       vmw_bo_unreference(&buf);
-       drm_gem_object_put(&buf->tbo.base);
+       vmw_user_bo_unref(buf);
 
 out_unlock:
        mutex_unlock(&overlay->mutex);
index e7226db8b242481fc7a7790ef07ba244f80aad94..1e81ff2422cf6f3900ab968b3a1c77e15834bd90 100644 (file)
@@ -809,8 +809,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv,
                                    shader_type, num_input_sig,
                                    num_output_sig, tfile, shader_handle);
 out_bad_arg:
-       vmw_bo_unreference(&buffer);
-       drm_gem_object_put(&buffer->tbo.base);
+       vmw_user_bo_unref(buffer);
        return ret;
 }