drm/vmwgfx: Fix stale file descriptors on failed usercopy

commit a0f90c8815706981c483a652a6aefca51a5e191c upstream.

A failing usercopy of the fence_rep object will lead to a stale entry in
the file descriptor table as put_unused_fd() won't release it. This
enables userland to refer to a dangling 'file' object through that still
valid file descriptor, leading to all kinds of use-after-free
exploitation scenarios.

Fix this by deferring the call to fd_install() until after the usercopy
has succeeded.

Fixes: c906965dee22 ("drm/vmwgfx: Add export fence to file descriptor support")
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
Signed-off-by: Zack Rusin <zackr@vmware.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 8c65cc3b0dda2d63ffae8629273ef4193ee86dbc..8f5321f098569a99b11fb71a8b3f6158e390c19b 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -837,15 +837,14 @@ extern int vmw_execbuf_fence_commands(struct drm_file *file_priv,
                                      struct vmw_private *dev_priv,
                                      struct vmw_fence_obj **p_fence,
                                      uint32_t *p_handle);
-extern void vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
+extern int vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
                                        struct vmw_fpriv *vmw_fp,
                                        int ret,
                                        struct drm_vmw_fence_rep __user
                                        *user_fence_rep,
                                        struct vmw_fence_obj *fence,
                                        uint32_t fence_handle,
-                                       int32_t out_fence_fd,
-                                       struct sync_file *sync_file);
+                                       int32_t out_fence_fd);
 extern int vmw_validate_single_buffer(struct vmw_private *dev_priv,
                                      struct ttm_buffer_object *bo,
                                      bool interruptible,

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index dc677ba4dc3804efb7c119baf98a014517fff8de..996696ad6f988e451174e8fc226333309f940808 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3848,20 +3848,19 @@ int vmw_execbuf_fence_commands(struct drm_file *file_priv,
  * object so we wait for it immediately, and then unreference the
  * user-space reference.
  */
-void
+int
 vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
                            struct vmw_fpriv *vmw_fp,
                            int ret,
                            struct drm_vmw_fence_rep __user *user_fence_rep,
                            struct vmw_fence_obj *fence,
                            uint32_t fence_handle,
-                           int32_t out_fence_fd,
-                           struct sync_file *sync_file)
+                           int32_t out_fence_fd)
 {
        struct drm_vmw_fence_rep fence_rep;
 
        if (user_fence_rep == NULL)
-               return;
+               return 0;
 
        memset(&fence_rep, 0, sizeof(fence_rep));
 
@@ -3889,20 +3888,14 @@ vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
         * and unreference the handle.
         */
        if (unlikely(ret != 0) && (fence_rep.error == 0)) {
-               if (sync_file)
-                       fput(sync_file->file);
-
-               if (fence_rep.fd != -1) {
-                       put_unused_fd(fence_rep.fd);
-                       fence_rep.fd = -1;
-               }
-
                ttm_ref_object_base_unref(vmw_fp->tfile,
                                          fence_handle, TTM_REF_USAGE);
                DRM_ERROR("Fence copy error. Syncing.\n");
                (void) vmw_fence_obj_wait(fence, false, false,
                                          VMW_FENCE_WAIT_TIMEOUT);
        }
+
+       return ret ? -EFAULT : 0;
 }
 
 /**
@@ -4262,16 +4255,23 @@ int vmw_execbuf_process(struct drm_file *file_priv,
 
                        (void) vmw_fence_obj_wait(fence, false, false,
                                                  VMW_FENCE_WAIT_TIMEOUT);
+               }
+       }
+
+       ret = vmw_execbuf_copy_fence_user(dev_priv, vmw_fpriv(file_priv), ret,
+                                   user_fence_rep, fence, handle, out_fence_fd);
+
+       if (sync_file) {
+               if (ret) {
+                       /* usercopy of fence failed, put the file object */
+                       fput(sync_file->file);
+                       put_unused_fd(out_fence_fd);
                } else {
                        /* Link the fence with the FD created earlier */
                        fd_install(out_fence_fd, sync_file->file);
                }
        }
 
-       vmw_execbuf_copy_fence_user(dev_priv, vmw_fpriv(file_priv), ret,
-                                   user_fence_rep, fence, handle,
-                                   out_fence_fd, sync_file);
-
        /* Don't unreference when handing fence out */
        if (unlikely(out_fence != NULL)) {
                *out_fence = fence;
@@ -4290,7 +4290,7 @@ int vmw_execbuf_process(struct drm_file *file_priv,
         */
        vmw_resource_list_unreference(sw_context, &resource_list);
 
-       return 0;
+       return ret;
 
 out_unlock_binding:
        mutex_unlock(&dev_priv->binding_mutex);

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index d6b1c509ae019d1aae404ff5163506d5e819155e..7d2482644ef70fe7069b563022f7b7246bb433d5 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -1150,7 +1150,7 @@ int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
        }
 
        vmw_execbuf_copy_fence_user(dev_priv, vmw_fp, 0, user_fence_rep, fence,
-                                   handle, -1, NULL);
+                                   handle, -1);
        vmw_fence_obj_unreference(&fence);
        return 0;
 out_no_create:

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 848c9d009be2a5343d5114995a891f459540fc04..fbba55edbfd02311b11019ed77dbf2b168c703a3 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2511,7 +2511,7 @@ void vmw_kms_helper_buffer_finish(struct vmw_private *dev_priv,
        if (file_priv)
                vmw_execbuf_copy_fence_user(dev_priv, vmw_fpriv(file_priv),
                                            ret, user_fence_rep, fence,
-                                           handle, -1, NULL);
+                                           handle, -1);
        if (out_fence)
                *out_fence = fence;
        else