From b26629453c7b2a6c82000b36fbd1cfc4d9101808 Mon Sep 17 00:00:00 2001 From: Murray McAllister Date: Mon, 27 Mar 2017 11:12:53 +0200 Subject: drm/vmwgfx: NULL pointer dereference in vmw_surface_define_ioctl() commit 36274ab8c596f1240c606bb514da329add2a1bcd upstream. Before memory allocations vmw_surface_define_ioctl() checks the upper-bounds of a user-supplied size, but does not check if the supplied size is 0. Add check to avoid NULL pointer dereferences. Signed-off-by: Murray McAllister Reviewed-by: Sinclair Yeh Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_surface.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index 7d620e82e000..b363f0be6512 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -718,8 +718,8 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, for (i = 0; i < DRM_VMW_MAX_SURFACE_FACES; ++i) num_sizes += req->mip_levels[i]; - if (num_sizes > DRM_VMW_MAX_SURFACE_FACES * - DRM_VMW_MAX_MIP_LEVELS) + if (num_sizes > DRM_VMW_MAX_SURFACE_FACES * DRM_VMW_MAX_MIP_LEVELS || + num_sizes == 0) return -EINVAL; size = vmw_user_surface_size + 128 + -- cgit v1.2.3 From ad4ae2feef4f65b860f139e0d8455e2a16efb93c Mon Sep 17 00:00:00 2001 From: Thomas Hellstrom Date: Mon, 27 Mar 2017 11:21:25 +0200 Subject: drm/ttm, drm/vmwgfx: Relax permission checking when opening surfaces commit fe25deb7737ce6c0879ccf79c99fa1221d428bf2 upstream. Previously, when a surface was opened using a legacy (non prime) handle, it was verified to have been created by a client in the same master realm. Relax this so that opening is also allowed recursively if the client already has the surface open. This works around a regression in svga mesa where opening of a shared surface is used recursively to obtain surface information. Signed-off-by: Thomas Hellstrom Reviewed-by: Sinclair Yeh Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_surface.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index b363f0be6512..79f78a68d92d 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -904,17 +904,16 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv, uint32_t handle; struct ttm_base_object *base; int ret; + bool require_exist = false; if (handle_type == DRM_VMW_HANDLE_PRIME) { ret = ttm_prime_fd_to_handle(tfile, u_handle, &handle); if (unlikely(ret != 0)) return ret; } else { - if (unlikely(drm_is_render_client(file_priv))) { - DRM_ERROR("Render client refused legacy " - "surface reference.\n"); - return -EACCES; - } + if (unlikely(drm_is_render_client(file_priv))) + require_exist = true; + if (ACCESS_ONCE(vmw_fpriv(file_priv)->locked_master)) { DRM_ERROR("Locked master refused legacy " "surface reference.\n"); @@ -942,17 +941,14 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv, /* * Make sure the surface creator has the same - * authenticating master. + * authenticating master, or is already registered with us. */ if (drm_is_primary_client(file_priv) && - user_srf->master != file_priv->master) { - DRM_ERROR("Trying to reference surface outside of" - " master domain.\n"); - ret = -EACCES; - goto out_bad_resource; - } + user_srf->master != file_priv->master) + require_exist = true; - ret = ttm_ref_object_add(tfile, base, TTM_REF_USAGE, NULL); + ret = ttm_ref_object_add(tfile, base, TTM_REF_USAGE, NULL, + require_exist); if (unlikely(ret != 0)) { DRM_ERROR("Could not add a reference to a surface.\n"); goto out_bad_resource; -- cgit v1.2.3 From c21636bd64c511160846bdf87ef4c7ff48680c99 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 27 Mar 2017 20:10:53 -0700 Subject: drm/vmwgfx: fix integer overflow in vmw_surface_define_ioctl() commit e7e11f99564222d82f0ce84bd521e57d78a6b678 upstream. In vmw_surface_define_ioctl(), the 'num_sizes' is the sum of the 'req->mip_levels' array. This array can be assigned any value from the user space. As both the 'num_sizes' and the array is uint32_t, it is easy to make 'num_sizes' overflow. The later 'mip_levels' is used as the loop count. This can lead an oob write. Add the check of 'req->mip_levels' to avoid this. Signed-off-by: Li Qiang Reviewed-by: Thomas Hellstrom Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_surface.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index 79f78a68d92d..c9c04ccccdd9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -715,8 +715,11 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, 128; num_sizes = 0; - for (i = 0; i < DRM_VMW_MAX_SURFACE_FACES; ++i) + for (i = 0; i < DRM_VMW_MAX_SURFACE_FACES; ++i) { + if (req->mip_levels[i] > DRM_VMW_MAX_MIP_LEVELS) + return -EINVAL; num_sizes += req->mip_levels[i]; + } if (num_sizes > DRM_VMW_MAX_SURFACE_FACES * DRM_VMW_MAX_MIP_LEVELS || num_sizes == 0) -- cgit v1.2.3 From 619cc02fd85d69270c95e7db320b305e975aae00 Mon Sep 17 00:00:00 2001 From: Vladis Dronov Date: Fri, 2 Jun 2017 07:42:09 +0200 Subject: drm/vmwgfx: limit the number of mip levels in vmw_gb_surface_define_ioctl() commit ee9c4e681ec4f58e42a83cb0c22a0289ade1aacf upstream. The 'req->mip_levels' parameter in vmw_gb_surface_define_ioctl() is a user-controlled 'uint32_t' value which is used as a loop count limit. This can lead to a kernel lockup and DoS. Add check for 'req->mip_levels'. References: https://bugzilla.redhat.com/show_bug.cgi?id=1437431 Signed-off-by: Vladis Dronov Reviewed-by: Sinclair Yeh Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_surface.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index c9c04ccccdd9..99b618ac2af6 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -1293,6 +1293,9 @@ int vmw_gb_surface_define_ioctl(struct drm_device *dev, void *data, if (req->multisample_count != 0) return -EINVAL; + if (req->mip_levels > DRM_VMW_MAX_MIP_LEVELS) + return -EINVAL; + if (unlikely(vmw_user_surface_size == 0)) vmw_user_surface_size = ttm_round_pot(sizeof(*user_srf)) + 128; -- cgit v1.2.3 From 74276868b45585f77eae2372c95e016aacfd0d3c Mon Sep 17 00:00:00 2001 From: Sinclair Yeh Date: Fri, 2 Jun 2017 07:50:57 +0200 Subject: drm/vmwgfx: Make sure backup_handle is always valid commit 07678eca2cf9c9a18584e546c2b2a0d0c9a3150c upstream. When vmw_gb_surface_define_ioctl() is called with an existing buffer, we end up returning an uninitialized variable in the backup_handle. The fix is to first initialize backup_handle to 0 just to be sure, and second, when a user-provided buffer is found, we will use the req->buffer_handle as the backup_handle. Reported-by: Murray McAllister Signed-off-by: Sinclair Yeh Reviewed-by: Deepak Rawat Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_surface.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index 99b618ac2af6..027987023400 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -1288,7 +1288,7 @@ int vmw_gb_surface_define_ioctl(struct drm_device *dev, void *data, struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile; int ret; uint32_t size; - uint32_t backup_handle; + uint32_t backup_handle = 0; if (req->multisample_count != 0) return -EINVAL; @@ -1331,12 +1331,16 @@ int vmw_gb_surface_define_ioctl(struct drm_device *dev, void *data, ret = vmw_user_dmabuf_lookup(tfile, req->buffer_handle, &res->backup, &user_srf->backup_base); - if (ret == 0 && res->backup->base.num_pages * PAGE_SIZE < - res->backup_size) { - DRM_ERROR("Surface backup buffer is too small.\n"); - vmw_dmabuf_unreference(&res->backup); - ret = -EINVAL; - goto out_unlock; + if (ret == 0) { + if (res->backup->base.num_pages * PAGE_SIZE < + res->backup_size) { + DRM_ERROR("Surface backup buffer is too small.\n"); + vmw_dmabuf_unreference(&res->backup); + ret = -EINVAL; + goto out_unlock; + } else { + backup_handle = req->buffer_handle; + } } } else if (req->drm_surface_flags & drm_vmw_surface_flag_create_buffer) ret = vmw_user_dmabuf_alloc(dev_priv, tfile, -- cgit v1.2.3