Skip to content

Commit a0f90c8

Browse files
minipli-osstorvalds
authored andcommitted
drm/vmwgfx: Fix stale file descriptors on failed usercopy
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 <[email protected]> Signed-off-by: Zack Rusin <[email protected]> Signed-off-by: Dave Airlie <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 626b2dd commit a0f90c8

File tree

4 files changed

+21
-21
lines changed

4 files changed

+21
-21
lines changed

drivers/gpu/drm/vmwgfx/vmwgfx_drv.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,15 +1140,14 @@ extern int vmw_execbuf_fence_commands(struct drm_file *file_priv,
11401140
struct vmw_private *dev_priv,
11411141
struct vmw_fence_obj **p_fence,
11421142
uint32_t *p_handle);
1143-
extern void vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
1143+
extern int vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
11441144
struct vmw_fpriv *vmw_fp,
11451145
int ret,
11461146
struct drm_vmw_fence_rep __user
11471147
*user_fence_rep,
11481148
struct vmw_fence_obj *fence,
11491149
uint32_t fence_handle,
1150-
int32_t out_fence_fd,
1151-
struct sync_file *sync_file);
1150+
int32_t out_fence_fd);
11521151
bool vmw_cmd_describe(const void *buf, u32 *size, char const **cmd);
11531152

11541153
/**

drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3879,17 +3879,17 @@ int vmw_execbuf_fence_commands(struct drm_file *file_priv,
38793879
* Also if copying fails, user-space will be unable to signal the fence object
38803880
* so we wait for it immediately, and then unreference the user-space reference.
38813881
*/
3882-
void
3882+
int
38833883
vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
38843884
struct vmw_fpriv *vmw_fp, int ret,
38853885
struct drm_vmw_fence_rep __user *user_fence_rep,
38863886
struct vmw_fence_obj *fence, uint32_t fence_handle,
3887-
int32_t out_fence_fd, struct sync_file *sync_file)
3887+
int32_t out_fence_fd)
38883888
{
38893889
struct drm_vmw_fence_rep fence_rep;
38903890

38913891
if (user_fence_rep == NULL)
3892-
return;
3892+
return 0;
38933893

38943894
memset(&fence_rep, 0, sizeof(fence_rep));
38953895

@@ -3917,19 +3917,13 @@ vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
39173917
* handle.
39183918
*/
39193919
if (unlikely(ret != 0) && (fence_rep.error == 0)) {
3920-
if (sync_file)
3921-
fput(sync_file->file);
3922-
3923-
if (fence_rep.fd != -1) {
3924-
put_unused_fd(fence_rep.fd);
3925-
fence_rep.fd = -1;
3926-
}
3927-
39283920
ttm_ref_object_base_unref(vmw_fp->tfile, fence_handle);
39293921
VMW_DEBUG_USER("Fence copy error. Syncing.\n");
39303922
(void) vmw_fence_obj_wait(fence, false, false,
39313923
VMW_FENCE_WAIT_TIMEOUT);
39323924
}
3925+
3926+
return ret ? -EFAULT : 0;
39333927
}
39343928

39353929
/**
@@ -4266,16 +4260,23 @@ int vmw_execbuf_process(struct drm_file *file_priv,
42664260

42674261
(void) vmw_fence_obj_wait(fence, false, false,
42684262
VMW_FENCE_WAIT_TIMEOUT);
4263+
}
4264+
}
4265+
4266+
ret = vmw_execbuf_copy_fence_user(dev_priv, vmw_fpriv(file_priv), ret,
4267+
user_fence_rep, fence, handle, out_fence_fd);
4268+
4269+
if (sync_file) {
4270+
if (ret) {
4271+
/* usercopy of fence failed, put the file object */
4272+
fput(sync_file->file);
4273+
put_unused_fd(out_fence_fd);
42694274
} else {
42704275
/* Link the fence with the FD created earlier */
42714276
fd_install(out_fence_fd, sync_file->file);
42724277
}
42734278
}
42744279

4275-
vmw_execbuf_copy_fence_user(dev_priv, vmw_fpriv(file_priv), ret,
4276-
user_fence_rep, fence, handle, out_fence_fd,
4277-
sync_file);
4278-
42794280
/* Don't unreference when handing fence out */
42804281
if (unlikely(out_fence != NULL)) {
42814282
*out_fence = fence;
@@ -4293,7 +4294,7 @@ int vmw_execbuf_process(struct drm_file *file_priv,
42934294
*/
42944295
vmw_validation_unref_lists(&val_ctx);
42954296

4296-
return 0;
4297+
return ret;
42974298

42984299
out_unlock_binding:
42994300
mutex_unlock(&dev_priv->binding_mutex);

drivers/gpu/drm/vmwgfx/vmwgfx_fence.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1128,7 +1128,7 @@ int vmw_fence_event_ioctl(struct drm_device *dev, void *data,
11281128
}
11291129

11301130
vmw_execbuf_copy_fence_user(dev_priv, vmw_fp, 0, user_fence_rep, fence,
1131-
handle, -1, NULL);
1131+
handle, -1);
11321132
vmw_fence_obj_unreference(&fence);
11331133
return 0;
11341134
out_no_create:

drivers/gpu/drm/vmwgfx/vmwgfx_kms.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2501,7 +2501,7 @@ void vmw_kms_helper_validation_finish(struct vmw_private *dev_priv,
25012501
if (file_priv)
25022502
vmw_execbuf_copy_fence_user(dev_priv, vmw_fpriv(file_priv),
25032503
ret, user_fence_rep, fence,
2504-
handle, -1, NULL);
2504+
handle, -1);
25052505
if (out_fence)
25062506
*out_fence = fence;
25072507
else

0 commit comments

Comments
 (0)