From patchwork Tue Aug 31 06:13:45 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: 12466499 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 4A085C43214 for ; Tue, 31 Aug 2021 06:14:50 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1976460F4B for ; Tue, 31 Aug 2021 06:14:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 1976460F4B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0A53589B20; Tue, 31 Aug 2021 06:14:49 +0000 (UTC) Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0B57D89B20; Tue, 31 Aug 2021 06:14:47 +0000 (UTC) Received: by mail-pj1-x102f.google.com with SMTP id z24-20020a17090acb1800b0018e87a24300so1641004pjt.0; Mon, 30 Aug 2021 23:14:47 -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=tHn1bKbgKWkWnpZZfeyzrlp2g8PdlS3oPFK4wibr8zM=; b=eXIjFmJAssVk3qum2qiXCLhVDT8BHwEP00acuPJC6D3sWtXepsNRzl7WSa6QoQy99y dUAJT/Gv/isjtGOA3E207i4Tsg3jM8j5lwtN9mQ5mIE1L4CtzGYd9e/zLEhVdXiEHfEX MK3cuuw+X9WOT70++OXU9foFpZmdwQL8+ub9sVd8DFGdh2I4LBOAdF3P8oABbUdS/Y54 4Qjd1EndwTYPXWN/mnR2yMNqh9M9wuTMFPtmjSuKYEHNCteFwK4boUyHPOoTuD7my/aS UQusq/C0g8X0wUQzNjsJflhtzcsL03ZTs8shCqEgQNJ4GLN+7wm4QawSviE32FE7ZpHH +2hA== 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=tHn1bKbgKWkWnpZZfeyzrlp2g8PdlS3oPFK4wibr8zM=; b=fV3z1QUy9qUqUFvfa9JEsnso/apO9+bl+sHTYQwLMdvkxFXyTCO4rY47nzQF1OQj06 rAMcNQ3VJMwXw8UDZ5wojPtYn/lBfBdEU+NvR8NtSJDN9sUZdfXnj9/27mdFNcr+g2Lr H1igRGObaD+ZAAcnU0AZ3Qw+bVc0jlDY/I2J4YxpQYzJ+yGJhCAmOh9nW4/SRcghwuN5 Q0D1PJuS6BIHRKn46GPgbXBDAAGAhR3RU5sy2M6p8mnJXSIOK60+K59OVSk1IqV4eAjl y8A76OFNvPHeoWejhuTLoKCm19+6KYuI/AiXkfWQ3f3BjF6T9CBiFHp0MxbyTKx3yNsS 8ydw== X-Gm-Message-State: AOAM530fdoxIgf11Q6hCP0O/4nJQw6js/wGiLrjVA7ubGFrgAQ9PQk/v Kl1kRkrLO0GmDUVpk0FbA5o= X-Google-Smtp-Source: ABdhPJzGmBXOz6okx6of+AFSntvusISTlHPl1xEvUsw8GRcQjM3xZQnn+uaj9N1kZDXhTmMeOXLoJw== X-Received: by 2002:a17:90b:390d:: with SMTP id ob13mr3460845pjb.129.1630390486636; Mon, 30 Aug 2021 23:14:46 -0700 (PDT) Received: from sanitydock.wifi-cloud.jp ([210.160.217.69]) by smtp.gmail.com with ESMTPSA id z7sm1405724pjr.42.2021.08.30.23.14.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Aug 2021 23:14:46 -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 Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, stable@vger.kernel.org, Daniel Vetter Subject: [PATCH v9 1/4] drm: fix null ptr dereference in drm_master_release Date: Tue, 31 Aug 2021 14:13:45 +0800 Message-Id: <20210831061348.97696-2-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210831061348.97696-1-desmondcheongzx@gmail.com> References: <20210831061348.97696-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" drm_master_release can be called on a drm_file without a master, which results in a null ptr dereference of file_priv->master->magic_map. The three cases are: 1. Error path in drm_open_helper drm_open(): drm_open_helper(): drm_master_open(): drm_new_set_master(); <--- returns -ENOMEM, drm_file.master not set drm_file_free(): drm_master_release(); <--- NULL ptr dereference (file_priv->master->magic_map) 2. Error path in mock_drm_getfile mock_drm_getfile(): anon_inode_getfile(); <--- returns error, drm_file.master not set drm_file_free(): drm_master_release(); <--- NULL ptr dereference (file_priv->master->magic_map) 3. In drm_client_close, as drm_client_open doesn't set up a master drm_file.master is set up in drm_open_helper through the call to drm_master_open, so we mirror it with a call to drm_master_release in drm_close_helper, and remove drm_master_release from drm_file_free to avoid the null ptr dereference. Fixes: 7eeaeb90a6a5 ("drm/file: Don't set master on in-kernel clients") Signed-off-by: Desmond Cheong Zhi Xi Cc: stable@vger.kernel.org Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_file.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index ed25168619fc..90b62f360da1 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -282,9 +282,6 @@ void drm_file_free(struct drm_file *file) drm_legacy_ctxbitmap_flush(dev, file); - if (drm_is_primary_client(file)) - drm_master_release(file); - if (dev->driver->postclose) dev->driver->postclose(dev, file); @@ -305,6 +302,9 @@ static void drm_close_helper(struct file *filp) list_del(&file_priv->lhead); mutex_unlock(&dev->filelist_mutex); + if (drm_is_primary_client(file_priv)) + drm_master_release(file_priv); + drm_file_free(file_priv); } From patchwork Tue Aug 31 06:13:46 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: 12466501 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 3DBE6C43216 for ; Tue, 31 Aug 2021 06:14:57 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0BA4760F4B for ; Tue, 31 Aug 2021 06:14:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0BA4760F4B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 127AC89B65; Tue, 31 Aug 2021 06:14:56 +0000 (UTC) Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9846689B5F; Tue, 31 Aug 2021 06:14:54 +0000 (UTC) Received: by mail-pf1-x42e.google.com with SMTP id t42so14078487pfg.12; Mon, 30 Aug 2021 23:14:54 -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=ksT6Kc+4cXcjdZlMaFAb0YKt3gVH5WYALt2dKHx5mko=; b=uspS9QeeQgfwkZvwtL/f+vUiAlHsC0yZUEzWWNaidiAtqCIPGNFMtjHwgzuogMBJfs AJf8T1AGQSOCNWKqVBZ2jMsZJlbybVJbxNBJFzmCUYd40Sb7ftAiHuB8vhgKN3wFgRR9 n0HSf+6+lKuLOqo7bM9ED6Oh5wpsoWZOF27wHO7OYVprvsIWwb/On0Mt2SmZgdE+MzjM sVJ1X0oUOsViPuKtr0+MGJb9US7oLh5tdtNo9W+I6g9Ugnqlt8FQYb378+R1mGSkrSUj jhY7JROv0/BV1laea79WyjnocMJCzDxDSVnvbIsvgqHvPMJLQEAxqsuNKfIWhgk19KFk KngQ== 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=ksT6Kc+4cXcjdZlMaFAb0YKt3gVH5WYALt2dKHx5mko=; b=ctohfblYkAzaS8BcdAfc1vmbbLMRBEj1XwQUvfMuPgPxMV/uLjkDQaurt6KBCmoRyh 0SHrmxAVY8HhmWWWPaOFS8ydy2/UnJ39yfgkYZjNe734vN9perfe56gl7W9JcStLJs/B pVNMsoEE/3p5yRpvWv2UmQo4gw62AfLfhYAvLU1+f1Wi6xjRIEMdUsa9B4hXlPYNAYav KXy1cV4hjNPDF9wK1QQ0onMUdDcE7ixzhTTmHcjbOYU2V5EPDQWvwIJFiyh364ccffJi xT6grsWr4+g38dFAayCS8HLUgDwi4Gh0wACMPLwO2wP0u/nuL2U1Gb6kxd8aOif9Ygyf t97g== X-Gm-Message-State: AOAM5311+uKog53R5JckJh+940m+Xav2O77yiwpKDvDX6MMcpwd5BbNw xnGK5xvsWjFuVl5E4BCI1mg= X-Google-Smtp-Source: ABdhPJx2eLZnttCUyxAf64iMhS7su0aL0z4KWBYqMGIy0eiCAtx2S7QlOJ9204/CcMjeKqj30iQGlA== X-Received: by 2002:aa7:8d0c:0:b029:3e0:2e32:3148 with SMTP id j12-20020aa78d0c0000b02903e02e323148mr26954205pfe.23.1630390494148; Mon, 30 Aug 2021 23:14:54 -0700 (PDT) Received: from sanitydock.wifi-cloud.jp ([210.160.217.69]) by smtp.gmail.com with ESMTPSA id z7sm1405724pjr.42.2021.08.30.23.14.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Aug 2021 23:14:53 -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 Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, Daniel Vetter Subject: [PATCH v9 2/4] drm: convert drm_device.master_mutex into a rwsem Date: Tue, 31 Aug 2021 14:13:46 +0800 Message-Id: <20210831061348.97696-3-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210831061348.97696-1-desmondcheongzx@gmail.com> References: <20210831061348.97696-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" drm_device.master_mutex currently protects the following: - drm_device.master - drm_file.master - drm_file.was_master - drm_file.is_master - drm_master.unique - drm_master.unique_len - drm_master.magic_map There is a clear separation between functions that read or change these attributes. Hence, convert master_mutex into a rwsem to enable concurrent readers. Signed-off-by: Desmond Cheong Zhi Xi Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_auth.c | 35 ++++++++++++++++++----------------- drivers/gpu/drm/drm_debugfs.c | 4 ++-- drivers/gpu/drm/drm_drv.c | 3 +-- drivers/gpu/drm/drm_ioctl.c | 10 +++++----- include/drm/drm_auth.h | 6 +++--- include/drm/drm_device.h | 10 ++++++---- include/drm/drm_file.h | 12 ++++++------ 7 files changed, 41 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 60a6b21474b1..73ade0513ccb 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -64,7 +64,7 @@ static bool drm_is_current_master_locked(struct drm_file *fpriv) { lockdep_assert_once(lockdep_is_held(&fpriv->master_lookup_lock) || - lockdep_is_held(&fpriv->minor->dev->master_mutex)); + lockdep_is_held(&fpriv->minor->dev->master_rwsem)); return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } @@ -96,7 +96,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) struct drm_auth *auth = data; int ret = 0; - mutex_lock(&dev->master_mutex); + down_write(&dev->master_rwsem); if (!file_priv->magic) { ret = idr_alloc(&file_priv->master->magic_map, file_priv, 1, 0, GFP_KERNEL); @@ -104,7 +104,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); + up_write(&dev->master_rwsem); DRM_DEBUG("%u\n", auth->magic); @@ -119,13 +119,13 @@ int drm_authmagic(struct drm_device *dev, void *data, DRM_DEBUG("%u\n", auth->magic); - mutex_lock(&dev->master_mutex); + down_write(&dev->master_rwsem); 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); + up_write(&dev->master_rwsem); return file ? 0 : -EINVAL; } @@ -167,7 +167,7 @@ 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); + lockdep_assert_held_once(&dev->master_rwsem); WARN_ON(fpriv->is_master); old_master = fpriv->master; @@ -249,7 +249,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, { int ret; - mutex_lock(&dev->master_mutex); + down_write(&dev->master_rwsem); ret = drm_master_check_perm(dev, file_priv); if (ret) @@ -281,7 +281,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, drm_set_master(dev, file_priv, false); out_unlock: - mutex_unlock(&dev->master_mutex); + up_write(&dev->master_rwsem); return ret; } @@ -298,7 +298,7 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, { int ret; - mutex_lock(&dev->master_mutex); + down_write(&dev->master_rwsem); ret = drm_master_check_perm(dev, file_priv); if (ret) @@ -321,8 +321,9 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, } drm_drop_master(dev, file_priv); + out_unlock: - mutex_unlock(&dev->master_mutex); + up_write(&dev->master_rwsem); return ret; } @@ -334,7 +335,7 @@ int drm_master_open(struct drm_file *file_priv) /* if there is no current master make this fd it, but do not create * any master object for render clients */ - mutex_lock(&dev->master_mutex); + down_write(&dev->master_rwsem); if (!dev->master) { ret = drm_new_set_master(dev, file_priv); } else { @@ -342,7 +343,7 @@ int drm_master_open(struct drm_file *file_priv) file_priv->master = drm_master_get(dev->master); spin_unlock(&file_priv->master_lookup_lock); } - mutex_unlock(&dev->master_mutex); + up_write(&dev->master_rwsem); return ret; } @@ -352,7 +353,7 @@ void drm_master_release(struct drm_file *file_priv) struct drm_device *dev = file_priv->minor->dev; struct drm_master *master; - mutex_lock(&dev->master_mutex); + down_write(&dev->master_rwsem); master = file_priv->master; if (file_priv->magic) idr_remove(&file_priv->master->magic_map, file_priv->magic); @@ -375,7 +376,7 @@ void drm_master_release(struct drm_file *file_priv) /* drop the master reference held by the file priv */ if (file_priv->master) drm_master_put(&file_priv->master); - mutex_unlock(&dev->master_mutex); + up_write(&dev->master_rwsem); } /** @@ -450,9 +451,9 @@ EXPORT_SYMBOL(drm_master_put); /* Used by drm_client and drm_fb_helper */ bool drm_master_internal_acquire(struct drm_device *dev) { - mutex_lock(&dev->master_mutex); + down_read(&dev->master_rwsem); if (dev->master) { - mutex_unlock(&dev->master_mutex); + up_read(&dev->master_rwsem); return false; } @@ -463,6 +464,6 @@ EXPORT_SYMBOL(drm_master_internal_acquire); /* Used by drm_client and drm_fb_helper */ void drm_master_internal_release(struct drm_device *dev) { - mutex_unlock(&dev->master_mutex); + up_read(&dev->master_rwsem); } EXPORT_SYMBOL(drm_master_internal_release); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index b0a826489488..b34c9c263188 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -55,7 +55,7 @@ static int drm_name_info(struct seq_file *m, void *data) struct drm_device *dev = minor->dev; struct drm_master *master; - mutex_lock(&dev->master_mutex); + down_read(&dev->master_rwsem); master = dev->master; seq_printf(m, "%s", dev->driver->name); if (dev->dev) @@ -65,7 +65,7 @@ static int drm_name_info(struct seq_file *m, void *data) if (dev->unique) seq_printf(m, " unique=%s", dev->unique); seq_printf(m, "\n"); - mutex_unlock(&dev->master_mutex); + up_read(&dev->master_rwsem); return 0; } diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7a5097467ba5..4556bf42954c 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -570,7 +570,6 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) /* Prevent use-after-free in drm_managed_release when debugging is * enabled. Slightly awkward, but can't really be helped. */ dev->dev = NULL; - mutex_destroy(&dev->master_mutex); mutex_destroy(&dev->clientlist_mutex); mutex_destroy(&dev->filelist_mutex); mutex_destroy(&dev->struct_mutex); @@ -611,7 +610,7 @@ static int drm_dev_init(struct drm_device *dev, mutex_init(&dev->struct_mutex); mutex_init(&dev->filelist_mutex); mutex_init(&dev->clientlist_mutex); - mutex_init(&dev->master_mutex); + init_rwsem(&dev->master_rwsem); ret = drmm_add_action(dev, drm_dev_init_release, NULL); if (ret) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 26f3a9ede8fe..d25713b09b80 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -119,16 +119,16 @@ int drm_getunique(struct drm_device *dev, void *data, struct drm_unique *u = data; struct drm_master *master; - mutex_lock(&dev->master_mutex); + down_read(&dev->master_rwsem); master = file_priv->master; if (u->unique_len >= master->unique_len) { if (copy_to_user(u->unique, master->unique, master->unique_len)) { - mutex_unlock(&dev->master_mutex); + up_read(&dev->master_rwsem); return -EFAULT; } } u->unique_len = master->unique_len; - mutex_unlock(&dev->master_mutex); + up_read(&dev->master_rwsem); return 0; } @@ -385,7 +385,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; - mutex_lock(&dev->master_mutex); + down_write(&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) { @@ -420,7 +420,7 @@ 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; - mutex_unlock(&dev->master_mutex); + up_write(&dev->master_rwsem); return retcode; } diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h index ba248ca8866f..f0a89e5fcaad 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_rwsem. */ char *unique; /** * @unique_len: Length of unique field. Protected by - * &drm_device.master_mutex. + * &drm_device.master_rwsem. */ int unique_len; /** * @magic_map: Map of used authentication tokens. Protected by - * &drm_device.master_mutex. + * &drm_device.master_rwsem. */ struct idr magic_map; void *driver_priv; diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 604b1d1b2d72..142fb2f6e74d 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -107,7 +107,7 @@ struct drm_device { * @master: * * Currently active master for this device. - * Protected by &master_mutex + * Protected by &master_rwsem */ struct drm_master *master; @@ -146,11 +146,13 @@ struct drm_device { struct mutex struct_mutex; /** - * @master_mutex: + * @master_rwsem: * - * Lock for &drm_minor.master and &drm_file.is_master + * Lock for &drm_device.master, &drm_file.was_master, + * &drm_file.is_master, &drm_file.master, &drm_master.unique, + * &drm_master.unique_len, and &drm_master.magic_map. */ - struct mutex master_mutex; + struct rw_semaphore master_rwsem; /** * @open_count: diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index a3acb7ac3550..d12bb2ba7814 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -205,7 +205,7 @@ struct drm_file { * @was_master: * * This client has or had, master capability. Protected by struct - * &drm_device.master_mutex. + * &drm_device.master_rwsem. * * This is used to ensure that CAP_SYS_ADMIN is not enforced, if the * client is or was master in the past. @@ -216,7 +216,7 @@ struct drm_file { * @is_master: * * This client is the creator of @master. Protected by struct - * &drm_device.master_mutex. + * &drm_device.master_rwsem. * * See also the :ref:`section on primary nodes and authentication * `. @@ -227,19 +227,19 @@ 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_rwsem, 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. * - * To update @master, both &drm_device.master_mutex and + * To update @master, both &drm_device.master_rwsem and * @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 - * use drm_file_get_master() if struct &drm_device.master_mutex is not + * &drm_device.master_rwsem for the duration of the pointer's use, or + * use drm_file_get_master() if struct &drm_device.master_rwsem is not * currently held and there is no other need to hold it. This prevents * @master from being freed during use. * From patchwork Tue Aug 31 06:13:47 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: 12466503 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 62869C4320E for ; Tue, 31 Aug 2021 06:15:03 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 30B2E60F4B for ; Tue, 31 Aug 2021 06:15:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 30B2E60F4B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2908E89B83; Tue, 31 Aug 2021 06:15:02 +0000 (UTC) Received: from mail-pl1-x62a.google.com (mail-pl1-x62a.google.com [IPv6:2607:f8b0:4864:20::62a]) by gabe.freedesktop.org (Postfix) with ESMTPS id AC00289B83; Tue, 31 Aug 2021 06:15:00 +0000 (UTC) Received: by mail-pl1-x62a.google.com with SMTP id bg1so4688650plb.13; Mon, 30 Aug 2021 23:15:00 -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=8hkPxmWIahwpRPq8B7GtGqA3EkMAolIN63S2SawwkKE=; b=u3DHwM2oboAS0ZS+3mC5Troi4b9j425hyQBxoON5rcNg7gbRyz6bFb/HNVYcYgRjXe QRCLG33ItY/zhpfIEpoNB9usWhaAB9rV9XlRBp4eJEaP7mJFFlhLdOXbBKpl9kJ0vnxO 9kpd9npyh9Qx6NhldeMLDO9kbZPERm5mel5aOlz46kOBFdM9PHtorvsCM+z9T+RF+mja zZd6FZTx2fo/nIzc4JovO0h9I4idoA45weme2RTDeEPL0Bcu1vXKWUhP1VZ84DJa77vu baujx/5CZlH+qX/QxlKFAyAOOgOhgXFy4BAcKCgFOtugIkWMKLGx6CgJ4VVwKwnI2OmV OJlA== 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=8hkPxmWIahwpRPq8B7GtGqA3EkMAolIN63S2SawwkKE=; b=A+sdVCJcuneB3e5TFSGN3T4qGQFozpAWvE1vTcslLgVeR4onziMgJgo2/sKETXcjx4 YndnDey8nsDFspesQe4nXGm7uNfdnttggFYJm+zssUzkBwEvYx9AenfJ1M5x93yk5BL8 FK/tUmUd0ml6+g2uiYv63TCdUrIo2Faq42Kpzu2tHLoacTfBY+qF2SFTvgrOLW1vNAzy T3jvc/wDuMNqYbnQlkbCfN46wY7FMU1qxvjHYeBPkJw2LaUzp5iLwzUlT+kxEm2arV85 UztNbqXfNIB7V7ac8PBlAgVlCoTZrkTK9dC5EhvWeAQtYBl4Si57HaiWkXfmNOy1kcAO OxvA== X-Gm-Message-State: AOAM530BPQO7pxZ/7x3yNavM7+AnKsGTo/jdtGmqyTZ72Kl3QL9W9GhI 0Jz6a8xWil8SGlgR7cNxFA8= X-Google-Smtp-Source: ABdhPJxNS3Mwi+vwp6emNlPWYdac3Ef7YicUK0bPsOF/2GZ0ApJ2i4XbFY8HEZTR5UKZBZz2OdQrRA== X-Received: by 2002:a17:90a:4a8a:: with SMTP id f10mr3424851pjh.195.1630390500318; Mon, 30 Aug 2021 23:15:00 -0700 (PDT) Received: from sanitydock.wifi-cloud.jp ([210.160.217.69]) by smtp.gmail.com with ESMTPSA id z7sm1405724pjr.42.2021.08.30.23.14.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Aug 2021 23:14:59 -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 Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, Daniel Vetter Subject: [PATCH v9 3/4] drm: lock drm_global_mutex earlier in the ioctl handler Date: Tue, 31 Aug 2021 14:13:47 +0800 Message-Id: <20210831061348.97696-4-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210831061348.97696-1-desmondcheongzx@gmail.com> References: <20210831061348.97696-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" 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 inverts the lock hierarchy of drm_global_mutex --> master_rwsem. To avoid this, we do some prep work to grab the drm_global_mutex before checking for ioctl permissions. Signed-off-by: Desmond Cheong Zhi Xi Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_ioctl.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index d25713b09b80..59c5aa850dd5 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -767,24 +767,27 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, { struct drm_file *file_priv = file->private_data; struct drm_device *dev = file_priv->minor->dev; + bool locked_ioctl; int retcode; if (drm_dev_is_unplugged(dev)) return -ENODEV; + /* Enforce sane locking for modern driver ioctls. */ + locked_ioctl = (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && + !(flags & DRM_UNLOCKED)); + if (locked_ioctl) + 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 (locked_ioctl) mutex_unlock(&drm_global_mutex); - } return retcode; } EXPORT_SYMBOL(drm_ioctl_kernel); From patchwork Tue Aug 31 06:13:48 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: 12466505 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 C224AC4320E for ; Tue, 31 Aug 2021 06:15:09 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9265260F4B for ; Tue, 31 Aug 2021 06:15:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9265260F4B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5CEBC89B96; Tue, 31 Aug 2021 06:15:08 +0000 (UTC) Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6017289B9F; Tue, 31 Aug 2021 06:15:06 +0000 (UTC) Received: by mail-pg1-x531.google.com with SMTP id 17so15682088pgp.4; Mon, 30 Aug 2021 23:15:06 -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=XOWjtPmVcuZURRHCItqd8KeCjmehloMsJa1Tvd/rfGo=; b=mYm7klhu8D1d0GjuqfIbWTRUjRNgIEAa3dL8lyoQNp0mVTIahS//yMyxbRIQDR1tAm cm/s2CR7gS51s5nM0xi6LPGiZlv09rLOPKOXoUxKwlJMn8GxUx6fFNKfhbe2Z6bbPtma 0K52TUob6x00E1p2GLsdVsUsTfgFY/FxXI9Q7bJASogTgCkIbZ7sgpacphMTmtGEfC6E 2DC2WkZCBnVHIPLJFSejF9aIeUAFT8Lt9OaiXcKsyD2kiJVH2Ytj5HGgvZRC7AOMGKFI ajUn8qFPhAblyPjCxIOwpFpn/kXha1yCrkZ5fXacFxoSrw1Cszy58viw4gWJ51fc0u46 M5+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=XOWjtPmVcuZURRHCItqd8KeCjmehloMsJa1Tvd/rfGo=; b=IdphWmqE1OaGALzQLUfIHdK5+rdulaPrpS9CdzeWGfg0WtVf12LFzBOXxiDUnE8Weu WO95s+zL94xiFpv/OfaoqCEx89MLxNtZTT5ZgayHmPzzjUign+yMkZwDy2chmSXXNVWR CIFPhTmtEXeLGy7nq2XVkTPnyob8yBblN73zM58RX0iy9pMT6NkZgNHk3tJuT1HCX9z9 XGkMhDTRoAx1zNkZXH82WcO33Wq66UjoqcI5RTZVhdlL2lFvb19llZldUljDEAaFLLu3 78itoFPYl/TS4wXC70HMNdltVvu5M2g/UJA3ChmEr3WtFZxMNPjs5RR17LkQCHcIOLcE XUPQ== X-Gm-Message-State: AOAM5328oD4x/vqyRnmqSCR5AUFoMm25zuKOt/+B/wYukbQI6FYCtMy/ u9mY8fYrpW9piQcMtxyAnxo= X-Google-Smtp-Source: ABdhPJztsAuXiccegNYT+v+yGrR62NbefH2Z/EZ4yrQw6VeBuAlR9qar3ZRXKLdP9fPvQh2G8zu7VA== X-Received: by 2002:a65:5845:: with SMTP id s5mr24426433pgr.227.1630390505792; Mon, 30 Aug 2021 23:15:05 -0700 (PDT) Received: from sanitydock.wifi-cloud.jp ([210.160.217.69]) by smtp.gmail.com with ESMTPSA id z7sm1405724pjr.42.2021.08.30.23.15.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Aug 2021 23:15:05 -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 Cc: Desmond Cheong Zhi Xi , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, Daniel Vetter Subject: [PATCH v9 4/4] drm: avoid races with modesetting rights Date: Tue, 31 Aug 2021 14:13:48 +0800 Message-Id: <20210831061348.97696-5-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210831061348.97696-1-desmondcheongzx@gmail.com> References: <20210831061348.97696-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" 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 hold a write lock. To avoid deadlocks with master_rwsem, for ioctls that need to check for modesetting permissions, but also need to hold a write lock on master_rwsem to protect some other attribute (or recurses to some function that holds a write lock, like drm_mode_create_lease_ioctl which eventually calls drm_master_open), we remove the DRM_MASTER flag and push the master_rwsem lock and permissions check into the ioctl. Reported-by: Daniel Vetter Signed-off-by: Desmond Cheong Zhi Xi Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_auth.c | 4 ++++ drivers/gpu/drm/drm_ioctl.c | 20 +++++++++++++++----- drivers/gpu/drm/drm_lease.c | 35 ++++++++++++++++++++++++----------- include/drm/drm_device.h | 6 ++++++ 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 73ade0513ccb..65065f7e1499 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -120,6 +120,10 @@ int drm_authmagic(struct drm_device *dev, void *data, DRM_DEBUG("%u\n", auth->magic); down_write(&dev->master_rwsem); + if (unlikely(!drm_is_current_master(file_priv))) { + up_write(&dev->master_rwsem); + return -EACCES; + } file = idr_find(&file_priv->master->magic_map, auth->magic); if (file) { file->authenticated = 1; diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 59c5aa850dd5..96ae6b661c0a 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -386,6 +386,10 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f int if_version, retcode = 0; down_write(&dev->master_rwsem); + if (unlikely(!drm_is_current_master(file_priv))) { + retcode = -EACCES; + goto unlock; + } 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) { @@ -420,8 +424,9 @@ 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_write(&dev->master_rwsem); +unlock: + up_write(&dev->master_rwsem); return retcode; } @@ -574,12 +579,12 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, 0), DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0), - DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER), + DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, 0), DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF(DRM_IOCTL_BLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF(DRM_IOCTL_UNBLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), - DRM_IOCTL_DEF(DRM_IOCTL_AUTH_MAGIC, drm_authmagic, DRM_MASTER), + DRM_IOCTL_DEF(DRM_IOCTL_AUTH_MAGIC, drm_authmagic, 0), DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_ADD_MAP, drm_legacy_addmap_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_RM_MAP, drm_legacy_rmmap_ioctl, DRM_AUTH), @@ -706,10 +711,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, 0), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, 0), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, 0), DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, 0), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) @@ -779,6 +784,9 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, if (locked_ioctl) 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; @@ -786,6 +794,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 (locked_ioctl) 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..bed6f7636cbe 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -500,6 +500,18 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, return -EINVAL; } + /* Clone the lessor file to create a new file for us */ + DRM_DEBUG_LEASE("Allocating lease file\n"); + lessee_file = file_clone_open(lessor_file); + if (IS_ERR(lessee_file)) + return PTR_ERR(lessee_file); + + down_read(&dev->master_rwsem); + if (unlikely(!drm_is_current_master(lessor_priv))) { + ret = -EACCES; + goto out_file; + } + lessor = drm_file_get_master(lessor_priv); /* Do not allow sub-leases */ if (lessor->lessor) { @@ -547,14 +559,6 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, goto out_leases; } - /* Clone the lessor file to create a new file for us */ - DRM_DEBUG_LEASE("Allocating lease file\n"); - lessee_file = file_clone_open(lessor_file); - if (IS_ERR(lessee_file)) { - ret = PTR_ERR(lessee_file); - goto out_lessee; - } - lessee_priv = lessee_file->private_data; /* Change the file to a master one */ drm_master_put(&lessee_priv->master); @@ -571,17 +575,19 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, fd_install(fd, lessee_file); drm_master_put(&lessor); + up_read(&dev->master_rwsem); DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n"); return 0; -out_lessee: - drm_master_put(&lessee); - out_leases: put_unused_fd(fd); out_lessor: drm_master_put(&lessor); + +out_file: + up_read(&dev->master_rwsem); + fput(lessee_file); DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret); return ret; } @@ -705,6 +711,11 @@ int drm_mode_revoke_lease_ioctl(struct drm_device *dev, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; + down_write(&dev->master_rwsem); + if (unlikely(!drm_is_current_master(lessor_priv))) { + ret = -EACCES; + goto unlock; + } lessor = drm_file_get_master(lessor_priv); mutex_lock(&dev->mode_config.idr_mutex); @@ -728,5 +739,7 @@ int drm_mode_revoke_lease_ioctl(struct drm_device *dev, mutex_unlock(&dev->mode_config.idr_mutex); drm_master_put(&lessor); +unlock: + up_write(&dev->master_rwsem); return ret; } diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 142fb2f6e74d..5504f9192408 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -151,6 +151,12 @@ struct drm_device { * Lock for &drm_device.master, &drm_file.was_master, * &drm_file.is_master, &drm_file.master, &drm_master.unique, * &drm_master.unique_len, and &drm_master.magic_map. + * + * Additionally, synchronizes access rights to exclusive resources like + * modesetting access between multiple users. For example, 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. */ struct rw_semaphore master_rwsem;