From patchwork Sun Oct 7 17:53:26 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 1561921 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 9E9163FD9C for ; Sun, 7 Oct 2012 17:50:49 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DDD2E9E9D1 for ; Sun, 7 Oct 2012 10:50:48 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 0AC9A9E759 for ; Sun, 7 Oct 2012 10:50:36 -0700 (PDT) Received: by mail-we0-f177.google.com with SMTP id u50so2204698wey.36 for ; Sun, 07 Oct 2012 10:50:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer; bh=+bc1WNzO4O4G6Nng+ZzwzVFjkGe0UkzHH5WQAoq+oGo=; b=pSYBaNY7CfTCsIm1P3YbGhz+tHv/7wGxhXIGHdPwU86RMHhtk5zpWR8c/Pb0dfHr6P LpFsXaaPjdCswnTnIIsSXEf4IM5ehWP/7YXH2zy3bkdOYutxF1cs0lCtOEvQB+BZIfzO XV75Wh5/+VFfqPD+GffdvkSL53bANhwhmbXwyQu61v3tCn6TvZNaNZxjGuYg+fjLlU95 8fla7mcU5Y7yH0bdzezVNTwIkrVGEykRKqYGKuw4ZNxjo3W4T/TAIjLw46/ehKt1GYr6 VCp9Em2jI2vF4ps790/oDF10+oPkCKjHsCJTkXeC8I0fS/PIStx128PJMZuCWr6qTBWh Ic5A== Received: by 10.216.195.25 with SMTP id o25mr7916976wen.6.1349632236001; Sun, 07 Oct 2012 10:50:36 -0700 (PDT) Received: from localhost.localdomain (stgt-5f718006.pool.mediaWays.net. [95.113.128.6]) by mx.google.com with ESMTPS id w8sm15094036wif.4.2012.10.07.10.50.34 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 07 Oct 2012 10:50:35 -0700 (PDT) From: David Herrmann To: dri-devel@lists.freedesktop.org Subject: [PATCH] drm: fix returning -EINVAL on setmaster if another master is active Date: Sun, 7 Oct 2012 19:53:26 +0200 Message-Id: <1349632406-24068-1-git-send-email-dh.herrmann@googlemail.com> X-Mailer: git-send-email 1.7.12.2 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; }