Message ID | 20220127205812.34060-2-cascardo@canonical.com |
---|---|
State | New |
Headers | show |
Series | CVE-2022-22942 | expand |
On Thu, Jan 27, 2022 at 12:59 PM Thadeu Lima de Souza Cascardo < cascardo@canonical.com> wrote: > From: Mathias Krause <minipli@grsecurity.net> > > 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. > > Cc: Zack Rusin <zackr@vmware.com> > Fixes: c906965dee22 ("drm/vmwgfx: Add export fence to file descriptor > support") > [mks: backport to v4.19 and older] > Signed-off-by: Mathias Krause <minipli@grsecurity.net> > CVE-2022-22942 > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 5 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 34 ++++++++++++------------- > drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- > 4 files changed, 21 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index 8c65cc3b0dda..8f5321f09856 100644 > --- 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 dc677ba4dc38..996696ad6f98 100644 > --- 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 55214d0da66e..7c92d369d544 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > @@ -1151,7 +1151,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 406f8e82a4d6..f1abdb135c86 100644 > --- 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 > -- > 2.32.0 > > > Applied to Bionic:linux/master-next. Thanks :) - Luke > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team >
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 8c65cc3b0dda..8f5321f09856 100644 --- 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 dc677ba4dc38..996696ad6f98 100644 --- 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 55214d0da66e..7c92d369d544 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c @@ -1151,7 +1151,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 406f8e82a4d6..f1abdb135c86 100644 --- 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