Skip to contentSkip to conversation

Patch "drm/vmwgfx: Fix a destoy-while-held mutex problem." has been added to the 4.9-stable tree

PATCH [email protected] 1 participant 1 message Mar 25, 2018

Subsystems (4)
Stable branch DRM drivers DRM misc DRM driver for VMware virtual GPU
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

drm/vmwgfx: Fix a destoy-while-held mutex problem.

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
drm-vmwgfx-fix-a-destoy-while-held-mutex-problem.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 73a88250b70954a8f27c2444e1c2411bba3c29d9 Mon Sep 17 00:00:00 2001

From: Thomas Hellstrom <[email protected]>
Date: Wed, 21 Mar 2018 10:18:38 +0100
Subject: drm/vmwgfx: Fix a destoy-while-held mutex problem.

From: Thomas Hellstrom <[email protected]>

commit 73a88250b70954a8f27c2444e1c2411bba3c29d9 upstream.

When validating legacy surfaces, the backup bo might be destroyed at surface validate time. However, the kms resource validation code may have the bo reserved, so we will destroy a locked mutex. While there shouldn't be any other users of that mutex when it is destroyed, it causes a lock leak and thus throws a lockdep error.

Fix this by having the kms resource validation code hold a reference to the bo while we have it reserved. We do this by introducing a validation context which might come in handy when the kms code is extended to validate multiple resources or buffers.

Cc <[email protected]>
Signed-off-by Thomas Hellstrom <[email protected]>
Reviewed-by Brian Paul <[email protected]>
Reviewed-by Sinclair Yeh <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |   28 +++++++++++++++++++---------
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |   12 +++++++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |    5 +++--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |    5 +++--
 4 files changed, 34 insertions(+), 16 deletions(-)
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +19 -9
@@ -27,7 +27,6 @@
 
 #include "vmwgfx_kms.h"
 
-
 /* Might need a hrtimer here? */
 #define VMWGFX_PRESENT_RATE ((HZ / 60 > 0) ? HZ / 60 : 1)
 

