From patchwork Wed Aug 18 07:38:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 499257 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.9 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNWANTED_LANGUAGE_BODY, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E7D9C4338F for ; Wed, 18 Aug 2021 07:40:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 54D4D61076 for ; Wed, 18 Aug 2021 07:40:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238718AbhHRHkx (ORCPT ); Wed, 18 Aug 2021 03:40:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37904 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238224AbhHRHkw (ORCPT ); Wed, 18 Aug 2021 03:40:52 -0400 Received: from mail-pl1-x631.google.com (mail-pl1-x631.google.com [IPv6:2607:f8b0:4864:20::631]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7FBAFC061764; Wed, 18 Aug 2021 00:40:18 -0700 (PDT) Received: by mail-pl1-x631.google.com with SMTP id u1so1294154plr.1; Wed, 18 Aug 2021 00:40:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=+V7VHTmfbYqxcJ76dou/taP9ZefhBvJ0ABxX4BEFPg8=; b=m7n8Xew0QgjJED5UdrcOheshCPBH0tOTHhxv660xjAbgyIpemb6WWMkWeOjM3nvhhi YBjY0Vw0mJFYYfhOB6GFTpmXrcR+S5UftwQpJUdjC9CP5vhdSBj88Fd7nUGahEFzZuKW 4gqVic6J3w0xZojmhuJzQ7aiMxKPoMaoQcmarwrsEmdpNNw7CS0U/dM55b0Si0LYcaNv hYeoPp1t8rlOl1BuCXVgd/S4J0vIXLK1ToRPRmS5zni6OrP4BxM8jnWGsu6XodZQCzS8 KEkr5hi6W6AElILHnDc9tP044YmuoT1hNaikI98llUuJ2y7FCwQy3gb1mPUl3Lo56iI9 iAqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=+V7VHTmfbYqxcJ76dou/taP9ZefhBvJ0ABxX4BEFPg8=; b=Icc1icZmcfHf+h/kjQkAbVxk/cMFEQ82LvBPKkDItZtMSJGtsjyJpStGx5WkKQewmP BS9zUdzONrVFGOi9G25lIt+yKt3DxXv8Iz28qS+/AX1zTuDGIYQQJgJ+p3J8sK9+1uQI FKl7ox3lnPbYSMZRr1UwGb/mcU98zAKPWSYsF8MnhLw8Y8UKX8lVx3vm5CCPG9avlN4P SpymwFNRZZQAH9Dw5L8IrG31jNdXpnkdyCYo/aMOYJEoLSGWFqSY2mYTPiTjrEqjL9D4 iFXp+PDiycq4hU1k205Jk/kKiwH9NYhp3dI7ta580PmS/8do9xoITq/lnkgcX4dudg82 eWkQ== X-Gm-Message-State: AOAM533MMQ49PbiOVAg87XCQHbfyXa+clT1d/sDmmKXunc9JiYQKpAI6 TFyZc73iRGnxY4Ib3lVubXI= X-Google-Smtp-Source: ABdhPJxykdwOQoyj/vaukkauBj0yW2JddlxJ9QU5ekI6Ur/DK3dD/PNFCCoHKHLsE4Eb+yPsJaq8ww== X-Received: by 2002:a17:90a:6684:: with SMTP id m4mr8131277pjj.226.1629272418090; Wed, 18 Aug 2021 00:40:18 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id u3sm3886729pjr.2.2021.08.18.00.40.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Aug 2021 00:40:17 -0700 (PDT) From: Desmond Cheong Zhi Xi To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, sumit.semwal@linaro.org, christian.koenig@amd.com, axboe@kernel.dk, oleg@redhat.com, tglx@linutronix.de, dvyukov@google.com, walter-zh.wu@mediatek.com Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v3 1/9] drm: move master_lookup_lock into drm_device Date: Wed, 18 Aug 2021 15:38:16 +0800 Message-Id: <20210818073824.1560124-2-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210818073824.1560124-1-desmondcheongzx@gmail.com> References: <20210818073824.1560124-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org The master_lookup_lock spinlock can be useful as an inner lock for other attributes that are currently protected by drm_device.master_mutex. However, since the spinlock belongs to struct drm_file, its use case is limited to serializing accesses by a single drm_file. Moving this lock into struct drm_device allows us to use it for structures that are accessed by multiple drm_files, such as drm_master.magic_map. Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_auth.c | 18 +++++++++--------- drivers/gpu/drm/drm_drv.c | 1 + drivers/gpu/drm/drm_file.c | 1 - include/drm/drm_device.h | 3 +++ include/drm/drm_file.h | 10 ++++------ 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 60a6b21474b1..8efb58aa7d95 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -63,7 +63,7 @@ static bool drm_is_current_master_locked(struct drm_file *fpriv) { - lockdep_assert_once(lockdep_is_held(&fpriv->master_lookup_lock) || + lockdep_assert_once(lockdep_is_held(&fpriv->minor->dev->master_lookup_lock) || lockdep_is_held(&fpriv->minor->dev->master_mutex)); return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; @@ -83,9 +83,9 @@ bool drm_is_current_master(struct drm_file *fpriv) { bool ret; - spin_lock(&fpriv->master_lookup_lock); + spin_lock(&fpriv->minor->dev->master_lookup_lock); ret = drm_is_current_master_locked(fpriv); - spin_unlock(&fpriv->master_lookup_lock); + spin_unlock(&fpriv->minor->dev->master_lookup_lock); return ret; } @@ -174,9 +174,9 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) new_master = drm_master_create(dev); if (!new_master) return -ENOMEM; - spin_lock(&fpriv->master_lookup_lock); + spin_lock(&dev->master_lookup_lock); fpriv->master = new_master; - spin_unlock(&fpriv->master_lookup_lock); + spin_unlock(&dev->master_lookup_lock); fpriv->is_master = 1; fpriv->authenticated = 1; @@ -338,9 +338,9 @@ int drm_master_open(struct drm_file *file_priv) if (!dev->master) { ret = drm_new_set_master(dev, file_priv); } else { - spin_lock(&file_priv->master_lookup_lock); + spin_lock(&dev->master_lookup_lock); file_priv->master = drm_master_get(dev->master); - spin_unlock(&file_priv->master_lookup_lock); + spin_unlock(&dev->master_lookup_lock); } mutex_unlock(&dev->master_mutex); @@ -405,13 +405,13 @@ struct drm_master *drm_file_get_master(struct drm_file *file_priv) { struct drm_master *master = NULL; - spin_lock(&file_priv->master_lookup_lock); + spin_lock(&file_priv->minor->dev->master_lookup_lock); if (!file_priv->master) goto unlock; master = drm_master_get(file_priv->master); unlock: - spin_unlock(&file_priv->master_lookup_lock); + spin_unlock(&file_priv->minor->dev->master_lookup_lock); return master; } EXPORT_SYMBOL(drm_file_get_master); diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7a5097467ba5..218c16f11c80 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -612,6 +612,7 @@ static int drm_dev_init(struct drm_device *dev, mutex_init(&dev->filelist_mutex); mutex_init(&dev->clientlist_mutex); mutex_init(&dev->master_mutex); + spin_lock_init(&dev->master_lookup_lock); ret = drmm_add_action(dev, drm_dev_init_release, NULL); if (ret) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index ed25168619fc..b8679bbaea69 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -176,7 +176,6 @@ 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_device.h b/include/drm/drm_device.h index 604b1d1b2d72..506eb2784819 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -152,6 +152,9 @@ struct drm_device { */ struct mutex master_mutex; + /** @master_lookup_lock: Serializes &drm_file.master. */ + spinlock_t master_lookup_lock; + /** * @open_count: * diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index a3acb7ac3550..0536e9612a46 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -227,15 +227,16 @@ struct drm_file { * @master: * * Master this node is currently associated with. Protected by struct - * &drm_device.master_mutex, and serialized by @master_lookup_lock. + * &drm_device.master_mutex, and serialized by + * &drm_device.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. * * To update @master, both &drm_device.master_mutex and - * @master_lookup_lock need to be held, therefore holding either of - * them is safe and enough for the read side. + * &drm_device.master_lookup_lock need to be held, therefore holding + * either of them is safe and enough for the read side. * * When dereferencing this pointer, either hold struct * &drm_device.master_mutex for the duration of the pointer's use, or @@ -248,9 +249,6 @@ struct drm_file { */ struct drm_master *master; - /** @master_lock: Serializes @master. */ - spinlock_t master_lookup_lock; - /** @pid: Process that opened this file. */ struct pid *pid; From patchwork Wed Aug 18 07:38:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 499256 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F24CC4338F for ; Wed, 18 Aug 2021 07:40:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 36BC361076 for ; Wed, 18 Aug 2021 07:40:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238603AbhHRHlQ (ORCPT ); Wed, 18 Aug 2021 03:41:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38006 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239245AbhHRHlM (ORCPT ); Wed, 18 Aug 2021 03:41:12 -0400 Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3D911C061796; Wed, 18 Aug 2021 00:40:38 -0700 (PDT) Received: by mail-pl1-x630.google.com with SMTP id o10so1327142plg.0; Wed, 18 Aug 2021 00:40:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=he3hjgMVhcOXzxQ8ZzPKGe5qy8qOVQuZUYfKXwOfAPg=; b=V3ChonAMDLG13W4R9mt1UZfcDRlf3NYYG5Rp6zwLe9hoPv7kNaLSz1SBaGSu4gacb2 IG7pihFofk738zo0sUjjMECc8BKbXHoRFgXUaz+rksmgG1umJdIhslaO4DouScE25nrz QhQN0KBGfOlxOAV5gm+r9YxMmPtBF13v/W8YFP2iCl19RlJ5zdD+L2hOm2sa+1iKHZGz CgFFYbvwQ/FegyZK22MDmuuW+iQHHAazsx4WnIW9u2x3s0WyVCRHOYH7otiYZitptlk3 7J02sZUKJx/5shMUykUIDn5WogVBsFWvX2MIVtWcImlFFB8LyqmG2vbSKdj3J6amSb+W /hSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=he3hjgMVhcOXzxQ8ZzPKGe5qy8qOVQuZUYfKXwOfAPg=; b=YBXT09Z4Mm9MZxnPeLB53s6OMQFtI4qCWuuEQTx5ZdV/0kTADHqdeiPwsZcDyA7nk0 ej4+VNkKbgm9SzfQoIpRcQVAs0MW+VKHu7MUVpunVxU2/EUGpvbj9zbOW/TpiFCTFF5x hFtDIfAYc6TfbaDTJ2YO1oUyTSzW/CYula56SoKCIQZxQ+LRN/ITWQfDz2Ov26O+Wz5Y JqkmhbqE1ZezoGLqjm8oLWBYKiGEuDP5cVLJkpqDmB2HkQsTHgPp1sgplbbhHhG4WYOz BjfWenJVPyKCoxV3qliqtS9QlrrTq8Xigw6dSB448r6h5Pj1GRjSC1OZZPxndbuwj1kN kPGA== X-Gm-Message-State: AOAM531GLsxjwe9JoD/VPUAOj+REPbmkdvMp9N6kfEgWYvzS8vHKNI3x 3irW24mvyEMRqtcVBL2WlwA= X-Google-Smtp-Source: ABdhPJykotiJspllqjLdDweoNoswYBNnKnCEjOlyfwnrSucYaBkYPe1fsbb0KtPlQEnYeO0NwCNLag== X-Received: by 2002:a17:90a:b009:: with SMTP id x9mr7893927pjq.97.1629272437819; Wed, 18 Aug 2021 00:40:37 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id u3sm3886729pjr.2.2021.08.18.00.40.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Aug 2021 00:40:37 -0700 (PDT) From: Desmond Cheong Zhi Xi To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, sumit.semwal@linaro.org, christian.koenig@amd.com, axboe@kernel.dk, oleg@redhat.com, tglx@linutronix.de, dvyukov@google.com, walter-zh.wu@mediatek.com Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v3 3/9] drm: check for null master in drm_is_current_master_locked Date: Wed, 18 Aug 2021 15:38:18 +0800 Message-Id: <20210818073824.1560124-4-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210818073824.1560124-1-desmondcheongzx@gmail.com> References: <20210818073824.1560124-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org There is a window after calling drm_master_release, and before a file is freed, where drm_file can have is_master set to true, but both the drm_file and drm_device have no master. This could result in wrongly approving permissions in drm_is_current_master_locked. Add a check that fpriv->master is non-NULl to guard against this scenario. Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_auth.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 8c0e0dba1611..f9267b21556e 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -66,7 +66,8 @@ static bool drm_is_current_master_locked(struct drm_file *fpriv) lockdep_assert_once(lockdep_is_held(&fpriv->minor->dev->master_lookup_lock) || lockdep_is_held(&fpriv->minor->dev->master_mutex)); - return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; + return (fpriv->is_master && fpriv->master && + drm_lease_owner(fpriv->master) == fpriv->minor->dev->master); } /** From patchwork Wed Aug 18 07:38:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 499255 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.9 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNWANTED_LANGUAGE_BODY, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83915C4320A for ; Wed, 18 Aug 2021 07:41:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6A0E561076 for ; Wed, 18 Aug 2021 07:41:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239284AbhHRHlg (ORCPT ); Wed, 18 Aug 2021 03:41:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38122 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239351AbhHRHld (ORCPT ); Wed, 18 Aug 2021 03:41:33 -0400 Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A7EF8C061764; Wed, 18 Aug 2021 00:40:59 -0700 (PDT) Received: by mail-pl1-x634.google.com with SMTP id q2so1234256plr.11; Wed, 18 Aug 2021 00:40:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=QEHuccOxvqNtSxSgci59cP9jo3dJUSb2vZvhFR8kmBM=; b=kyrTRnECEHbET1U8ahqUBcDC2k+PuuaNd2ZFISYrMAgnQWFXBkgRULnxtJd3D4A0Qk sv6Dy6gL1tZXoZf95+l5ZqMuBedFp7tD2JJ9vpU9tJZmG80Ru6fRUUErQa00tQF7JXal q4nkcdIu7vZAVlo48jb1ep26SNhQpD49gwhKdP9DhDK71KDMigl0pXVj+xgph1iYTpCV GEdyfy7nD9jCd6cWDxnfHI1Pl3E996Z52giWcXylPXYNY8AzLtKn/qEwWl6u46KGiJ2f tXSdXtqI2+YMaHhJ9XwkIBJrMaTN/Tr85hDUyG0oe6NBmufq+MqHUnY8beTAAQ7dkVvb zMZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=QEHuccOxvqNtSxSgci59cP9jo3dJUSb2vZvhFR8kmBM=; b=NUX0FzAKKTxUXyGgoRSw33fEnE4tFU2zdnslCQe7Mwj99X1Jj12URQO3oysjFL+leI o/DX2PHwZSXd3PgrIpBgZdga/9pFZOBzj2tYifXbBJU0BfohWiE4EzYdowAyLDbYimN6 y1Xq7dKXCtAUNMd4BCN9aKVzWyT4AncMTi7vncmShv2xzkXMA7Hto9zfegtIs2uF/zaU DuCMIgPm1GikkWg7i0y3D9eDquuiZo+KfjxfMisEM88Tk7trmXYEw5Qk+5YBGq6xY81O ssQOdRjzLycSldSgPBcFI8CFEY01/nHhTbdPnhxCHJfiV/JTBreqX+haFRkqKDil4f7T U7Uw== X-Gm-Message-State: AOAM5331xU8aPNGRNSzyVwLkGk0Y+GcK29526xbH8oH3mvT6S4HmmlIy 0QX6mifuKiF+PetQjoZMnS0= X-Google-Smtp-Source: ABdhPJxoBfNvfFHKP3I6P1QO1863L8cr1aqR7zIgRwfSUR+ThNMh9rENh0OPTK/qPRiwQJIg/iQTmg== X-Received: by 2002:a17:90a:bf85:: with SMTP id d5mr8198209pjs.210.1629272459270; Wed, 18 Aug 2021 00:40:59 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id u3sm3886729pjr.2.2021.08.18.00.40.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Aug 2021 00:40:58 -0700 (PDT) From: Desmond Cheong Zhi Xi To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, sumit.semwal@linaro.org, christian.koenig@amd.com, axboe@kernel.dk, oleg@redhat.com, tglx@linutronix.de, dvyukov@google.com, walter-zh.wu@mediatek.com Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v3 5/9] drm: protect magic_map, unique{_len} with master_lookup_lock Date: Wed, 18 Aug 2021 15:38:20 +0800 Message-Id: <20210818073824.1560124-6-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210818073824.1560124-1-desmondcheongzx@gmail.com> References: <20210818073824.1560124-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Currently, drm_device.master_mutex is used to serialize writes to the drm_master.magic_map idr and to protect drm_master.unique{_len}. In preparation for converting drm_device.master_mutex into an outer rwsem that might be read locked before entering some of these functions, we can instead serialize access to drm_master.magic_map and drm_master.unique{_len} using drm_device.master_lookup_lock which is an inner lock. Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_auth.c | 12 +++++++----- drivers/gpu/drm/drm_ioctl.c | 10 ++++++---- include/drm/drm_auth.h | 6 +++--- include/drm/drm_device.h | 7 ++++++- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index b7230604496b..0acb444fbbac 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -98,10 +98,10 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) struct drm_master *master; int ret = 0; - mutex_lock(&dev->master_mutex); + spin_lock(&dev->master_lookup_lock); master = file_priv->master; if (!master) { - mutex_unlock(&dev->master_mutex); + spin_unlock(&dev->master_lookup_lock); return -EINVAL; } @@ -112,7 +112,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) file_priv->magic = ret; } auth->magic = file_priv->magic; - mutex_unlock(&dev->master_mutex); + spin_unlock(&dev->master_lookup_lock); DRM_DEBUG("%u\n", auth->magic); @@ -127,13 +127,13 @@ int drm_authmagic(struct drm_device *dev, void *data, DRM_DEBUG("%u\n", auth->magic); - mutex_lock(&dev->master_mutex); + spin_lock(&dev->master_lookup_lock); file = idr_find(&file_priv->master->magic_map, auth->magic); if (file) { file->authenticated = 1; idr_replace(&file_priv->master->magic_map, NULL, auth->magic); } - mutex_unlock(&dev->master_mutex); + spin_unlock(&dev->master_lookup_lock); return file ? 0 : -EINVAL; } @@ -366,8 +366,10 @@ void drm_master_release(struct drm_file *file_priv) if (!master) goto unlock; + spin_lock(&dev->master_lookup_lock); if (file_priv->magic) idr_remove(&master->magic_map, file_priv->magic); + spin_unlock(&dev->master_lookup_lock); if (!drm_is_current_master_locked(file_priv)) goto out; diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 4d029d3061d9..e5c3845b6e62 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -119,21 +119,21 @@ int drm_getunique(struct drm_device *dev, void *data, struct drm_unique *u = data; struct drm_master *master; - mutex_lock(&dev->master_mutex); + spin_lock(&dev->master_lookup_lock); master = file_priv->master; if (!master) { - mutex_unlock(&dev->master_mutex); + spin_unlock(&dev->master_lookup_lock); return -EINVAL; } if (u->unique_len >= master->unique_len) { if (copy_to_user(u->unique, master->unique, master->unique_len)) { - mutex_unlock(&dev->master_mutex); + spin_unlock(&dev->master_lookup_lock); return -EFAULT; } } u->unique_len = master->unique_len; - mutex_unlock(&dev->master_mutex); + spin_unlock(&dev->master_lookup_lock); return 0; } @@ -405,7 +405,9 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f * Version 1.1 includes tying of DRM to specific device * Version 1.4 has proper PCI domain support */ + spin_lock(&dev->master_lookup_lock); retcode = drm_set_busid(dev, file_priv); + spin_unlock(&dev->master_lookup_lock); if (retcode) goto done; } diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h index ba248ca8866f..f5be73153798 100644 --- a/include/drm/drm_auth.h +++ b/include/drm/drm_auth.h @@ -67,17 +67,17 @@ struct drm_master { struct drm_device *dev; /** * @unique: Unique identifier: e.g. busid. Protected by - * &drm_device.master_mutex. + * &drm_device.master_lookup_lock. */ char *unique; /** * @unique_len: Length of unique field. Protected by - * &drm_device.master_mutex. + * &drm_device.master_lookup_lock. */ int unique_len; /** * @magic_map: Map of used authentication tokens. Protected by - * &drm_device.master_mutex. + * &drm_device.master_lookup_lock. */ struct idr magic_map; void *driver_priv; diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 506eb2784819..cf5d15aeb25f 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -152,7 +152,12 @@ struct drm_device { */ struct mutex master_mutex; - /** @master_lookup_lock: Serializes &drm_file.master. */ + /** + * @master_lookup_lock: + * + * Serializes &drm_file.master, &drm_master.magic_map, + * &drm_master.unique, and &drm_master.unique_len. + */ spinlock_t master_lookup_lock; /** From patchwork Wed Aug 18 07:38:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 499254 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3D0AEC4338F for ; Wed, 18 Aug 2021 07:41:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2724E6108D for ; Wed, 18 Aug 2021 07:41:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238878AbhHRHl4 (ORCPT ); Wed, 18 Aug 2021 03:41:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38206 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239364AbhHRHls (ORCPT ); Wed, 18 Aug 2021 03:41:48 -0400 Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF634C0613CF; Wed, 18 Aug 2021 00:41:13 -0700 (PDT) Received: by mail-pl1-x632.google.com with SMTP id a5so1270589plh.5; Wed, 18 Aug 2021 00:41:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=6qsW0sYS+QWl9YqcEcKPcEnVn6HLCKGFheZ9UsQK7Pw=; b=UOFTIL4BlsedoXnh4zHxO1Ey3z7ZulzCS5A0HofF4Pdc6KSOphdAQfUboSXTigN3QJ KV9GWvBIpiqXwe2DDYu2nmNE8CrFkYYFXVATDHFQup4d7oi9AssfF8qYtWT9XhKXgvqV hI0Q1Gq8wgxyphDTnoroV9F5koJtJ59wKMcWiCPDa1Bkq6RqPGY9N3aa8EV6KItNemMa gP5HwtFGnKuqLIsbCWkrhhrBrq+ZV++p+8hZ1RmRx/QEg7TYVGg0v9wiGESj2m60zwuM E0/lsPiZhzcN7BbB6TNJh84nkcOUcRh6VEIOKIcuc4++u7nT6silUQXPZR4R/jFnhWG0 eMWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=6qsW0sYS+QWl9YqcEcKPcEnVn6HLCKGFheZ9UsQK7Pw=; b=ebZURKDm0juV4KL1EZfMH6OcqeCqXKS928bjOjb7YBlkViUAqJG7QsYKK+UwIN8f6t 7e43rpKcRKlZWXhrV+O5XfwIJncbFvT43ojcsHIYdc/tlwqYjuy5krdWjmILdcctNWm6 SQpe9pUxvUkVljpHPpRj9ABaA9mlaejAnjRx6oLJJFD+CKOit2kbPBuH3i0b7+SCFpDR fGJOORpJzskoZFpMUIEzOsTBHVU/3x0OXbk11NzNZRJJJTsO6upzHYXNJWTFMLglcVMZ TA6hzApNU+sC4etQWviFl9WtwOxKBnNP+0gmTDeCaw+dUVOxbBCF7CG1ZGjv5f5huHbi 8PCw== X-Gm-Message-State: AOAM531Dxgb3+pzqjmqW06wrVnfAt9lq4Zdarvzzqe13Nw4otYpmlB6U iWK7rFnSJJeTBkLT6mS4eLY= X-Google-Smtp-Source: ABdhPJyoRQikktAIIYzk2SdRrsIHolgC6+oIsd/deH794I59G6Uc3YKKU+oqVIrW58xvLjcm506Y1A== X-Received: by 2002:a17:902:d713:b029:12d:5b1a:c49 with SMTP id w19-20020a170902d713b029012d5b1a0c49mr6051047ply.70.1629272473538; Wed, 18 Aug 2021 00:41:13 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id u3sm3886729pjr.2.2021.08.18.00.41.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Aug 2021 00:41:13 -0700 (PDT) From: Desmond Cheong Zhi Xi To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, sumit.semwal@linaro.org, christian.koenig@amd.com, axboe@kernel.dk, oleg@redhat.com, tglx@linutronix.de, dvyukov@google.com, walter-zh.wu@mediatek.com Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v3 7/9] drm: update global mutex lock in the ioctl handler Date: Wed, 18 Aug 2021 15:38:22 +0800 Message-Id: <20210818073824.1560124-8-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210818073824.1560124-1-desmondcheongzx@gmail.com> References: <20210818073824.1560124-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org In a future patch, a read lock on drm_device.master_rwsem is held in the ioctl handler before the check for ioctl permissions. However, this produces the following lockdep splat: ====================================================== WARNING: possible circular locking dependency detected 5.14.0-rc6-CI-Patchwork_20831+ #1 Tainted: G U ------------------------------------------------------ kms_lease/1752 is trying to acquire lock: ffffffff827bad88 (drm_global_mutex){+.+.}-{3:3}, at: drm_open+0x64/0x280 but task is already holding lock: ffff88812e350108 (&dev->master_rwsem){++++}-{3:3}, at: drm_ioctl_kernel+0xfb/0x1a0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (&dev->master_rwsem){++++}-{3:3}: lock_acquire+0xd3/0x310 down_read+0x3b/0x140 drm_master_internal_acquire+0x1d/0x60 drm_client_modeset_commit+0x10/0x40 __drm_fb_helper_restore_fbdev_mode_unlocked+0x88/0xb0 drm_fb_helper_set_par+0x34/0x40 intel_fbdev_set_par+0x11/0x40 [i915] fbcon_init+0x270/0x4f0 visual_init+0xc6/0x130 do_bind_con_driver+0x1de/0x2c0 do_take_over_console+0x10e/0x180 do_fbcon_takeover+0x53/0xb0 register_framebuffer+0x22d/0x310 __drm_fb_helper_initial_config_and_unlock+0x36c/0x540 intel_fbdev_initial_config+0xf/0x20 [i915] async_run_entry_fn+0x28/0x130 process_one_work+0x26d/0x5c0 worker_thread+0x37/0x390 kthread+0x13b/0x170 ret_from_fork+0x1f/0x30 -> #1 (&helper->lock){+.+.}-{3:3}: lock_acquire+0xd3/0x310 __mutex_lock+0xa8/0x930 __drm_fb_helper_restore_fbdev_mode_unlocked+0x44/0xb0 intel_fbdev_restore_mode+0x2b/0x50 [i915] drm_lastclose+0x27/0x50 drm_release_noglobal+0x42/0x60 __fput+0x9e/0x250 task_work_run+0x6b/0xb0 exit_to_user_mode_prepare+0x1c5/0x1d0 syscall_exit_to_user_mode+0x19/0x50 do_syscall_64+0x46/0xb0 entry_SYSCALL_64_after_hwframe+0x44/0xae -> #0 (drm_global_mutex){+.+.}-{3:3}: validate_chain+0xb39/0x1e70 __lock_acquire+0x5a1/0xb70 lock_acquire+0xd3/0x310 __mutex_lock+0xa8/0x930 drm_open+0x64/0x280 drm_stub_open+0x9f/0x100 chrdev_open+0x9f/0x1d0 do_dentry_open+0x14a/0x3a0 dentry_open+0x53/0x70 drm_mode_create_lease_ioctl+0x3cb/0x970 drm_ioctl_kernel+0xc9/0x1a0 drm_ioctl+0x201/0x3d0 __x64_sys_ioctl+0x6a/0xa0 do_syscall_64+0x37/0xb0 entry_SYSCALL_64_after_hwframe+0x44/0xae other info that might help us debug this: Chain exists of: drm_global_mutex --> &helper->lock --> &dev->master_rwsem Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&dev->master_rwsem); lock(&helper->lock); lock(&dev->master_rwsem); lock(drm_global_mutex); *** DEADLOCK *** The lock hierarchy inversion happens because we grab the drm_global_mutex while already holding on to master_rwsem. To avoid this, we do some prep work to grab the drm_global_mutex before checking for ioctl permissions. At the same time, we update the check for the global mutex to use the drm_dev_needs_global_mutex helper function. Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_ioctl.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 880fc565d599..2cb57378a787 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -779,19 +779,19 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, if (drm_dev_is_unplugged(dev)) return -ENODEV; + /* Enforce sane locking for modern driver ioctls. */ + if (unlikely(drm_dev_needs_global_mutex(dev)) && !(flags & DRM_UNLOCKED)) + mutex_lock(&drm_global_mutex); + retcode = drm_ioctl_permit(flags, file_priv); if (unlikely(retcode)) - return retcode; + goto out; - /* Enforce sane locking for modern driver ioctls. */ - if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) || - (flags & DRM_UNLOCKED)) - retcode = func(dev, kdata, file_priv); - else { - mutex_lock(&drm_global_mutex); - retcode = func(dev, kdata, file_priv); + retcode = func(dev, kdata, file_priv); + +out: + if (unlikely(drm_dev_needs_global_mutex(dev)) && !(flags & DRM_UNLOCKED)) mutex_unlock(&drm_global_mutex); - } return retcode; } EXPORT_SYMBOL(drm_ioctl_kernel); From patchwork Wed Aug 18 07:38:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 499253 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 51D6CC432BE for ; Wed, 18 Aug 2021 07:41:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 39ED16108D for ; Wed, 18 Aug 2021 07:41:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239320AbhHRHmF (ORCPT ); Wed, 18 Aug 2021 03:42:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38280 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239470AbhHRHmC (ORCPT ); Wed, 18 Aug 2021 03:42:02 -0400 Received: from mail-pf1-x42f.google.com (mail-pf1-x42f.google.com [IPv6:2607:f8b0:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CBC63C06179A; Wed, 18 Aug 2021 00:41:28 -0700 (PDT) Received: by mail-pf1-x42f.google.com with SMTP id j187so1270955pfg.4; Wed, 18 Aug 2021 00:41:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=1bkLMEw2t/totCFLQIzrVHXO8lyHlY0O9pr/RZGsdI0=; b=bLirg/ekc0It+7MhEmdqeKW1uskct7uSLEYEWb5Ts5xbzkUlwtt0MXY+lJk9DGWH0b Uqv9IwaVom6h+9HWW2VpWO/ZrDFqJx917Te1eyb59FUAVKaVVQ6cIDAYwQZGA/SqTxQ3 okt1jnoVvDtYswWK6eXPFUwy4mbZ41c62jY+cFma3PrODfQ6olrM1dkWU8e+fnUdhuA6 LcRvdhF2laqs5TgwKBgnqpmuons9CZfNplWWsgdfHzRXRlDU4/zxRj0MD2cYdlg/kgz6 uQy/f3qvZI+gA9UJxHE+io77IuzIlxyEJuKlHcVdF3i4FEKKISV1yS+JwxrBszCcmW6+ ed/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=1bkLMEw2t/totCFLQIzrVHXO8lyHlY0O9pr/RZGsdI0=; b=qSkP/I+WErYgUBDLcAUsKxR7C4EsVEpplCGrHflvTHmeyoBOOXf05wXqX45XbmCC11 xlBmz4GnDnqqyMb1Yo9vnomOZ9UVxRXxvh7Xbdzv5v38ZxBFvIR1EEvbdhQxrCWZGBBr R5oIyr6MmihvivqoAAZzvW11N7M+h/xvs3fKNIE6XnDssUz90XZF0FIz8Be+WRZF4/2J 8c8v2L2r+AeNUX9DjFDEojAxfTeRzJcqhZrVscAvlb83NvA5STvZQtH6iDjX7vsBWipr NpNM8ePsJCcGt1HC/zltwhfF/+gtS+sNJAD2YoIKbOWoseZX9sIdHY76P90OcH3oFlNm rCbg== X-Gm-Message-State: AOAM530BVhfJBVO7jgwXfHHX0tqgA2Urg0n0tBYQzXEtLQAonIt0/MpE gJbIKCcqEKXPcdWFpSDTwNs= X-Google-Smtp-Source: ABdhPJys3HPHWTRsAjT+5YnD6rJNYZL136LiYwwEF8beumJZ6hVGWSIIaE7eeFYyLYOhAlW2I2XuJQ== X-Received: by 2002:a05:6a00:181c:b029:3c6:2258:a844 with SMTP id y28-20020a056a00181cb02903c62258a844mr8044761pfa.6.1629272488381; Wed, 18 Aug 2021 00:41:28 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id u3sm3886729pjr.2.2021.08.18.00.41.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Aug 2021 00:41:27 -0700 (PDT) From: Desmond Cheong Zhi Xi To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, sumit.semwal@linaro.org, christian.koenig@amd.com, axboe@kernel.dk, oleg@redhat.com, tglx@linutronix.de, dvyukov@google.com, walter-zh.wu@mediatek.com Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, Daniel Vetter Subject: [PATCH v3 9/9] drm: avoid races with modesetting rights Date: Wed, 18 Aug 2021 15:38:24 +0800 Message-Id: <20210818073824.1560124-10-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210818073824.1560124-1-desmondcheongzx@gmail.com> References: <20210818073824.1560124-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org In drm_client_modeset.c and drm_fb_helper.c, drm_master_internal_{acquire,release} are used to avoid races with DRM userspace. These functions hold onto drm_device.master_rwsem while committing, and bail if there's already a master. However, there are other places where modesetting rights can race. A time-of-check-to-time-of-use error can occur if an ioctl that changes the modeset has its rights revoked after it validates its permissions, but before it completes. There are four places where modesetting permissions can change: - DROP_MASTER ioctl removes rights for a master and its leases - REVOKE_LEASE ioctl revokes rights for a specific lease - SET_MASTER ioctl sets the device master if the master role hasn't been acquired yet - drm_open which can create a new master for a device if one does not currently exist These races can be avoided using drm_device.master_rwsem: users that perform modesetting should hold a read lock on the new drm_device.master_rwsem, and users that change these permissions should either hold a write lock, or should flush readers before returning to userspace. Reported-by: Daniel Vetter Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_auth.c | 29 +++++++++++++++++++++++++++++ drivers/gpu/drm/drm_internal.h | 1 + drivers/gpu/drm/drm_ioctl.c | 8 ++++++-- drivers/gpu/drm/drm_lease.c | 1 + include/drm/drm_device.h | 10 +++++++++- 5 files changed, 46 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index b65681ff42fc..84d00275ff8a 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -29,6 +29,7 @@ */ #include +#include #include #include @@ -127,6 +128,7 @@ int drm_authmagic(struct drm_device *dev, void *data, DRM_DEBUG("%u\n", auth->magic); + lockdep_assert_held_once(&dev->master_rwsem); spin_lock(&dev->master_lookup_lock); file = idr_find(&file_priv->master->magic_map, auth->magic); if (file) { @@ -485,3 +487,30 @@ void drm_master_internal_release(struct drm_device *dev) up_read(&dev->master_rwsem); } EXPORT_SYMBOL(drm_master_internal_release); + +/* After flushing, all readers that might have seen old master/lease + * permissions are guaranteed to have completed. + */ +static void master_flush(struct callback_head *cb) +{ + struct drm_device *dev = container_of(cb, struct drm_device, + master_flush_work); + + down_write(&dev->master_rwsem); + up_write(&dev->master_rwsem); +} + +/* Queues up work to flush all readers of master/lease permissions. This work + * is run before this task returns to user mode. Calling this function when a + * task changes modesetting rights ensures that other processes that perform + * modesetting do not race with userspace. + */ +void drm_master_flush(struct drm_device *dev) +{ + init_task_work(&dev->master_flush_work, master_flush); + task_work_add(current, &dev->master_flush_work, TWA_RESUME); + /* If task_work_add fails, then the task is exiting. In this case, it + * doesn't matter if master_flush is run, so we don't need an + * alternative mechanism for flushing. + */ +} diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 17f3548c8ed2..b1cd39338756 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -144,6 +144,7 @@ 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); +void drm_master_flush(struct drm_device *dev); /* drm_sysfs.c */ extern struct class *drm_class; diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 2cb57378a787..7f523e1c5650 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -390,7 +390,7 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f struct drm_set_version *sv = data; int if_version, retcode = 0; - down_read(&dev->master_rwsem); + lockdep_assert_held_once(&dev->master_rwsem); if (sv->drm_di_major != -1) { if (sv->drm_di_major != DRM_IF_MAJOR || sv->drm_di_minor < 0 || sv->drm_di_minor > DRM_IF_MINOR) { @@ -427,7 +427,6 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f sv->drm_di_minor = DRM_IF_MINOR; sv->drm_dd_major = dev->driver->major; sv->drm_dd_minor = dev->driver->minor; - up_read(&dev->master_rwsem); return retcode; } @@ -783,6 +782,9 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, if (unlikely(drm_dev_needs_global_mutex(dev)) && !(flags & DRM_UNLOCKED)) mutex_lock(&drm_global_mutex); + if (unlikely(flags & DRM_MASTER)) + down_read(&dev->master_rwsem); + retcode = drm_ioctl_permit(flags, file_priv); if (unlikely(retcode)) goto out; @@ -790,6 +792,8 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, retcode = func(dev, kdata, file_priv); out: + if (unlikely(flags & DRM_MASTER)) + up_read(&dev->master_rwsem); if (unlikely(drm_dev_needs_global_mutex(dev)) && !(flags & DRM_UNLOCKED)) mutex_unlock(&drm_global_mutex); return retcode; diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index dee4f24a1808..983701198ffd 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -723,6 +723,7 @@ int drm_mode_revoke_lease_ioctl(struct drm_device *dev, } _drm_lease_revoke(lessee); + drm_master_flush(dev); fail: mutex_unlock(&dev->mode_config.idr_mutex); diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index f1ae4570a20a..617f7fe1d3bf 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -151,10 +151,18 @@ struct drm_device { * Synchronizes modesetting rights between multiple users. Users that * can change the modeset or display state must hold a read lock on * @master_rwsem, and users that change modesetting rights should hold - * a write lock. + * a write lock or flush readers before returning to userspace. */ struct rw_semaphore master_rwsem; + /** + * @master_flush_work: + * + * Callback structure used internally to queue work to flush readers of + * master/lease permissions. + */ + struct callback_head master_flush_work; + /** * @master_lookup_lock: *