Received: by 2002:a05:6a10:eb17:0:0:0:0 with SMTP id hx23csp444921pxb; Thu, 9 Sep 2021 04:45:39 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz75/o5e1TmaZ7DM1oj/syBmqmpmd+Fr6CzU2GWNda3JPfqlLOS7t/Ll5+K5j22d4nIg8hG X-Received: by 2002:a17:906:4784:: with SMTP id cw4mr2999344ejc.160.1631187939471; Thu, 09 Sep 2021 04:45:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1631187939; cv=none; d=google.com; s=arc-20160816; b=KR6hmXWEK3mcyqxGjhRcmetmxXB869pjZ4VjQsJEyZ0jE498n7jXiBZFtotbp8X2KS 1jW3F/y91HdBgW1Q9zkMomuaihWsa2wCTnO4BQxS4DVps252MkAttoZO8mVMXYcsVC0z uKFBLkabazRFmxJzQYqjR2JV3SsLK4NfThPgRpWNXFzOxem805wNS9vOf5ouhfmGRHOn Z7cFvLSEDUlEByrOE4mXNfb1stCB9p47Fpu08L9vnyV933xgRDXAIY4Jzg44XlMS6rP+ cOyzYMPATf9dDW5BAPEDaJrlwK2O5VNhMHlrIOb1+x+9Ohv3RQJNfYuX7mjR2zDurkh9 KGVA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=MQsP4QazT2T8jp93oL9bU/C6XTf98/on6LEn3POd6Yw=; b=JXIPZ9Q+t1dHUKOjXN95rhgT8Vmv4I1OutbqCx8rFu7nNYReBqwj0IVKMTK0v59x6a 6StrNeca8ntfSFf92wu2KaO88COXGrMankXd0e/Tqp6Nx+mB/exrSR3yW6DXfJkKpX0N s657/V+wJkoxgv3ZLo9jKsRi58e+0kbtjS1fbk8cFD3oRiAXmq6XM6bGyvpoUCF/woum gTe6b3zaZ/OBQhtvqwG/SCmJeCGGctXt4ISGJiW5nYvyGDmeBylN9jQFnKvQXM4T0HHW 6zrbqsSpkSKSBbXqqFfjUfXeiNPXOnJaE95Er+Tph3LNoAPSOUb+oDO9xtejfD9Clg3n i2oA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=L+itrMWP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id qk42si1531185ejc.171.2021.09.09.04.45.15; Thu, 09 Sep 2021 04:45:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=L+itrMWP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237805AbhIILow (ORCPT + 99 others); Thu, 9 Sep 2021 07:44:52 -0400 Received: from mail.kernel.org ([198.145.29.99]:46772 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236219AbhIILnB (ORCPT ); Thu, 9 Sep 2021 07:43:01 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 35DFA611C2; Thu, 9 Sep 2021 11:41:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1631187711; bh=XyWHhv3636q/zbngEfCAjfVmU9xHd2U25U/a7JN0HHA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=L+itrMWPsk0a4FKDsPZoCCyAtg/Sp0H5swq6+mvXXK4VDL0FDALA3ZtFDinwd3dWz lJzUXvJzo/Olu6mt9y2kV1EHE2VrOLJky5EL/yFg2r0OkoKPe6JLmxJVI2JQWH65CO wAOJHpMK9ayXlzz51vDI31jssUifbnNcqH7dqBqgAihF9u9G/kWquhRHHfKLgfwLCJ Fd66OwaYfSOZiygBgzafbQyJBlEmguYMbVlM7G/ZuIhyaLmlO8pqmKYRHKNRTqFbuQ p7GoZ2tc2jqTD5eT1zjZt/G4Pvct6islG89fjUnrLQ8jdfG2omM+k7ne0YCBSQJUqr UrIL1XJGbVDvw== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Desmond Cheong Zhi Xi , Daniel Vetter , Sasha Levin , dri-devel@lists.freedesktop.org Subject: [PATCH AUTOSEL 5.14 035/252] drm: serialize drm_file.master with a new spinlock Date: Thu, 9 Sep 2021 07:37:29 -0400 Message-Id: <20210909114106.141462-35-sashal@kernel.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210909114106.141462-1-sashal@kernel.org> References: <20210909114106.141462-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Desmond Cheong Zhi Xi [ Upstream commit 0b0860a3cf5eccf183760b1177a1dcdb821b0b66 ] Currently, drm_file.master pointers should be protected by drm_device.master_mutex when being dereferenced. This is because drm_file.master is not invariant for the lifetime of drm_file. If drm_file is not the creator of master, then drm_file.is_master is false, and a call to drm_setmaster_ioctl will invoke drm_new_set_master, which then allocates a new master for drm_file and puts the old master. Thus, without holding drm_device.master_mutex, the old value of drm_file.master could be freed while it is being used by another concurrent process. However, it is not always possible to lock drm_device.master_mutex to dereference drm_file.master. Through the fbdev emulation code, this might occur in a deep nest of other locks. But drm_device.master_mutex is also the outermost lock in the nesting hierarchy, so this leads to potential deadlocks. To address this, we introduce a new spin lock at the bottom of the lock hierarchy that only serializes drm_file.master. With this change, the value of drm_file.master changes only when both drm_device.master_mutex and drm_file.master_lookup_lock are held. Hence, any process holding either of those locks can ensure that the value of drm_file.master will not change concurrently. Since no lock depends on the new drm_file.master_lookup_lock, when drm_file.master is dereferenced, but drm_device.master_mutex cannot be held, we can safely protect the master pointer with drm_file.master_lookup_lock. Reported-by: Daniel Vetter Signed-off-by: Desmond Cheong Zhi Xi Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20210712043508.11584-5-desmondcheongzx@gmail.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/drm_auth.c | 17 +++++++++++------ drivers/gpu/drm/drm_file.c | 1 + include/drm/drm_file.h | 12 +++++++++--- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index b59b26a71ad5..cbb896b91d94 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -135,16 +135,18 @@ static void drm_set_master(struct drm_device *dev, struct drm_file *fpriv, static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) { struct drm_master *old_master; + struct drm_master *new_master; lockdep_assert_held_once(&dev->master_mutex); WARN_ON(fpriv->is_master); old_master = fpriv->master; - fpriv->master = drm_master_create(dev); - if (!fpriv->master) { - fpriv->master = old_master; + new_master = drm_master_create(dev); + if (!new_master) return -ENOMEM; - } + spin_lock(&fpriv->master_lookup_lock); + fpriv->master = new_master; + spin_unlock(&fpriv->master_lookup_lock); fpriv->is_master = 1; fpriv->authenticated = 1; @@ -303,10 +305,13 @@ int drm_master_open(struct drm_file *file_priv) * any master object for render clients */ mutex_lock(&dev->master_mutex); - if (!dev->master) + if (!dev->master) { ret = drm_new_set_master(dev, file_priv); - else + } else { + spin_lock(&file_priv->master_lookup_lock); file_priv->master = drm_master_get(dev->master); + spin_unlock(&file_priv->master_lookup_lock); + } mutex_unlock(&dev->master_mutex); return ret; diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index d4f0bac6f8f8..ceb1a9723855 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -176,6 +176,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) init_waitqueue_head(&file->event_wait); file->event_space = 4096; /* set aside 4k for event buffer */ + spin_lock_init(&file->master_lookup_lock); mutex_init(&file->event_read_lock); if (drm_core_check_feature(dev, DRIVER_GEM)) diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index b81b3bfb08c8..9b82988e3427 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -226,15 +226,21 @@ struct drm_file { /** * @master: * - * Master this node is currently associated with. Only relevant if - * drm_is_primary_client() returns true. Note that this only - * matches &drm_device.master if the master is the currently active one. + * Master this node is currently associated with. Protected by struct + * &drm_device.master_mutex, and serialized by @master_lookup_lock. + * + * Only relevant if drm_is_primary_client() returns true. Note that + * this only matches &drm_device.master if the master is the currently + * active one. * * See also @authentication and @is_master and the :ref:`section on * primary nodes and authentication `. */ struct drm_master *master; + /** @master_lock: Serializes @master. */ + spinlock_t master_lookup_lock; + /** @pid: Process that opened this file. */ struct pid *pid; -- 2.30.2