[prev in list] [next in list] [prev in thread] [next in thread]
List: dri-devel
Subject: Re: [PATCH] drm/vmwgfx: Fix possible invalid drm gem put calls
From: "Martin Krastev (VMware)" <martinkrastev768 () gmail ! com>
Date: 2023-08-18 11:53:43
Message-ID: 098d9ce4-965b-9376-e0ea-bb987c38f82c () gmail ! com
[Download RAW message or body]
From: Martin Krastev <krastevm@vmware.com>
LGTM!
Reviewed-by: Martin Krastev <krastevm@vmware.com>
Regards,
Martin
On 18 Aug 2023 04:13:14, Zack Rusin wrote:
>From: Zack Rusin <zackr@vmware.com>
>
>vmw_bo_unreference sets the input buffer to null on exit, resulting in
>null ptr deref's on the subsequent drm gem put calls.
>
>This went unnoticed because only very old userspace would be exercising
>those paths but it wouldn't be hard to hit on old distros with brand
>new kernels.
>
>Introduce a new function that abstracts unrefing of user bo's to make
>the code cleaner and more explicit.
>
>Signed-off-by: Zack Rusin <zackr@vmware.com>
>Reported-by: Ian Forbes <iforbes@vmware.com>
>Fixes: 9ef8d83e8e25 ("drm/vmwgfx: Do not drop the reference to the
handle too soon")
>Cc: <stable@vger.kernel.org> # v6.4+
>---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 6 ++----
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 8 ++++++++
> drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 6 ++----
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ++----
> drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 3 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 3 +--
> 6 files changed, 16 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
>index 82094c137855..c43853597776 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
>@@ -497,10 +497,9 @@ static int vmw_user_bo_synccpu_release(struct
drm_file *filp,
> if (!(flags & drm_vmw_synccpu_allow_cs)) {
> atomic_dec(&vmw_bo->cpu_writers);
> }
>- ttm_bo_put(&vmw_bo->tbo);
>+ vmw_user_bo_unref(vmw_bo);
> }
>
>- drm_gem_object_put(&vmw_bo->tbo.base);
> return ret;
> }
>
>@@ -540,8 +539,7 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device
*dev, void *data,
> return ret;
>
> ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
>- vmw_bo_unreference(&vbo);
>- drm_gem_object_put(&vbo->tbo.base);
>+ vmw_user_bo_unref(vbo);
> if (unlikely(ret != 0)) {
> if (ret == -ERESTARTSYS || ret == -EBUSY)
> return -EBUSY;
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
>index 50a836e70994..1d433fceed3d 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
>@@ -195,6 +195,14 @@ static inline struct vmw_bo
*vmw_bo_reference(struct vmw_bo *buf)
> return buf;
> }
>
>+static inline void vmw_user_bo_unref(struct vmw_bo *vbo)
>+{
>+ if (vbo) {
>+ ttm_bo_put(&vbo->tbo);
>+ drm_gem_object_put(&vbo->tbo.base);
>+ }
>+}
>+
> static inline struct vmw_bo *to_vmw_bo(struct drm_gem_object *gobj)
> {
> return container_of((gobj), struct vmw_bo, tbo.base);
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
>index 6b9aa2b4ef54..25b96821df0f 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
>@@ -1164,8 +1164,7 @@ static int vmw_translate_mob_ptr(struct
vmw_private *dev_priv,
> }
> vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_MOB,
VMW_BO_DOMAIN_MOB);
> ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
>- ttm_bo_put(&vmw_bo->tbo);
>- drm_gem_object_put(&vmw_bo->tbo.base);
>+ vmw_user_bo_unref(vmw_bo);
> if (unlikely(ret != 0))
> return ret;
>
>@@ -1221,8 +1220,7 @@ static int vmw_translate_guest_ptr(struct
vmw_private *dev_priv,
> vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_GMR |
VMW_BO_DOMAIN_VRAM,
> VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM);
> ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
>- ttm_bo_put(&vmw_bo->tbo);
>- drm_gem_object_put(&vmw_bo->tbo.base);
>+ vmw_user_bo_unref(vmw_bo);
> if (unlikely(ret != 0))
> return ret;
>
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>index b62207be3363..1489ad73c103 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>@@ -1665,10 +1665,8 @@ static struct drm_framebuffer
*vmw_kms_fb_create(struct drm_device *dev,
>
> err_out:
> /* vmw_user_lookup_handle takes one ref so does new_fb */
>- if (bo) {
>- vmw_bo_unreference(&bo);
>- drm_gem_object_put(&bo->tbo.base);
>- }
>+ if (bo)
>+ vmw_user_bo_unref(bo);
> if (surface)
> vmw_surface_unreference(&surface);
>
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
>index 7e112319a23c..fb85f244c3d0 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
>@@ -451,8 +451,7 @@ int vmw_overlay_ioctl(struct drm_device *dev, void
*data,
>
> ret = vmw_overlay_update_stream(dev_priv, buf, arg, true);
>
>- vmw_bo_unreference(&buf);
>- drm_gem_object_put(&buf->tbo.base);
>+ vmw_user_bo_unref(buf);
>
> out_unlock:
> mutex_unlock(&overlay->mutex);
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
>index e7226db8b242..1e81ff2422cf 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
>@@ -809,8 +809,7 @@ static int vmw_shader_define(struct drm_device
*dev, struct drm_file *file_priv,
> shader_type, num_input_sig,
> num_output_sig, tfile,
shader_handle);
> out_bad_arg:
>- vmw_bo_unreference(&buffer);
>- drm_gem_object_put(&buffer->tbo.base);
>+ vmw_user_bo_unref(buffer);
> return ret;
> }
>
>--
>2.39.2
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic