Skip to content

Commit

Permalink
drm/vmwgfx: Fix stale file descriptors on failed usercopy
Browse files Browse the repository at this point in the history
commit a0f90c8 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: c906965 ("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>
  • Loading branch information
Mathias Krause authored and Greg Kroah-Hartman committed Jan 29, 2022
1 parent 6328816 commit e8d092a
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 22 deletions.
5 changes: 2 additions & 3 deletions drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
34 changes: 17 additions & 17 deletions drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e8d092a

Please sign in to comment.