From patchwork Wed Aug 25 10:24:05 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: 12457105 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 762C5C432BE for ; Wed, 25 Aug 2021 10:26:13 +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 427D5610C7 for ; Wed, 25 Aug 2021 10:26:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 427D5610C7 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 5BCB56E1B3; Wed, 25 Aug 2021 10:26:12 +0000 (UTC) Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) by gabe.freedesktop.org (Postfix) with ESMTPS id D68B66E1A8; Wed, 25 Aug 2021 10:26:10 +0000 (UTC) Received: by mail-pj1-x1031.google.com with SMTP id qe12-20020a17090b4f8c00b00179321cbae7so4383786pjb.2; Wed, 25 Aug 2021 03:26:10 -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=Hm7YjUyiFZ/KTSC9DbXmgQsGRHrND7m8Eh1agHCitnc=; b=Pv0sswx6KV/D8D8iLbEXpPK6gv6aPKlPhIuzZJ538pPeg7Zu19TY2wFsCnnGME4d82 wCDwEjmR57/22LqSFLhTLalvNlddN72iu5RGqZrxtJl4Fm/MA/ibi4rEWki8bFcw3BZq FhIwjTU+7TZyifAU4oNOykCJaqfkvPBIyvN9eu4oRtpntdky0i+q6RqOUjNOd54ff2NJ Go8JXSz8cs/llxPn+HAJ5wIDBK4Ku56pJ+mqopZWjQsMcfatXngrcnj1jHP8unbdVhmx 4j9/RJPaZz/Ktx88hAYLUV4Iy0xMOHPut0EeJGi4IiFpqPgd9ShegS092J+LAhKieWB9 39EA== 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=Hm7YjUyiFZ/KTSC9DbXmgQsGRHrND7m8Eh1agHCitnc=; b=rnB7bG1uYO8mnobe2ZJWLROoHqygWCmkvGRFOJ9s+3udwDKotFTuK2FwBw61yrAjzK ynDNTAVl77Y7LlJ+G4bl5ut7e54HwCYZGuyAkutXFbo55fy9epkpNDVMXs9LD72tmhw+ BdRpsROun97wIgbMe9M/qBV25dd1koOrWOkrdR2/oEo6iQwwoGxvNb5A080sZ9ktNaha IqwDiUt471WB1ULHDAzsaK1tv2tMrGp7falOIyB0u62EHzcvJ0k+APD/MhYr21aZ2jg+ C2/+WOyjT1y/Zv6qUjyybeJrwM7oSrLa43wQ6fhI1F6c+rMlxKya1Y3zVkCRhNcvgG1w TLLQ== X-Gm-Message-State: AOAM532xBAVUfgXTut2ErumGzQRw88zIC3KVpurboIMgJ8Lj8URYS7rL CywQrqa2PeL/ViLXGQSdadE= X-Google-Smtp-Source: ABdhPJwk7uNf/Z8KNv7UeYe6/0UZgwLP4fBmgUIJNZd8m2slwG3kkkKx214CBAj66cPPHJFhaxKJjw== X-Received: by 2002:a17:902:c950:b029:12d:2ada:9ef7 with SMTP id i16-20020a170902c950b029012d2ada9ef7mr37392203pla.61.1629887170492; Wed, 25 Aug 2021 03:26:10 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id t42sm10228377pfg.30.2021.08.25.03.26.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 03:26:10 -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, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, chris@chris-wilson.co.uk, ville.syrjala@linux.intel.com, matthew.auld@intel.com, dan.carpenter@oracle.com, tvrtko.ursulin@intel.com, matthew.d.roper@intel.com, lucas.demarchi@intel.com, karthik.b.s@intel.com, jose.souza@intel.com, manasi.d.navare@intel.com, airlied@redhat.com, aditya.swarup@intel.com, andrescj@chromium.org, linux-graphics-maintainer@vmware.com, zackr@vmware.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 v6 1/7] drm: fix null ptr dereference in drm_master_release Date: Wed, 25 Aug 2021 18:24:05 +0800 Message-Id: <20210825102411.1084220-2-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210825102411.1084220-1-desmondcheongzx@gmail.com> References: <20210825102411.1084220-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. Signed-off-by: Desmond Cheong Zhi Xi --- 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 Wed Aug 25 10:24:06 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: 12457107 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 F0833C4338F for ; Wed, 25 Aug 2021 10:26:21 +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 BBB5060FE6 for ; Wed, 25 Aug 2021 10:26:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BBB5060FE6 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 4B5356E1A4; Wed, 25 Aug 2021 10:26:21 +0000 (UTC) Received: from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com [IPv6:2607:f8b0:4864:20::52b]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5AAEB6E1A4; Wed, 25 Aug 2021 10:26:20 +0000 (UTC) Received: by mail-pg1-x52b.google.com with SMTP id s11so22632611pgr.11; Wed, 25 Aug 2021 03:26:20 -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=bkFQht7vR3ZjKExLOYXZei3qepyeU4QeAm6JOJ26wAc=; b=NtRGH4yKpFkwT9szm26rPLHFulE/7tnh55YNFnhAUaf8ZMRg4ktw/Vs154Hq0rSJH/ 494DM7NG9qcUuHvCg56Wf8yREDiX7TwF6H+rf5ryR6lNj+wjrYGn3E+HOZG3Q79QbMiJ PHZ4TBtA43XG2XN/utM0Ih8hB6L2PHxxmi5OBp7um57THIwXtRoC+ogUF3yDQFzfixZ4 QqSU1v+h3QmbAYcN4PUafiVtO5jyR2oI0ct+zgXafiVP0H+3MnrTG7KZc+BRD/ytoRm3 jZrDiWeRMJ/4iAeaBdbKyT8zbNYpl0H7PCaCYsuDze3HUJsQx+022oF3ihTb1UcYz6IN 4x6w== 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=bkFQht7vR3ZjKExLOYXZei3qepyeU4QeAm6JOJ26wAc=; b=LUcqXOxUqs3MHZG7cHWw7ij4FnhF/EHjFKNv8E3iIsA9yn97fhgRWPxBdrVEVd9cNm VgKV990lXB0xZ9LgWhxTuEoIW4mOD7DVkQCPLnQkKzbuMPghmgTTYJsjCmvVQh705GIn 5M7v1pyERdbVWKKCS0lz/gpIg7QaRz9AJdgdmAPjaCy0hewFDqRwMpRiJ3BNl3ifKSCe XPiLuEW4J8xz0cJtIiczmYFhkfF379qlAF8ZekjV062uTnKW1LW3KiQAbpIuswVJcCDY NXfhvgja7XTtIsx9odrn1+BNCi4JExy/fbkG64qzlQ+zTg1bFpd4HZy+DWYRfazuFoLx B8KQ== X-Gm-Message-State: AOAM530Ma5et8acILgtNqRFs3Dn7DE1XCigqRy20MInXi8lUhRk4mFfU XYNwKgn3LtxsI6SWPfsqrCA= X-Google-Smtp-Source: ABdhPJyYsZXZCGInVr1q6ufjXN783I1tYbCMT1RbbFUhtKELqAQQliPdlNARB0ZbGMXkpT5pJ2NRkA== X-Received: by 2002:aa7:8151:0:b029:3e0:bba6:9754 with SMTP id d17-20020aa781510000b02903e0bba69754mr44482728pfn.19.1629887179839; Wed, 25 Aug 2021 03:26:19 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id t42sm10228377pfg.30.2021.08.25.03.26.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 03:26:19 -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, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, chris@chris-wilson.co.uk, ville.syrjala@linux.intel.com, matthew.auld@intel.com, dan.carpenter@oracle.com, tvrtko.ursulin@intel.com, matthew.d.roper@intel.com, lucas.demarchi@intel.com, karthik.b.s@intel.com, jose.souza@intel.com, manasi.d.navare@intel.com, airlied@redhat.com, aditya.swarup@intel.com, andrescj@chromium.org, linux-graphics-maintainer@vmware.com, zackr@vmware.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 v6 2/7] drm: convert drm_device.master_mutex into a rwsem Date: Wed, 25 Aug 2021 18:24:06 +0800 Message-Id: <20210825102411.1084220-3-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210825102411.1084220-1-desmondcheongzx@gmail.com> References: <20210825102411.1084220-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 --- 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 Wed Aug 25 10:24:07 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: 12457109 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 06C3AC4338F for ; Wed, 25 Aug 2021 10:26:31 +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 C4132611C9 for ; Wed, 25 Aug 2021 10:26:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C4132611C9 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 1619E8994D; Wed, 25 Aug 2021 10:26:30 +0000 (UTC) Received: from mail-pl1-x62e.google.com (mail-pl1-x62e.google.com [IPv6:2607:f8b0:4864:20::62e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 660B489913; Wed, 25 Aug 2021 10:26:29 +0000 (UTC) Received: by mail-pl1-x62e.google.com with SMTP id j2so9635398pll.1; Wed, 25 Aug 2021 03:26:29 -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=P/HLeCdm1ln+WDWlHx3gbH2c2TGPxEmJ7R7sNcfLOXw=; b=mcrlqXx2veIFAPIuR1zZjjMGBGC3pLIA2IFrCCQwW8jK9wRON0L+mvneUtG9EWMRx+ 7d7b5AXWPfLm1KEB+5LTnOhEipuf9Di9/CY1lUxuUiDbu4v9A0doKF0Qm6poKea5VseM ko4jRSDZrGQ2WXvHQx7y+9kGiXJtIFws3WUyprbvinMgQ4n6Qa9xbgJM1o7xdsImwYt+ jNyeTWPF5243/7Qr5xB2lsXL4BXLNs/euuJ3AId/5+8W+fGHy+mP8eAJf8vH4fD8GXIa 7zXINxn+uv8DZbkky03Hg1aWUm/9c9gILHsj+0dX8m4Jw3w8F3dvcfvnnKkpIajiXCE7 0Z8Q== 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=P/HLeCdm1ln+WDWlHx3gbH2c2TGPxEmJ7R7sNcfLOXw=; b=a5bCJLQsO+rcp5k4Utv449hoSt2b68sr9IpXdW96YjeXrjhjUrSGVdBuMPCZhlnIt8 UkTXH7ZjWv2BUdL631TwPvGIdeQUbTM4FgqpszgrcX32C8rM6SGHdDt1mnPRkbGBVBfb l8GLmVZJBYVKAS64Kd3oMet19/pBmOOA59ICIMxn9NUGyNlo29AgBIQnOphBiDzeCS9k kfn50sXwzDLK6cNJGRaG2a6RgdRvM3/E2Z4alEV3mcXMpqUVZ/zOipY85Z07o2fcpbs6 ZvVm56BHeIBYjKy3NJhQO0raXLtp4iBAdNvG2RU0iAmn4nLIRJ5nqKkeeHEg0wJC5UZE pVVQ== X-Gm-Message-State: AOAM533SLNmedWIlqK4aUf+yTkhULf8nKKsD2X+TvZ7+jOomcB74dR4I dRCJFQ/yJIdHyJd52WhA2ew= X-Google-Smtp-Source: ABdhPJzePDrz0gmAXJF/b1c+ZQtAypG8eji/dWUqtQ+OfAnYEoiu9NXIpjQUuVh17nWzvMUuZRXSnQ== X-Received: by 2002:a17:902:bcc5:b0:133:1943:b48b with SMTP id o5-20020a170902bcc500b001331943b48bmr17088082pls.52.1629887188912; Wed, 25 Aug 2021 03:26:28 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id t42sm10228377pfg.30.2021.08.25.03.26.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 03:26:28 -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, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, chris@chris-wilson.co.uk, ville.syrjala@linux.intel.com, matthew.auld@intel.com, dan.carpenter@oracle.com, tvrtko.ursulin@intel.com, matthew.d.roper@intel.com, lucas.demarchi@intel.com, karthik.b.s@intel.com, jose.souza@intel.com, manasi.d.navare@intel.com, airlied@redhat.com, aditya.swarup@intel.com, andrescj@chromium.org, linux-graphics-maintainer@vmware.com, zackr@vmware.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 v6 3/7] drm: lock drm_global_mutex earlier in the ioctl handler Date: Wed, 25 Aug 2021 18:24:07 +0800 Message-Id: <20210825102411.1084220-4-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210825102411.1084220-1-desmondcheongzx@gmail.com> References: <20210825102411.1084220-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 --- 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 d25713b09b80..158629d88319 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -772,19 +772,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_core_check_feature(dev, DRIVER_LEGACY)) && !(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_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED)) mutex_unlock(&drm_global_mutex); - } return retcode; } EXPORT_SYMBOL(drm_ioctl_kernel); From patchwork Wed Aug 25 10:24:08 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: 12457111 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 7C1B2C432BE for ; Wed, 25 Aug 2021 10:26:42 +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 4D125610C7 for ; Wed, 25 Aug 2021 10:26:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4D125610C7 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 05210899AB; Wed, 25 Aug 2021 10:26:41 +0000 (UTC) Received: from mail-pj1-x1033.google.com (mail-pj1-x1033.google.com [IPv6:2607:f8b0:4864:20::1033]) by gabe.freedesktop.org (Postfix) with ESMTPS id B2F78899A7; Wed, 25 Aug 2021 10:26:39 +0000 (UTC) Received: by mail-pj1-x1033.google.com with SMTP id z24-20020a17090acb1800b0018e87a24300so4409260pjt.0; Wed, 25 Aug 2021 03:26:39 -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=rtuBBmsJ+FZGp2Lavog/mblfNKKX32jG6r2O7uMrLj0=; b=uNmqIbqM9BaR2y0b4Wp8NB2/PGxcq//1Uo6fu5LtTD77WugTgNKaDSsrG4FMlKo042 0sZQTFI7zgAmpm3MYtqXoR1l1yQ3c9dxIXuPNuWP8ZQrRrPNePyDpWcoo4y5NHeNBQ2/ 0Vis5xtZ2pwSNtOzG7nfZA/fuC7HQLJPyC8BRD/BDIamhu9y2wWSmz5JDjqaaub9WS3V fEUUD3v0ZtHWmg+GHZQvEXOOtmiegf8ITXW2sjuzbUwcX+j9SXKK/cn6OO6alsfkq0Z9 0ZMa08VtTFpWg17x0dnA8HiGo1NWvBeCJ/j1FzENBil3slZSDWh4pTC2Swp8eD+qx7D+ /JsQ== 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=rtuBBmsJ+FZGp2Lavog/mblfNKKX32jG6r2O7uMrLj0=; b=b+U9s6z4LQKUZEjHVVBTgL6sUfwBSfjhYPJQwFzmOgJ0I/hvPhaLrJ2IJR0i785sFo zKsdn26tqKaVAmoP2Ss2pFzshXe7OaA+drdPTfL6MzNREPaTFKpFENOYJ9XL2eBieER5 VdrzFzlto8XH5uSUkcKzkL78fM3wJF6MnL1OYvh8NNeifF1RJ2bcXnpVeDL2qbgUPOvN eaWug+guFlJ4gWk45hRs6RKhGEefzaNDfQuNW4DlZj8IkF90dqOPeKpd5R0OYandHU5a tK1iJJoQpwe6YaLhsMywD05z8x8ej6bvMRA/0cK2xfIWQ03CJ+sLF9RMoJgZVxou/yMD 3VLg== X-Gm-Message-State: AOAM532jHZBgJEkGvso/ivYwlhdIL9GUi6DcAeef87Z8eFdfLKwYbrZh M8q7ZCamHGht12A9JlWSAvw= X-Google-Smtp-Source: ABdhPJykTvjiPSdy+hQPBVXpJRKpZWOaj6fEsQfMzlaSCQu2SRXqrIlSqO075jxG74SuYoXK/cTUVw== X-Received: by 2002:a17:902:ee93:b0:133:f9fa:f3c1 with SMTP id a19-20020a170902ee9300b00133f9faf3c1mr14456314pld.82.1629887199337; Wed, 25 Aug 2021 03:26:39 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id t42sm10228377pfg.30.2021.08.25.03.26.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 03:26:38 -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, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, chris@chris-wilson.co.uk, ville.syrjala@linux.intel.com, matthew.auld@intel.com, dan.carpenter@oracle.com, tvrtko.ursulin@intel.com, matthew.d.roper@intel.com, lucas.demarchi@intel.com, karthik.b.s@intel.com, jose.souza@intel.com, manasi.d.navare@intel.com, airlied@redhat.com, aditya.swarup@intel.com, andrescj@chromium.org, linux-graphics-maintainer@vmware.com, zackr@vmware.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 v6 4/7] drm: avoid races with modesetting rights Date: Wed, 25 Aug 2021 18:24:08 +0800 Message-Id: <20210825102411.1084220-5-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210825102411.1084220-1-desmondcheongzx@gmail.com> References: <20210825102411.1084220-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 --- 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 | 5 +++++ 4 files changed, 48 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 158629d88319..8bea39ffc5c0 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 ) @@ -776,6 +781,9 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(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; @@ -783,6 +791,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_core_check_feature(dev, DRIVER_LEGACY)) && !(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..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..7d32bb69e6db 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -151,6 +151,11 @@ 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 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. */ struct rw_semaphore master_rwsem; From patchwork Wed Aug 25 10:24:09 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: 12457119 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 F1854C432BE for ; Wed, 25 Aug 2021 10:26:56 +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 C39CC61184 for ; Wed, 25 Aug 2021 10:26:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C39CC61184 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 E73016E1BA; Wed, 25 Aug 2021 10:26:55 +0000 (UTC) Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3F3456E1A7; Wed, 25 Aug 2021 10:26:49 +0000 (UTC) Received: by mail-pj1-x1029.google.com with SMTP id n13-20020a17090a4e0d00b0017946980d8dso3825827pjh.5; Wed, 25 Aug 2021 03:26:49 -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=Kd8L1k/K0XDx37szGGI10im4dGkDr2a+HW5mTISpLDU=; b=auHzLy+DxwzceRQGnI50O4/DBmI5k2Nn8HIUK7cef8kneBYH1k2lSEDcD35wxcffyX LRIDagOseE2F97YsjkEavR3nDr2o3SFEOiWYk0Yl8cBAiSO2AfzOmd4KUj5WPr0ST41f GeCoDNJJ0ZEnoR5iwUXQg9/CTKjPlzA5NN5ipmTe0kga52AWizsnF8FagNZVuP+PDqw2 mZGb5EJyZTQW+1UqBO00itpe46RGlgqrHBNwk8QUo/NHGFuJzLpr56QTUK0Y3Sedg//s eNn+zqHXRRs+8nC4BQVXanzdy0mX981W5xd3T0vcbx6VX4siYVKVzUWAxiHSmsEAnHbU 8CbA== 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=Kd8L1k/K0XDx37szGGI10im4dGkDr2a+HW5mTISpLDU=; b=DXgNX12Wd2QZ07lGBkzXH/lF5U14ynTTMGpdOKQVUYxb2jYt+gE6PExAc1v53iIqyx F3lPTzlq7gUmP5GlouTJ3Yr6eXq1aUx/CfX0+g+HdV87MLcI2jDOIqxz0sRUP646EZxz Uj3jBnCXjrkGMf5IIPP6GlqkQu/x/p+483iOnJFlepTtv5WOzIDMyO53Dw4o+p8s3pLT X23MG4hfOenXxE/Ss5y8AFp86kdsPDZzc4txDUipOX4keLuqV9gg5Co6oPVXRbohfAYw oklpwuWIIPsNLamYA8emu5Yu4OwxcSksb1LtKBb2W8tzMoEJ/hKraioRj0kg8bv3xzzl 5zVw== X-Gm-Message-State: AOAM532XNslTDCwJDf8XLxwYkrMnmIqTUVApPK/wMr1bV+FHUx9fxc2D jguuBPdW6xJK8tXMk0YVu+8= X-Google-Smtp-Source: ABdhPJyHtIrYW2M1It2OiYi7ednBivyimPRYxCQkEKvgMqhfz7F+yWqrA+UeVPsfAerHDvk7d+P05A== X-Received: by 2002:a17:90a:3b0e:: with SMTP id d14mr9850352pjc.164.1629887208714; Wed, 25 Aug 2021 03:26:48 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id t42sm10228377pfg.30.2021.08.25.03.26.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 03:26:48 -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, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, chris@chris-wilson.co.uk, ville.syrjala@linux.intel.com, matthew.auld@intel.com, dan.carpenter@oracle.com, tvrtko.ursulin@intel.com, matthew.d.roper@intel.com, lucas.demarchi@intel.com, karthik.b.s@intel.com, jose.souza@intel.com, manasi.d.navare@intel.com, airlied@redhat.com, aditya.swarup@intel.com, andrescj@chromium.org, linux-graphics-maintainer@vmware.com, zackr@vmware.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 v6 5/7] drm: avoid circular locks in drm_mode_object_find Date: Wed, 25 Aug 2021 18:24:09 +0800 Message-Id: <20210825102411.1084220-6-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210825102411.1084220-1-desmondcheongzx@gmail.com> References: <20210825102411.1084220-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_mode_object_find checks if the given drm file holds the required lease on a object by calling _drm_lease_held. _drm_lease_held in turn uses drm_file_get_master to access drm_file.master. However, in a future patch, the drm_file.master_lookup_lock in drm_file_get_master will be replaced by drm_device.master_rwsem. This is an issue for two reasons: 1. master_rwsem is sometimes already held when __drm_mode_object_find is called, which leads to recursive locks on master_rwsem 2. drm_mode_object_find is sometimes called with the modeset_mutex held, which leads to an inversion of the master_rwsem --> modeset_mutex lock hierarchy To fix this, we make __drm_mode_object_find the locked version of drm_mode_object_find, and wrap calls to __drm_mode_object_find with locks on master_rwsem. This allows us to safely access drm_file.master in _drm_lease_held (__drm_mode_object_find is its only caller) without the use of drm_file_get_master. Functions that already lock master_rwsem are modified to call __drm_mode_object_find, whereas functions that haven't locked master_rwsem should call drm_mode_object_find. These two options allow us to grab master_rwsem before modeset_mutex (such as in drm_mode_get_obj_get_properties_ioctl). This new rule requires more extensive changes to three functions: drn_connector_lookup, drm_crtc_find, and drm_plane_find. These functions are only sometimes called with master_rwsem held. Hence, we have to further split them into locked and unlocked versions that call __drm_mode_object_find and drm_mode_object_find respectively. Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_atomic_uapi.c | 7 +++--- drivers/gpu/drm/drm_color_mgmt.c | 2 +- drivers/gpu/drm/drm_crtc.c | 5 +++-- drivers/gpu/drm/drm_framebuffer.c | 2 +- drivers/gpu/drm/drm_lease.c | 21 +++++------------- drivers/gpu/drm/drm_mode_object.c | 13 ++++++++--- drivers/gpu/drm/drm_plane.c | 8 +++---- drivers/gpu/drm/drm_property.c | 6 ++--- drivers/gpu/drm/i915/display/intel_overlay.c | 2 +- drivers/gpu/drm/i915/display/intel_sprite.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- include/drm/drm_connector.h | 23 ++++++++++++++++++++ include/drm/drm_crtc.h | 22 +++++++++++++++++++ include/drm/drm_mode_object.h | 3 +++ include/drm/drm_plane.h | 20 +++++++++++++++++ 15 files changed, 103 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 909f31833181..cda9a501cf74 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -557,7 +557,7 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, return -EINVAL; } else if (property == config->prop_crtc_id) { - struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val); + struct drm_crtc *crtc = __drm_crtc_find(dev, file_priv, val); if (val && !crtc) return -EACCES; @@ -709,7 +709,7 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, int ret; if (property == config->prop_crtc_id) { - struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val); + struct drm_crtc *crtc = __drm_crtc_find(dev, file_priv, val); if (val && !crtc) return -EACCES; @@ -1385,7 +1385,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, goto out; } - obj = drm_mode_object_find(dev, file_priv, obj_id, DRM_MODE_OBJECT_ANY); + obj = __drm_mode_object_find(dev, file_priv, obj_id, + DRM_MODE_OBJECT_ANY); if (!obj) { ret = -ENOENT; goto out; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6..9dcb2ccca3ab 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -365,7 +365,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; - crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id); + crtc = __drm_crtc_find(dev, file_priv, crtc_lut->crtc_id); if (!crtc) return -ENOENT; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 26a77a735905..b1279bb3fa61 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -656,7 +656,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, if (crtc_req->x & 0xffff0000 || crtc_req->y & 0xffff0000) return -ERANGE; - crtc = drm_crtc_find(dev, file_priv, crtc_req->crtc_id); + crtc = __drm_crtc_find(dev, file_priv, crtc_req->crtc_id); if (!crtc) { DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_req->crtc_id); return -ENOENT; @@ -787,7 +787,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, goto out; } - connector = drm_connector_lookup(dev, file_priv, out_id); + connector = __drm_connector_lookup(dev, file_priv, + out_id); if (!connector) { DRM_DEBUG_KMS("Connector id %d unknown\n", out_id); diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 07f5abc875e9..9c1db91b150d 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -887,7 +887,7 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev, struct drm_mode_object *obj; struct drm_framebuffer *fb = NULL; - obj = __drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_FB); + obj = drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_FB); if (obj) fb = obj_to_fb(obj); return fb; diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index bed6f7636cbe..1b156c85d1c8 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -105,22 +105,13 @@ static bool _drm_has_leased(struct drm_master *master, int id) return false; } -/* Called with idr_mutex held */ +/* Called with idr_mutex and master_rwsem held */ bool _drm_lease_held(struct drm_file *file_priv, int id) { - bool ret; - struct drm_master *master; - - if (!file_priv) + if (!file_priv || !file_priv->master) return true; - master = drm_file_get_master(file_priv); - if (!master) - return true; - ret = _drm_lease_held_master(master, id); - drm_master_put(&master); - - return ret; + return _drm_lease_held_master(file_priv->master, id); } bool drm_lease_held(struct drm_file *file_priv, int id) @@ -391,9 +382,9 @@ static int fill_object_idr(struct drm_device *dev, /* step one - get references to all the mode objects and check for validity. */ for (o = 0; o < object_count; o++) { - objects[o] = drm_mode_object_find(dev, lessor_priv, - object_ids[o], - DRM_MODE_OBJECT_ANY); + objects[o] = __drm_mode_object_find(dev, lessor_priv, + object_ids[o], + DRM_MODE_OBJECT_ANY); if (!objects[o]) { ret = -ENOENT; goto out_free_objects; diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 86d9e907c0b2..2e068b96926f 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -135,10 +135,11 @@ bool drm_mode_object_lease_required(uint32_t type) struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev, struct drm_file *file_priv, - uint32_t id, uint32_t type) + u32 id, u32 type) { struct drm_mode_object *obj = NULL; + lockdep_assert_held_once(&dev->master_rwsem); mutex_lock(&dev->mode_config.idr_mutex); obj = idr_find(&dev->mode_config.object_idr, id); if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type) @@ -176,7 +177,9 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, { struct drm_mode_object *obj = NULL; + down_read(&dev->master_rwsem); obj = __drm_mode_object_find(dev, file_priv, id, type); + up_read(&dev->master_rwsem); return obj; } EXPORT_SYMBOL(drm_mode_object_find); @@ -408,9 +411,12 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; + down_read(&dev->master_rwsem); DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); - obj = drm_mode_object_find(dev, file_priv, arg->obj_id, arg->obj_type); + obj = __drm_mode_object_find(dev, file_priv, arg->obj_id, + arg->obj_type); + up_read(&dev->master_rwsem); if (!obj) { ret = -ENOENT; goto out; @@ -534,7 +540,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; - arg_obj = drm_mode_object_find(dev, file_priv, arg->obj_id, arg->obj_type); + arg_obj = __drm_mode_object_find(dev, file_priv, arg->obj_id, + arg->obj_type); if (!arg_obj) return -ENOENT; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 82afb854141b..b5566167a798 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -971,7 +971,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, * First, find the plane, crtc, and fb objects. If not available, * we don't bother to call the driver. */ - plane = drm_plane_find(dev, file_priv, plane_req->plane_id); + plane = __drm_plane_find(dev, file_priv, plane_req->plane_id); if (!plane) { DRM_DEBUG_KMS("Unknown plane ID %d\n", plane_req->plane_id); @@ -986,7 +986,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, return -ENOENT; } - crtc = drm_crtc_find(dev, file_priv, plane_req->crtc_id); + crtc = __drm_crtc_find(dev, file_priv, plane_req->crtc_id); if (!crtc) { drm_framebuffer_put(fb); DRM_DEBUG_KMS("Unknown crtc ID %d\n", @@ -1108,7 +1108,7 @@ static int drm_mode_cursor_common(struct drm_device *dev, if (!req->flags || (~DRM_MODE_CURSOR_FLAGS & req->flags)) return -EINVAL; - crtc = drm_crtc_find(dev, file_priv, req->crtc_id); + crtc = __drm_crtc_find(dev, file_priv, req->crtc_id); if (!crtc) { DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id); return -ENOENT; @@ -1229,7 +1229,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip) return -EINVAL; - crtc = drm_crtc_find(dev, file_priv, page_flip->crtc_id); + crtc = __drm_crtc_find(dev, file_priv, page_flip->crtc_id); if (!crtc) return -ENOENT; diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index 6c353c9dc772..9f04dcb81d07 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -656,7 +656,7 @@ struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev, struct drm_mode_object *obj; struct drm_property_blob *blob = NULL; - obj = __drm_mode_object_find(dev, NULL, id, DRM_MODE_OBJECT_BLOB); + obj = drm_mode_object_find(dev, NULL, id, DRM_MODE_OBJECT_BLOB); if (obj) blob = obj_to_blob(obj); return blob; @@ -919,8 +919,8 @@ bool drm_property_change_valid_get(struct drm_property *property, if (value == 0) return true; - *ref = __drm_mode_object_find(property->dev, NULL, value, - property->values[0]); + *ref = drm_mode_object_find(property->dev, NULL, value, + property->values[0]); return *ref != NULL; } diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c index 7e3f5c6ca484..1d4af29e31c9 100644 --- a/drivers/gpu/drm/i915/display/intel_overlay.c +++ b/drivers/gpu/drm/i915/display/intel_overlay.c @@ -1120,7 +1120,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data, return ret; } - drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id); + drmmode_crtc = __drm_crtc_find(dev, file_priv, params->crtc_id); if (!drmmode_crtc) return -ENOENT; crtc = to_intel_crtc(drmmode_crtc); diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index 4ae9a7455b23..e19ef2d90bac 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -1505,7 +1505,7 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data, set->flags & I915_SET_COLORKEY_DESTINATION) return -EINVAL; - plane = drm_plane_find(dev, file_priv, set->plane_id); + plane = __drm_plane_find(dev, file_priv, set->plane_id); if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY) return -ENOENT; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 74fa41909213..d368b2bcb1fa 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -1862,7 +1862,7 @@ int vmw_kms_cursor_bypass_ioctl(struct drm_device *dev, void *data, return 0; } - crtc = drm_crtc_find(dev, file_priv, arg->crtc_id); + crtc = __drm_crtc_find(dev, file_priv, arg->crtc_id); if (!crtc) { ret = -ENOENT; goto out; diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 1647960c9e50..322c823583c7 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1591,6 +1591,29 @@ static inline u32 drm_connector_mask(const struct drm_connector *connector) return 1 << connector->index; } +/** + * __drm_connector_lookup - lookup connector object + * @dev: DRM device + * @file_priv: drm file to check for lease against. + * @id: connector object id + * + * This function looks up the connector object specified by id + * add takes a reference to it. + * + * Similar to drm_connector_lookup(), but called with &drm_device.master_rwsem + * held. + */ +static inline struct drm_connector *__drm_connector_lookup(struct drm_device *dev, + struct drm_file *file_priv, + uint32_t id) +{ + struct drm_mode_object *mo; + + mo = __drm_mode_object_find(dev, file_priv, id, + DRM_MODE_OBJECT_CONNECTOR); + return mo ? obj_to_connector(mo) : NULL; +} + /** * drm_connector_lookup - lookup connector object * @dev: DRM device diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 13eeba2a750a..69df854dd322 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1283,6 +1283,28 @@ static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc) int drm_mode_set_config_internal(struct drm_mode_set *set); struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx); +/** + * __drm_crtc_find - look up a CRTC object from its ID + * @dev: DRM device + * @file_priv: drm file to check for lease against. + * @id: &drm_mode_object ID + * + * This can be used to look up a CRTC from its userspace ID. Only used by + * drivers for legacy IOCTLs and interface, nowadays extensions to the KMS + * userspace interface should be done using &drm_property. + * + * Similar to drm_crtc_find(), but called with &drm_device.master_rwsem held. + */ +static inline struct drm_crtc *__drm_crtc_find(struct drm_device *dev, + struct drm_file *file_priv, + uint32_t id) +{ + struct drm_mode_object *mo; + + mo = __drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_CRTC); + return mo ? obj_to_crtc(mo) : NULL; +} + /** * drm_crtc_find - look up a CRTC object from its ID * @dev: DRM device diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index c34a3e8030e1..1906af9cae9b 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -114,6 +114,9 @@ struct drm_object_properties { return "(unknown)"; \ } +struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev, + struct drm_file *file_priv, + u32 id, u32 type); struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, struct drm_file *file_priv, uint32_t id, uint32_t type); diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index fed97e35626f..49e35d3724c9 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -842,6 +842,26 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane, struct drm_property *property, uint64_t value); +/** + * __drm_plane_find - find a &drm_plane + * @dev: DRM device + * @file_priv: drm file to check for lease against. + * @id: plane id + * + * Returns the plane with @id, NULL if it doesn't exist. + * + * Similar to drm_plane_find(), but called with &drm_device.master_rwsem held. + */ +static inline struct drm_plane *__drm_plane_find(struct drm_device *dev, + struct drm_file *file_priv, + uint32_t id) +{ + struct drm_mode_object *mo; + + mo = __drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_PLANE); + return mo ? obj_to_plane(mo) : NULL; +} + /** * drm_plane_find - find a &drm_plane * @dev: DRM device From patchwork Wed Aug 25 10:24:10 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: 12457121 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 15BACC4320A for ; Wed, 25 Aug 2021 10:27:01 +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 D99ED61184 for ; Wed, 25 Aug 2021 10:27:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D99ED61184 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 D24856E1A7; Wed, 25 Aug 2021 10:26:59 +0000 (UTC) Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by gabe.freedesktop.org (Postfix) with ESMTPS id 03F316E1BD; Wed, 25 Aug 2021 10:26:58 +0000 (UTC) Received: by mail-pj1-x1032.google.com with SMTP id j1so16218676pjv.3; Wed, 25 Aug 2021 03:26:57 -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=uk23yrQj2bYGx5iUQvrvtVaf8WaEKXc6e3h1Qpr0Wxc=; b=BWSKA8AUjVQ50e47odJHTUdgySf+Ui5SxD63HSnvhT3Y2nW/W0LeIK7ED0XXqlgOPp CnsORRw46Z0JjgWe4AAS+3YKmgLO8GFneYav7pypBwEoTFNCsPlberP+D66TxmKp9zGl bDgTWZNHKNUbpQ/UlWFVE2rwq4pfH/42C3u/yCdpo4ffhPFp9xooRzeQTPl79EEtYNqT 7uXvOAApPrrARRsuZmogj9TBFTMqJfZnALu0o8OujnrBJ1Eo0AOSSOadPyLqxxfhkURH ve+E6G3HuuqLW4pSke2ZTWl4gScZpH/UHzFkdbpvUfaykvjzBjYoXchp8pFKfaNqH21u wO9Q== 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=uk23yrQj2bYGx5iUQvrvtVaf8WaEKXc6e3h1Qpr0Wxc=; b=qHHe8l6e1x1ZVm+yMKBq32dgyU8FsHsQn5d98lSC/M9L2WmLyMMIMQnHICwglqVdcR GazxTu59fvSdIUd7QObqkgrnevVCvN+DzZvHdeSUgfPNq2LLh+/mpJxlWYscdsUO8kyj 1hPSFkSd4oTYSY29Icl581zD1CDg9euBabh7q1LNWzj/5x9ouR7vId3Y0bTLhsPBDgsx /9/1d8xXvzJnYQoyIIYLmdN2Hx41QnvdGDXsXjZs4jtpkmFHED2WLH3q+fxCcXJR0KxC ugCw8BD8x4GeFmPEBDPWj1rW2j1IcOJ21iRMtpJTFtXXuNPBb0XwtfryHfqreD6NzElT Pfaw== X-Gm-Message-State: AOAM531ULT+2p8TX5A/ydvyE9dUniIMysRLqezAEwBSEdiFrBRQtYP7G SXDTWv8qz2ZqYaFC4Jnd0KU= X-Google-Smtp-Source: ABdhPJytkjiDZ8qPb5DBfLVt7ff+o3cMdp+LthsSPCnnII0Plz6+Bb78f8IMUv18k6LCIVbqrGYgJA== X-Received: by 2002:a17:90b:384a:: with SMTP id nl10mr9780352pjb.65.1629887217646; Wed, 25 Aug 2021 03:26:57 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id t42sm10228377pfg.30.2021.08.25.03.26.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 03:26:57 -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, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, chris@chris-wilson.co.uk, ville.syrjala@linux.intel.com, matthew.auld@intel.com, dan.carpenter@oracle.com, tvrtko.ursulin@intel.com, matthew.d.roper@intel.com, lucas.demarchi@intel.com, karthik.b.s@intel.com, jose.souza@intel.com, manasi.d.navare@intel.com, airlied@redhat.com, aditya.swarup@intel.com, andrescj@chromium.org, linux-graphics-maintainer@vmware.com, zackr@vmware.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 v6 6/7] drm: avoid circular locks with modeset_mutex and master_rwsem Date: Wed, 25 Aug 2021 18:24:10 +0800 Message-Id: <20210825102411.1084220-7-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210825102411.1084220-1-desmondcheongzx@gmail.com> References: <20210825102411.1084220-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_lease_held calls drm_file_get_master. However, this function is sometimes called while holding on to modeset_mutex. Since drm_device.master_rwsem will replace drm_file.master_lookup_lock in drm_file_get_master in a future patch, this inverts the master_rwsem --> modeset_mutex lock hierarchy. To fix this, we create a new drm_lease_held_master helper function that enables us to avoid calling drm_file_get_master after locking modeset_mutex. Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_auth.c | 3 +++ drivers/gpu/drm/drm_encoder.c | 7 ++++++- drivers/gpu/drm/drm_lease.c | 30 +++++++++++++++--------------- drivers/gpu/drm/drm_plane.c | 14 +++++++++++--- include/drm/drm_lease.h | 2 ++ 5 files changed, 37 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 65065f7e1499..f2b2f197052a 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -410,6 +410,9 @@ struct drm_master *drm_file_get_master(struct drm_file *file_priv) { struct drm_master *master = NULL; + if (!file_priv) + return NULL; + spin_lock(&file_priv->master_lookup_lock); if (!file_priv->master) goto unlock; diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 72e982323a5e..bacb2f6a325c 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -22,6 +22,7 @@ #include +#include #include #include #include @@ -281,6 +282,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, struct drm_mode_get_encoder *enc_resp = data; struct drm_encoder *encoder; struct drm_crtc *crtc; + struct drm_master *master; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; @@ -289,13 +291,16 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, if (!encoder) return -ENOENT; + master = drm_file_get_master(file_priv); drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); crtc = drm_encoder_get_crtc(encoder); - if (crtc && drm_lease_held(file_priv, crtc->base.id)) + if (crtc && drm_lease_held_master(master, crtc->base.id)) enc_resp->crtc_id = crtc->base.id; else enc_resp->crtc_id = 0; drm_modeset_unlock(&dev->mode_config.connection_mutex); + if (master) + drm_master_put(&master); enc_resp->encoder_type = encoder->encoder_type; enc_resp->encoder_id = encoder->base.id; diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 1b156c85d1c8..15bf3a3c76d1 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -114,27 +114,30 @@ bool _drm_lease_held(struct drm_file *file_priv, int id) return _drm_lease_held_master(file_priv->master, id); } -bool drm_lease_held(struct drm_file *file_priv, int id) +bool drm_lease_held_master(struct drm_master *master, int id) { - struct drm_master *master; bool ret; - if (!file_priv) + if (!master || !master->lessor) return true; - master = drm_file_get_master(file_priv); - if (!master) - return true; - if (!master->lessor) { - ret = true; - goto out; - } mutex_lock(&master->dev->mode_config.idr_mutex); ret = _drm_lease_held_master(master, id); mutex_unlock(&master->dev->mode_config.idr_mutex); -out: - drm_master_put(&master); + return ret; +} + +bool drm_lease_held(struct drm_file *file_priv, int id) +{ + struct drm_master *master; + bool ret; + + master = drm_file_get_master(file_priv); + ret = drm_lease_held_master(master, id); + if (master) + drm_master_put(&master); + return ret; } @@ -150,9 +153,6 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs_in) int count_in, count_out; uint32_t crtcs_out = 0; - if (!file_priv) - return crtcs_in; - master = drm_file_get_master(file_priv); if (!master) return crtcs_in; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index b5566167a798..af9f65bf74d4 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -687,6 +688,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data, struct drm_mode_get_plane *plane_resp = data; struct drm_plane *plane; uint32_t __user *format_ptr; + struct drm_master *master; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; @@ -695,10 +697,13 @@ int drm_mode_getplane(struct drm_device *dev, void *data, if (!plane) return -ENOENT; + master = drm_file_get_master(file_priv); drm_modeset_lock(&plane->mutex, NULL); - if (plane->state && plane->state->crtc && drm_lease_held(file_priv, plane->state->crtc->base.id)) + if (plane->state && plane->state->crtc && + drm_lease_held_master(master, plane->state->crtc->base.id)) plane_resp->crtc_id = plane->state->crtc->base.id; - else if (!plane->state && plane->crtc && drm_lease_held(file_priv, plane->crtc->base.id)) + else if (!plane->state && plane->crtc && + drm_lease_held_master(master, plane->crtc->base.id)) plane_resp->crtc_id = plane->crtc->base.id; else plane_resp->crtc_id = 0; @@ -710,6 +715,8 @@ int drm_mode_getplane(struct drm_device *dev, void *data, else plane_resp->fb_id = 0; drm_modeset_unlock(&plane->mutex); + if (master) + drm_master_put(&master); plane_resp->plane_id = plane->base.id; plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv, @@ -1114,6 +1121,7 @@ static int drm_mode_cursor_common(struct drm_device *dev, return -ENOENT; } + lockdep_assert_held_once(&dev->master_rwsem); drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); retry: ret = drm_modeset_lock(&crtc->mutex, &ctx); @@ -1128,7 +1136,7 @@ static int drm_mode_cursor_common(struct drm_device *dev, if (ret) goto out; - if (!drm_lease_held(file_priv, crtc->cursor->base.id)) { + if (file_priv && !drm_lease_held_master(file_priv->master, crtc->cursor->base.id)) { ret = -EACCES; goto out; } diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h index 5c9ef6a2aeae..426ea86d3c6a 100644 --- a/include/drm/drm_lease.h +++ b/include/drm/drm_lease.h @@ -18,6 +18,8 @@ bool drm_lease_held(struct drm_file *file_priv, int id); bool _drm_lease_held(struct drm_file *file_priv, int id); +bool drm_lease_held_master(struct drm_master *master, int id); + void drm_lease_revoke(struct drm_master *master); uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs); From patchwork Wed Aug 25 10:24:11 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: 12457123 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 4E1B5C432BE for ; Wed, 25 Aug 2021 10:27:10 +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 1987061184 for ; Wed, 25 Aug 2021 10:27:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 1987061184 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 026D86E1BD; Wed, 25 Aug 2021 10:27:09 +0000 (UTC) Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) by gabe.freedesktop.org (Postfix) with ESMTPS id 29E5B6E1BB; Wed, 25 Aug 2021 10:27:07 +0000 (UTC) Received: by mail-pf1-x429.google.com with SMTP id w68so20847506pfd.0; Wed, 25 Aug 2021 03:27:07 -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=ZiZptxAG6LZlwjR95iOzUI6M5xQ+c4I+M1PZ5LikpHE=; b=hBK8jLnz+Qvhn9vJow4RjUvNtdb4EOIVfTN9Df0pqvZGRzhpFOHLBNw4tmkYhbXP0O i3vrPrX8jjED7W2mAwTWfMFP5b7/t2FRtMrR0FJSt/U864PEVKnvw/mdUciFtytnvkm2 1l5QOWUEHAkfaK5MP7d9rEzDG7b4YQiT4Z5Cvh2ZeCvnFXVF1B6kGaeXbj01mPn4oFtn gCHjyg6d0MeG30CGzA/ue2z4P+bVseZblBqnXX57g2D7sc4DuEahkPuGRf9MgePRzusT lo55pmUHaBLL0JHGbN9FHNSYfBZu7+fWjP0aiJ0i3PCkjFF7P/w2CHrn5k/U/+cLQ1fi L3lA== 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=ZiZptxAG6LZlwjR95iOzUI6M5xQ+c4I+M1PZ5LikpHE=; b=YUJxZQKL4JbUWv4ZaqlpnbOX3HUFKOfW1Wz4J7CoGnMA5yTUeCgQOTHf4M1uW9qhNS 8tgLcL6VW3koqzhSeJIdK9kpfF8cpNKC6H6jf/0BGeCMa3AHu+d0h1J9NlOw1oo1eqB0 4EuaNuEM7lXfo1t4p9cJRRagY8CTdMMyHPG/oRt0P5xIpl7gkQfsIWZNl4GDss4MXHbC uEWwEhZmO7uA4AgPsD5lt+1ERtyhWKWIVOYb/wNVS+VueBa4RUoZciVDa567bM9UyiB6 E4tCUx0JgRMPX8bAfMsPvtThQdknhuzHZoIpSSsYBimECYzVISn7epDXEPSBpHRoYUEM zSKg== X-Gm-Message-State: AOAM5305mMX97v4ljSIrFJj7a2ORInVoB/bft5dqEQUASRLqb3B84Vwh ITufEhH2G/pkOveU6WB4f98= X-Google-Smtp-Source: ABdhPJwpGHyw/JNX8vQV+9CxZcjmMtcAfrqRTNZJ2G1s/7vSRd37Xu0JMq88+7TSzkG9QKgoG6Pv9A== X-Received: by 2002:a05:6a00:2491:b0:3ef:5233:8f6 with SMTP id c17-20020a056a00249100b003ef523308f6mr3421378pfv.45.1629887226719; Wed, 25 Aug 2021 03:27:06 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id t42sm10228377pfg.30.2021.08.25.03.26.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 03:27:06 -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, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, chris@chris-wilson.co.uk, ville.syrjala@linux.intel.com, matthew.auld@intel.com, dan.carpenter@oracle.com, tvrtko.ursulin@intel.com, matthew.d.roper@intel.com, lucas.demarchi@intel.com, karthik.b.s@intel.com, jose.souza@intel.com, manasi.d.navare@intel.com, airlied@redhat.com, aditya.swarup@intel.com, andrescj@chromium.org, linux-graphics-maintainer@vmware.com, zackr@vmware.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 v6 7/7] drm: remove drm_file.master_lookup_lock Date: Wed, 25 Aug 2021 18:24:11 +0800 Message-Id: <20210825102411.1084220-8-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210825102411.1084220-1-desmondcheongzx@gmail.com> References: <20210825102411.1084220-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" Previously, master_lookup_lock was introduced in commit 0b0860a3cf5e ("drm: serialize drm_file.master with a new spinlock") to serialize accesses to drm_file.master. This then allowed us to write drm_file_get_master in commit 56f0729a510f ("drm: protect drm_master pointers in drm_lease.c"). The rationale behind introducing a new spinlock at the time was that the other lock that could have been used (drm_device.master_mutex) was the outermost lock, so embedding calls to drm_file_get_master and drm_is_current_master in various functions easily caused us to invert the lock hierarchy. Following the conversion of master_mutex into a rwsem, and its use to plug races with modesetting rights, we've untangled some lock hierarchies and removed the need for using drm_file_get_master and the unlocked version of drm_is_current_master in multiple places. Hence, we can take this opportunity to clean up the locking design by replacing master_lookup_lock with drm_device.master_rwsem. Signed-off-by: Desmond Cheong Zhi Xi --- drivers/gpu/drm/drm_auth.c | 19 +++++++------------ drivers/gpu/drm/drm_file.c | 1 - drivers/gpu/drm/drm_internal.h | 1 + drivers/gpu/drm/drm_ioctl.c | 4 ++-- drivers/gpu/drm/drm_lease.c | 18 ++++++++---------- include/drm/drm_file.h | 9 +-------- 6 files changed, 19 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index f2b2f197052a..232416119407 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -61,10 +61,9 @@ * trusted clients. */ -static bool drm_is_current_master_locked(struct drm_file *fpriv) +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_rwsem)); + lockdep_assert_held_once(&fpriv->minor->dev->master_rwsem); return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } @@ -83,9 +82,9 @@ bool drm_is_current_master(struct drm_file *fpriv) { bool ret; - spin_lock(&fpriv->master_lookup_lock); + down_read(&fpriv->minor->dev->master_rwsem); ret = drm_is_current_master_locked(fpriv); - spin_unlock(&fpriv->master_lookup_lock); + up_read(&fpriv->minor->dev->master_rwsem); return ret; } @@ -120,7 +119,7 @@ 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))) { + if (unlikely(!drm_is_current_master_locked(file_priv))) { up_write(&dev->master_rwsem); return -EACCES; } @@ -178,9 +177,7 @@ 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); fpriv->master = new_master; - spin_unlock(&fpriv->master_lookup_lock); fpriv->is_master = 1; fpriv->authenticated = 1; @@ -343,9 +340,7 @@ 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); file_priv->master = drm_master_get(dev->master); - spin_unlock(&file_priv->master_lookup_lock); } up_write(&dev->master_rwsem); @@ -413,13 +408,13 @@ struct drm_master *drm_file_get_master(struct drm_file *file_priv) if (!file_priv) return NULL; - spin_lock(&file_priv->master_lookup_lock); + down_read(&file_priv->minor->dev->master_rwsem); if (!file_priv->master) goto unlock; master = drm_master_get(file_priv->master); unlock: - spin_unlock(&file_priv->master_lookup_lock); + up_read(&file_priv->minor->dev->master_rwsem); return master; } EXPORT_SYMBOL(drm_file_get_master); diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 90b62f360da1..8c846e0179d7 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/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 17f3548c8ed2..5d421f749a17 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -132,6 +132,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); /* drm_auth.c */ +bool drm_is_current_master_locked(struct drm_file *fpriv); int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv); int drm_authmagic(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 8bea39ffc5c0..c728437466c3 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -386,7 +386,7 @@ 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))) { + if (unlikely(!drm_is_current_master_locked(file_priv))) { retcode = -EACCES; goto unlock; } @@ -540,7 +540,7 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) /* MASTER is only for master or control clients */ if (unlikely((flags & DRM_MASTER) && - !drm_is_current_master(file_priv))) + !drm_is_current_master_locked(file_priv))) return -EACCES; /* Render clients must be explicitly allowed */ diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 15bf3a3c76d1..0eecf320b1ab 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -498,12 +498,12 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, return PTR_ERR(lessee_file); down_read(&dev->master_rwsem); - if (unlikely(!drm_is_current_master(lessor_priv))) { + if (unlikely(!drm_is_current_master_locked(lessor_priv))) { ret = -EACCES; goto out_file; } - lessor = drm_file_get_master(lessor_priv); + lessor = lessor_priv->master; /* Do not allow sub-leases */ if (lessor->lessor) { DRM_DEBUG_LEASE("recursive leasing not allowed\n"); @@ -565,7 +565,6 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, /* Hook up the fd */ 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; @@ -600,7 +599,8 @@ int drm_mode_list_lessees_ioctl(struct drm_device *dev, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; - lessor = drm_file_get_master(lessor_priv); + lockdep_assert_held_once(&dev->master_rwsem); + lessor = lessor_priv->master; DRM_DEBUG_LEASE("List lessees for %d\n", lessor->lessee_id); mutex_lock(&dev->mode_config.idr_mutex); @@ -624,7 +624,6 @@ int drm_mode_list_lessees_ioctl(struct drm_device *dev, arg->count_lessees = count; mutex_unlock(&dev->mode_config.idr_mutex); - drm_master_put(&lessor); return ret; } @@ -650,7 +649,8 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EOPNOTSUPP; - lessee = drm_file_get_master(lessee_priv); + lockdep_assert_held_once(&dev->master_rwsem); + lessee = lessee_priv->master; DRM_DEBUG_LEASE("get lease for %d\n", lessee->lessee_id); mutex_lock(&dev->mode_config.idr_mutex); @@ -678,7 +678,6 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev, arg->count_objects = count; mutex_unlock(&dev->mode_config.idr_mutex); - drm_master_put(&lessee); return ret; } @@ -703,11 +702,11 @@ int drm_mode_revoke_lease_ioctl(struct drm_device *dev, return -EOPNOTSUPP; down_write(&dev->master_rwsem); - if (unlikely(!drm_is_current_master(lessor_priv))) { + if (unlikely(!drm_is_current_master_locked(lessor_priv))) { ret = -EACCES; goto unlock; } - lessor = drm_file_get_master(lessor_priv); + lessor = lessor_priv->master; mutex_lock(&dev->mode_config.idr_mutex); lessee = _drm_find_lessee(lessor, arg->lessee_id); @@ -728,7 +727,6 @@ int drm_mode_revoke_lease_ioctl(struct drm_device *dev, fail: mutex_unlock(&dev->mode_config.idr_mutex); - drm_master_put(&lessor); unlock: up_write(&dev->master_rwsem); diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index d12bb2ba7814..e2d49fe3e32d 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -227,16 +227,12 @@ struct drm_file { * @master: * * Master this node is currently associated with. Protected by struct - * &drm_device.master_rwsem, and serialized by @master_lookup_lock. + * &drm_device.master_rwsem. * * 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_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_rwsem for the duration of the pointer's use, or * use drm_file_get_master() if struct &drm_device.master_rwsem is not @@ -248,9 +244,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;