From patchwork Thu Nov 15 13:04:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 1748981 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 9D81A3FC8A for ; Thu, 15 Nov 2012 12:59:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4DE229F611 for ; Thu, 15 Nov 2012 04:59:48 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49]) by gabe.freedesktop.org (Postfix) with ESMTP id A43399E852 for ; Thu, 15 Nov 2012 04:59:35 -0800 (PST) Received: by mail-ee0-f49.google.com with SMTP id c1so933982eek.36 for ; Thu, 15 Nov 2012 04:59:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer; bh=+bc1WNzO4O4G6Nng+ZzwzVFjkGe0UkzHH5WQAoq+oGo=; b=dAisGkPnuQxolBrcuW1U1bA2YME4rFBaw7ByMkPtRTfD6MM8ql50E+rE03VeaLWvUt DC1Sps6vztveW1AgklwjYrM4AeIU6rKSRvtw9JxB06FRi664fjtMqELvvnBEuwCRIB5a tDYHCJL2ZNFRwLI7n2LLG5dlkZh5gEDuzZ3T/XgMJipwS0MiG4LiWOHaK1Xl5DbDay8w mbru7Y4gLJcJ0j7kjL0fXXTjf2BtPSjIRlK0UIZhIGpPpQT/oZwwxUhWaYAvwB9toH3p M1yn4Gtm+/XkjrA4L7Ik4N61/Rn/IGsiHAxsspghUbf19FharVvxRAIq8EcU52Fj8XFR 0N4w== Received: by 10.14.221.8 with SMTP id q8mr3553626eep.28.1352984374790; Thu, 15 Nov 2012 04:59:34 -0800 (PST) Received: from david-nb.uni-tuebingen.de (u-087-c115.eap.uni-tuebingen.de. [134.2.87.115]) by mx.google.com with ESMTPS id z43sm36084851een.16.2012.11.15.04.59.33 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 15 Nov 2012 04:59:33 -0800 (PST) From: David Herrmann To: dri-devel@lists.freedesktop.org Subject: [PATCH RESEND] drm: fix returning -EINVAL on setmaster if another master is active Date: Thu, 15 Nov 2012 14:04:37 +0100 Message-Id: <1352984677-14565-1-git-send-email-dh.herrmann@googlemail.com> X-Mailer: git-send-email 1.8.0 Cc: Kristian Hoegsberg X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org 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 --- 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; }