[PATCH v3] drm/fb-helper: Avoid race with DRM userspace

Noralf Trønnes noralf at tronnes.org
Fri Apr 26 21:03:20 UTC 2019


drm_fb_helper_is_bound() is used to check if DRM userspace is in control.
This is done by looking at the fb on the primary plane. By the time
fb-helper gets around to committing, it's possible that the facts have
changed.

Avoid this race by holding the drm_device->master_mutex lock while
committing. When DRM userspace does its first open, it will now wait
until fb-helper is done. The helper will stay away if there's a master.

Locking rule: Always take the fb-helper lock first.

v2:
- Remove drm_fb_helper_is_bound() (Daniel Vetter)
- No need to check fb_helper->dev->master in
  drm_fb_helper_single_fb_probe(), restore_fbdev_mode() has the check.

Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/drm_auth.c      | 20 ++++++++++++++++++++
 drivers/gpu/drm/drm_fb_helper.c |  7 +++++--
 drivers/gpu/drm/drm_internal.h  |  2 ++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 1669c42c40ed..db199807b7dc 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -368,3 +368,23 @@ void drm_master_put(struct drm_master **master)
 	*master = NULL;
 }
 EXPORT_SYMBOL(drm_master_put);
+
+/* Used by drm_client and drm_fb_helper */
+bool drm_master_internal_acquire(struct drm_device *dev)
+{
+	mutex_lock(&dev->master_mutex);
+	if (dev->master) {
+		mutex_unlock(&dev->master_mutex);
+		return false;
+	}
+
+	return true;
+}
+EXPORT_SYMBOL(drm_master_internal_acquire);
+
+/* Used by drm_client and drm_fb_helper */
+void drm_master_internal_release(struct drm_device *dev)
+{
+	mutex_unlock(&dev->master_mutex);
+}
+EXPORT_SYMBOL(drm_master_internal_release);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 2339f0f8f5a8..b1fe6e36f51f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -44,6 +44,7 @@
 
 #include "drm_crtc_internal.h"
 #include "drm_crtc_helper_internal.h"
+#include "drm_internal.h"
 
 static bool drm_fbdev_emulation = true;
 module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
@@ -1548,12 +1549,13 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 			unsigned long arg)
 {
 	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_device *dev = fb_helper->dev;
 	struct drm_mode_set *mode_set;
 	struct drm_crtc *crtc;
 	int ret = 0;
 
 	mutex_lock(&fb_helper->lock);
-	if (!drm_fb_helper_is_bound(fb_helper)) {
+	if (!drm_master_internal_acquire(dev)) {
 		ret = -EBUSY;
 		goto unlock;
 	}
@@ -1591,11 +1593,12 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 		}
 
 		ret = 0;
-		goto unlock;
+		break;
 	default:
 		ret = -ENOTTY;
 	}
 
+	drm_master_internal_release(dev);
 unlock:
 	mutex_unlock(&fb_helper->lock);
 	return ret;
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index d9a483a5fce0..3ee97c9998a2 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -91,6 +91,8 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv);
 int drm_master_open(struct drm_file *file_priv);
 void drm_master_release(struct drm_file *file_priv);
+bool drm_master_internal_acquire(struct drm_device *dev);
+void drm_master_internal_release(struct drm_device *dev);
 
 /* drm_sysfs.c */
 extern struct class *drm_class;
-- 
2.20.1



More information about the Intel-gfx-trybot mailing list