Skip to content

Commit

Permalink
drm/vmwgfx: Fix possible invalid drm gem put calls
Browse files Browse the repository at this point in the history
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: 9ef8d83 ("drm/vmwgfx: Do not drop the reference to the handle too soon")
Cc: <stable@vger.kernel.org> # v6.4+
Reviewed-by: Maaz Mombasawala<mombasawalam@vmware.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230818041301.407636-1-zack@kde.org
  • Loading branch information
Zack Rusin committed Aug 23, 2023
1 parent 14abdfa commit f9e96bf
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 16 deletions.
6 changes: 2 additions & 4 deletions drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down
6 changes: 2 additions & 4 deletions drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit f9e96bf

Please sign in to comment.