drm/vmwgfx: Do not drop the reference to the handle too soon

commit a950b989ea29ab3b38ea7f6e3d2540700a3c54e8 upstream.

v3: Fix vmw_user_bo_lookup which was also dropping the gem reference
before the kernel was done with buffer depending on userspace doing
the right thing. Same bug, different spot.

It is possible for userspace to predict the next buffer handle and
to destroy the buffer while it's still used by the kernel. Delay
dropping the internal reference on the buffers until kernel is done
with them.

Instead of immediately dropping the gem reference in vmw_user_bo_lookup
and vmw_gem_object_create_with_handle let the callers decide when they're
ready give the control back to userspace.

Also fixes the second usage of vmw_gem_object_create_with_handle in
vmwgfx_surface.c which wasn't grabbing an explicit reference
to the gem object which could have been destroyed by the userspace
on the owning surface at any point.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
Reviewed-by: Martin Krastev <krastevm@vmware.com>
Reviewed-by: Maaz Mombasawala <mombasawalam@vmware.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230211050514.2431155-1-zack@kde.org
(cherry picked from commit 9ef8d83e8e25d5f1811b3a38eb1484f85f64296c)
Cc: <stable@vger.kernel.org> # v5.17+
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 6e5483300ebc1f4362fe84c0e844f540e47aeddf..ae01d22b8f840d20d71c4f8fa06fb9cc9460be3c 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -598,6 +598,7 @@ static int vmw_user_bo_synccpu_release(struct drm_file *filp,
                ttm_bo_put(&vmw_bo->base);
        }
 
+       drm_gem_object_put(&vmw_bo->base.base);
        return ret;
 }
 
@@ -638,6 +639,7 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device *dev, void *data,
 
                ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
                vmw_bo_unreference(&vbo);
+               drm_gem_object_put(&vbo->base.base);
                if (unlikely(ret != 0)) {
                        if (ret == -ERESTARTSYS || ret == -EBUSY)
                                return -EBUSY;
@@ -695,7 +697,7 @@ int vmw_bo_unref_ioctl(struct drm_device *dev, void *data,
  * struct vmw_buffer_object should be placed.
  * Return: Zero on success, Negative error code on error.
  *
- * The vmw buffer object pointer will be refcounted.
+ * The vmw buffer object pointer will be refcounted (both ttm and gem)
  */
 int vmw_user_bo_lookup(struct drm_file *filp,
                       uint32_t handle,
@@ -712,7 +714,6 @@ int vmw_user_bo_lookup(struct drm_file *filp,
 
        *out = gem_to_vmw_bo(gobj);
        ttm_bo_get(&(*out)->base);
-       drm_gem_object_put(gobj);
 
        return 0;
 }
@@ -779,7 +780,8 @@ int vmw_dumb_create(struct drm_file *file_priv,
        ret = vmw_gem_object_create_with_handle(dev_priv, file_priv,
                                                args->size, &args->handle,
                                                &vbo);
-
+       /* drop reference from allocate - handle holds it now */
+       drm_gem_object_put(&vbo->base.base);
        return ret;
 }
 

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 70cfed4fdba042866be3482630b5bb6789a57871..1c88b74d68cf0db84466b829785131fa5fcce72c 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -1160,6 +1160,7 @@ static int vmw_translate_mob_ptr(struct vmw_private *dev_priv,
        }
        ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo, true, false);
        ttm_bo_put(&vmw_bo->base);
+       drm_gem_object_put(&vmw_bo->base.base);
        if (unlikely(ret != 0))
                return ret;
 
@@ -1214,6 +1215,7 @@ static int vmw_translate_guest_ptr(struct vmw_private *dev_priv,
        }
        ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo, false, false);
        ttm_bo_put(&vmw_bo->base);
+       drm_gem_object_put(&vmw_bo->base.base);
        if (unlikely(ret != 0))
                return ret;
 

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index 83d8f18cc16ff87eb24eafa12cc22992fe200f21..4d2c28e39f4e099dd6a83d74bdbb7366fc9fd174 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -152,8 +152,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv,
        (*p_vbo)->base.base.funcs = &vmw_gem_object_funcs;
 
        ret = drm_gem_handle_create(filp, &(*p_vbo)->base.base, handle);
-       /* drop reference from allocate - handle holds it now */
-       drm_gem_object_put(&(*p_vbo)->base.base);
 out_no_bo:
        return ret;
 }
@@ -180,6 +178,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev, void *data,
        rep->map_handle = drm_vma_node_offset_addr(&vbo->base.base.vma_node);
        rep->cur_gmr_id = handle;
        rep->cur_gmr_offset = 0;
+       /* drop reference from allocate - handle holds it now */
+       drm_gem_object_put(&vbo->base.base);
 out_no_bo:
        return ret;
 }

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 7a2f262414ad416e37941c3070c5a368b662332b..13721bcf047c074f191be30471aaf71fc1443d24 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1669,8 +1669,10 @@ 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)
+       if (bo) {
                vmw_bo_unreference(&bo);
+               drm_gem_object_put(&bo->base.base);
+       }
        if (surface)
                vmw_surface_unreference(&surface);
 

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
index e9f5c89b4ca6936b82d3e1eb91acfe6389b6abbd..b5b311f2a91a4061511a5afb56c65c00dd7f93c0 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
@@ -458,6 +458,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->base.base);
 
 out_unlock:
        mutex_unlock(&overlay->mutex);

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
index 108a496b5d1895ad4df21726d5e5222d56a7eb15..51e83dfa1cace6a8362523cb84b23d03c89562dc 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
@@ -807,6 +807,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv,
                                    num_output_sig, tfile, shader_handle);
 out_bad_arg:
        vmw_bo_unreference(&buffer);
+       drm_gem_object_put(&buffer->base.base);
        return ret;
 }
 

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index ace7ca150b0362ec69c4ec22530ad19d7ccc3ae7..591c301e6cf21921f1e1e50eda9ebd18a9f19f55 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -683,7 +683,7 @@ static void vmw_user_surface_base_release(struct ttm_base_object **p_base)
            container_of(base, struct vmw_user_surface, prime.base);
        struct vmw_resource *res = &user_srf->srf.res;
 
-       if (base->shareable && res && res->backup)
+       if (res && res->backup)
                drm_gem_object_put(&res->backup->base.base);
 
        *p_base = NULL;
@@ -860,7 +860,11 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data,
                        goto out_unlock;
                }
                vmw_bo_reference(res->backup);
-               drm_gem_object_get(&res->backup->base.base);
+               /*
+                * We don't expose the handle to the userspace and surface
+                * already holds a gem reference
+                */
+               drm_gem_handle_delete(file_priv, backup_handle);
        }
 
        tmp = vmw_resource_reference(&srf->res);
@@ -1564,8 +1568,6 @@ vmw_gb_surface_define_internal(struct drm_device *dev,
                        drm_vma_node_offset_addr(&res->backup->base.base.vma_node);
                rep->buffer_size = res->backup->base.base.size;
                rep->buffer_handle = backup_handle;
-               if (user_srf->prime.base.shareable)
-                       drm_gem_object_get(&res->backup->base.base);
        } else {
                rep->buffer_map_handle = 0;
                rep->buffer_size = 0;