@@ -1933,9 +1932,12 @@ void vmw_kms_helper_buffer_finish(struct
  * Helper to be used if an error forces the caller to undo the actions of
  * vmw_kms_helper_resource_prepare.
  */
-void vmw_kms_helper_resource_revert(struct vmw_resource *res)
+void vmw_kms_helper_resource_revert(struct vmw_validation_ctx *ctx)
 {
-	vmw_kms_helper_buffer_revert(res->backup);
+	struct vmw_resource *res = ctx->res;
+
+	vmw_kms_helper_buffer_revert(ctx->buf);
+	vmw_dmabuf_unreference(&ctx->buf);
 	vmw_resource_unreserve(res, false, NULL, 0);
 	mutex_unlock(&res->dev_priv->cmdbuf_mutex);
 }

@@ -1952,10 +1954,14 @@ void vmw_kms_helper_resource_revert(stru
  * interrupted by a signal.
  */
 int vmw_kms_helper_resource_prepare(struct vmw_resource *res,
-				    bool interruptible)
+				    bool interruptible,
+				    struct vmw_validation_ctx *ctx)
 {
 	int ret = 0;
 
+	ctx->buf = NULL;
+	ctx->res = res;
+
 	if (interruptible)
 		ret = mutex_lock_interruptible(&res->dev_priv->cmdbuf_mutex);
 	else

@@ -1974,6 +1980,8 @@ int vmw_kms_helper_resource_prepare(stru
 						    res->dev_priv->has_mob);
 		if (ret)
 			goto out_unreserve;
+
+		ctx->buf = vmw_dmabuf_reference(res->backup);
 	}
 	ret = vmw_resource_validate(res);
 	if (ret)

@@ -1981,7 +1989,7 @@ int vmw_kms_helper_resource_prepare(stru
 	return 0;
 
 out_revert:
-	vmw_kms_helper_buffer_revert(res->backup);
+	vmw_kms_helper_buffer_revert(ctx->buf);
 out_unreserve:
 	vmw_resource_unreserve(res, false, NULL, 0);
 out_unlock:

@@ -1997,11 +2005,13 @@ out_unlock:
  * @out_fence: Optional pointer to a fence pointer. If non-NULL, a
  * ref-counted fence pointer is returned here.
  */
-void vmw_kms_helper_resource_finish(struct vmw_resource *res,
-			     struct vmw_fence_obj **out_fence)
+void vmw_kms_helper_resource_finish(struct vmw_validation_ctx *ctx,
+				    struct vmw_fence_obj **out_fence)
 {
-	if (res->backup || out_fence)
-		vmw_kms_helper_buffer_finish(res->dev_priv, NULL, res->backup,
+	struct vmw_resource *res = ctx->res;
+
+	if (ctx->buf || out_fence)
+		vmw_kms_helper_buffer_finish(res->dev_priv, NULL, ctx->buf,
 					     out_fence, NULL);
 
 	vmw_resource_unreserve(res, false, NULL, 0);
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h +9 -3
@@ -183,6 +183,11 @@ struct vmw_display_unit {
 	int set_gui_y;
 };
 
+struct vmw_validation_ctx {
+	struct vmw_resource *res;
+	struct vmw_dma_buffer *buf;
+};
+
 #define vmw_crtc_to_du(x) \
 	container_of(x, struct vmw_display_unit, crtc)
 #define vmw_connector_to_du(x) \

@@ -233,9 +238,10 @@ void vmw_kms_helper_buffer_finish(struct
 				  struct drm_vmw_fence_rep __user *
 				  user_fence_rep);
 int vmw_kms_helper_resource_prepare(struct vmw_resource *res,
-				    bool interruptible);
-void vmw_kms_helper_resource_revert(struct vmw_resource *res);
-void vmw_kms_helper_resource_finish(struct vmw_resource *res,
+				    bool interruptible,
+				    struct vmw_validation_ctx *ctx);
+void vmw_kms_helper_resource_revert(struct vmw_validation_ctx *ctx);
+void vmw_kms_helper_resource_finish(struct vmw_validation_ctx *ctx,
 				    struct vmw_fence_obj **out_fence);
 int vmw_kms_readback(struct vmw_private *dev_priv,
 		     struct drm_file *file_priv,
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c +3 -2
@@ -753,12 +753,13 @@ int vmw_kms_sou_do_surface_dirty(struct
 	struct vmw_framebuffer_surface *vfbs =
 		container_of(framebuffer, typeof(*vfbs), base);
 	struct vmw_kms_sou_surface_dirty sdirty;
+	struct vmw_validation_ctx ctx;
 	int ret;
 
 	if (!srf)
 		srf = &vfbs->surface->res;
 
-	ret = vmw_kms_helper_resource_prepare(srf, true);
+	ret = vmw_kms_helper_resource_prepare(srf, true, &ctx);
 	if (ret)
 		return ret;
 

@@ -777,7 +778,7 @@ int vmw_kms_sou_do_surface_dirty(struct
 	ret = vmw_kms_helper_dirty(dev_priv, framebuffer, clips, vclips,
 				   dest_x, dest_y, num_clips, inc,
 				   &sdirty.base);
-	vmw_kms_helper_resource_finish(srf, out_fence);
+	vmw_kms_helper_resource_finish(&ctx, out_fence);
 
 	return ret;
 }
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +3 -2
@@ -977,12 +977,13 @@ int vmw_kms_stdu_surface_dirty(struct vm
 	struct vmw_framebuffer_surface *vfbs =
 		container_of(framebuffer, typeof(*vfbs), base);
 	struct vmw_stdu_dirty sdirty;
+	struct vmw_validation_ctx ctx;
 	int ret;
 
 	if (!srf)
 		srf = &vfbs->surface->res;
 
-	ret = vmw_kms_helper_resource_prepare(srf, true);
+	ret = vmw_kms_helper_resource_prepare(srf, true, &ctx);
 	if (ret)
 		return ret;
 

@@ -1005,7 +1006,7 @@ int vmw_kms_stdu_surface_dirty(struct vm
 				   dest_x, dest_y, num_clips, inc,
 				   &sdirty.base);
 out_finish:
-	vmw_kms_helper_resource_finish(srf, out_fence);
+	vmw_kms_helper_resource_finish(&ctx, out_fence);
 
 	return ret;
 }

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/drm-vmwgfx-fix-a-destoy-while-held-mutex-problem.patch