From mboxrd@z Thu Jan 1 00:00:00 1970 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=-13.5 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_INVALID,DKIM_SIGNED,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 26E8CC43214 for ; Mon, 23 Aug 2021 17:15:42 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (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 CFD6461206 for ; Mon, 23 Aug 2021 17:15:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CFD6461206 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id B0BB94016C; Mon, 23 Aug 2021 17:15:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Y11kAAW1weMx; Mon, 23 Aug 2021 17:15:37 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id 99F79402BB; Mon, 23 Aug 2021 17:15:37 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 777AFC001A; Mon, 23 Aug 2021 17:15:37 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 03ED9C000E for ; Mon, 23 Aug 2021 17:15:37 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id D6C10808AE for ; Mon, 23 Aug 2021 17:15:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp1.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cx0j73CpvoFb for ; Mon, 23 Aug 2021 17:15:36 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.8.0 Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by smtp1.osuosl.org (Postfix) with ESMTPS id 1F3388001C for ; Mon, 23 Aug 2021 17:15:36 +0000 (UTC) Received: by mail-pj1-x102e.google.com with SMTP id u11-20020a17090adb4b00b00181668a56d6so421428pjx.5 for ; Mon, 23 Aug 2021 10:15:36 -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=e7uGGTmhO+HW1Nizy73RZhNXpoJcMOyUBRatHDEtcSEbl4wH5flpNGeotwgb4sFZ1d uLh0hZ5tWOj48C1zoLlCLjUBSGCxC0fS7N91OOvc4fa1fkOEqECuXy5ZAnOWmIq6Yqj9 TwxQsOiN9kxnU0IEKnl1qG9/dvDdoGonR3D8o4jyPv6SG66Bfe3OviRM3A70aDuwtiqp KEO5ddg5hkcLV0iDjLO7Y+hAU2UhgzsVSTI8WMd1Vka5+HuS61n7TojMfImVSpwkVJtP k8KXyWyaVcLlsT+B7mNKDUfoyOxOadvUD1DDrvpMrMMLvQfbk/XB+AOU+BB0C5G8FAEs ZerQ== 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=jQtKu9zp1fl58oHLbuFmY5JaUgP6zjSgPPYQeTchB24T3MVT2wCGfTgWY6nt7LzYP/ 1B5C1D+zF8zVBfq96CFm19JelJpcaKbP2KojoSv2/btQq2IWY5rAlV0AimS55E8F38hy wkaHP4G6byfc7gKqdeobsOcA27w69Q8lpRuG6+0TxlOt8eEo1YaAuCbgeGq8HdaBgab7 KRh0+pRrASXnhULeCBHzxUjzLh+39jQ5nbHzAoNF2lUEOcziD1CsUmCv7a9W3AqadxSU 0U1mulqRekp1pZBg5g7YrFvW6ZvG5H20wvnDn7bDcbwF/ikMbW9YB8e8/vqpV9xWR829 7jsQ== X-Gm-Message-State: AOAM532KgsYM8eK80eWETuKUWxgu/oRnYysASKPki6XSzn7ROH6rK//e OC8rWVt1EU8yce37w9KJwQw= X-Google-Smtp-Source: ABdhPJx+F+vy3/OxAlCs6LGIrtGmdduva4wi4tRr9T6hBpSbW9CIXvc7HHH7d9jGUoAUUdQgrWfViQ== X-Received: by 2002:a17:903:31cd:b0:134:5b6f:2ff8 with SMTP id v13-20020a17090331cd00b001345b6f2ff8mr5521620ple.46.1629738935621; Mon, 23 Aug 2021 10:15:35 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id q4sm14290830pjd.52.2021.08.23.10.15.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Aug 2021 10:15:35 -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 Subject: [PATCH v5 4/6] drm: avoid races with modesetting rights Date: Tue, 24 Aug 2021 01:14:35 +0800 Message-Id: <20210823171437.829404-5-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210823171437.829404-1-desmondcheongzx@gmail.com> References: <20210823171437.829404-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, Desmond Cheong Zhi Xi , linux-kernel-mentees@lists.linuxfoundation.org, linux-media@vger.kernel.org X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" 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; -- 2.25.1 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees