[PATCH] drm: fix returning -EINVAL on setmaster if another master is active
David Herrmann
dh.herrmann at googlemail.com
Sun Oct 7 10:53:26 PDT 2012
We link every DRM "file_priv" to a "drm_master" structure. Currently, the
drmSetMaster() call returns 0 when there is _any_ active master associated
with the "drm_master" structure of the calling "file_priv". This means,
that after drmSetMaster() we are not guaranteed to be DRM-Master and might
not be able to perform mode-setting.
A way to reproduce this is by starting weston with the DRM backend from
within an X-console (eg., xterm). Because the xserver's "drm_master" is
currently active, weston is assigned to the same master but is inactive
because its VT is inactive and the xserver is still active. But when
"fake-activating" weston, it calls drmSetMaster(). With current behavior
this returns "0/success" and weston thinks that it is DRM-Master, even
though it is not (as the xserver is still DRM-Master).
Expected behavior would be drmSetMaster() to return -EINVAL, because the
xserver is still DRM-Master. This patch changes exactly that.
The only way this bogus behavior would be useful is for clients to check
whether their associated "drm_master" is currently the active DRM-Master.
But this logic fails if no DRM-Master is currently active at all. Because
then the client itself would become DRM-Master (if it is root) and this
makes this whole thing useles.
Also note that the second "if-condition":
file_priv->minor->master != file_priv->master
is always true and can be skipped.
Signed-off-by: David Herrmann <dh.herrmann at googlemail.com>
---
Note:
Note that this only removes the "if-clause". The code that performs the
setmaster() is actually left unchanged but makes the patch look scarier than it
really is.
drivers/gpu/drm/drm_stub.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index c236fd2..581e61d 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -221,20 +221,20 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
if (!file_priv->master)
return -EINVAL;
- if (!file_priv->minor->master &&
- file_priv->minor->master != file_priv->master) {
- mutex_lock(&dev->struct_mutex);
- file_priv->minor->master = drm_master_get(file_priv->master);
- file_priv->is_master = 1;
- if (dev->driver->master_set) {
- ret = dev->driver->master_set(dev, file_priv, false);
- if (unlikely(ret != 0)) {
- file_priv->is_master = 0;
- drm_master_put(&file_priv->minor->master);
- }
+ if (file_priv->minor->master)
+ return -EINVAL;
+
+ mutex_lock(&dev->struct_mutex);
+ file_priv->minor->master = drm_master_get(file_priv->master);
+ file_priv->is_master = 1;
+ if (dev->driver->master_set) {
+ ret = dev->driver->master_set(dev, file_priv, false);
+ if (unlikely(ret != 0)) {
+ file_priv->is_master = 0;
+ drm_master_put(&file_priv->minor->master);
}
- mutex_unlock(&dev->struct_mutex);
}
+ mutex_unlock(&dev->struct_mutex);
return 0;
}
--
1.7.12.2
More information about the dri-devel
mailing list