[SRU,Bionic,1/1] UBUNTU: SAUCE: drm/vmwgfx: Fix stale file descriptors on failed usercopy

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(-)

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
>