From patchwork Wed Mar 23 17:23:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bartlomiej Zolnierkiewicz X-Patchwork-Id: 1608750 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=UxkYQoUP; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4KNwDf1gt7z9s1l for ; Thu, 24 Mar 2022 04:23:29 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1nX4he-0005Ta-8z; Wed, 23 Mar 2022 17:23:22 +0000 Received: from smtp-relay-internal-1.internal ([10.131.114.114] helo=smtp-relay-internal-1.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1nX4hc-0005Ss-KP for kernel-team@lists.ubuntu.com; Wed, 23 Mar 2022 17:23:20 +0000 Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id CE9193F1CA for ; Wed, 23 Mar 2022 17:23:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1648056198; bh=YTeemUlIRuooyA4OXrYUKh4HQ6nRn0pS4jVh2CxFGPw=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=UxkYQoUP7mFv2fqViW236ZDk0ydx7D9WU54ZyfWrYkSRe6izuxyHElEpZ49AEBjbh Wilf3B/nIpwgiz6u7xB7wEMI7wSXheWeMyw8KDmi5vfkuXKAtSpN8jFPUFr4OjgKXE Ip46n5aEDv6trwTAH3xBhsfMrzJnmmXg/MLbJXcf2c9FYDA+N4hU248hbDz4tiOMxA gE9EOOlf8Ie1BmElTJZX6GZuvOfua8yoglx/CJT6RbNBz6QlxsGNlb+OZklstCZxdq 6i46Wsehc2qmACbczG+HyolooHPRt9QLNpawO0XifJXpQJkf7yAMSqQcuaglmN3ddn WV6eecHfG2p9A== Received: by mail-ed1-f70.google.com with SMTP id d28-20020a50f69c000000b00419125c67f4so1434408edn.17 for ; Wed, 23 Mar 2022 10:23:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=YTeemUlIRuooyA4OXrYUKh4HQ6nRn0pS4jVh2CxFGPw=; b=7eCEfjgWRxcAT7WkRB2rEoF1bclgHjzgF1E1VBCGMzzvX410IwsdRzwNVC0RalXw7S w5TFR0GxSnpVizAUyKt6LYA+mM54sLlTGy2xMsAWye93N0dk0jWrIStTyVzu7WyseIno 6UkjbjUStzSf7zAen7L7zBeRcdYFXZCE8iJiOIBcu/oI2rDeml3SlrGMu9jETmSLa7AZ fxdPePhI6bgIc3BWvcm8zc6KQNYrSz6N3gp/ZBef10to6rOIXY5jLVcnNaNVQUL5a1X2 /Ff/HIVsW9nGsaO3BbOAVRFUGcVki3ZaYwxzzo+Oo7aY9/HPzXs4AANtS/dkXaoRIxjB TrRw== X-Gm-Message-State: AOAM531o+jHPI4Zpy0KmkYSqTe0PxZ/pYGWBL35d7/MhgHvU8CsWwpTf y3urwrLlIhe/HD3IeyLQVufjxxOWznkk35oElgVbhA08lJ2L86pHVlPMTnnRWecxriTye6hoj7y xIrNusv+01K6l3R3ExS+qsPXsTGvQGGwgJg2qrbwHww== X-Received: by 2002:a17:907:7fab:b0:6e0:6f26:123e with SMTP id qk43-20020a1709077fab00b006e06f26123emr1195941ejc.29.1648056198278; Wed, 23 Mar 2022 10:23:18 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzpOKUznxBqxse9772tAcXKYKEJd99FNaI9I+FVNct3UABiDT0LJctZxnyGKNCEVXycCPcZsQ== X-Received: by 2002:a17:907:7fab:b0:6e0:6f26:123e with SMTP id qk43-20020a1709077fab00b006e06f26123emr1195925ejc.29.1648056198067; Wed, 23 Mar 2022 10:23:18 -0700 (PDT) Received: from localhost.localdomain (89-65-217-23.dynamic.chello.pl. [89.65.217.23]) by smtp.gmail.com with ESMTPSA id l2-20020aa7cac2000000b003f9b3ac68d6sm257463edt.15.2022.03.23.10.23.17 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Mar 2022 10:23:17 -0700 (PDT) From: Bartlomiej Zolnierkiewicz To: kernel-team@lists.ubuntu.com Subject: [SRU][Focal][PATCH 1/2] drm/nouveau: Add a dedicated mutex for the clients list Date: Wed, 23 Mar 2022 18:23:12 +0100 Message-Id: <20220323172313.32899-2-bartlomiej.zolnierkiewicz@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220323172313.32899-1-bartlomiej.zolnierkiewicz@canonical.com> References: <20220323172313.32899-1-bartlomiej.zolnierkiewicz@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jeremy Cline Rather than protecting the nouveau_drm clients list with the lock within the "client" nouveau_cli, add a dedicated lock to serialize access to the list. This is both clearer and necessary to avoid lockdep being upset with us when we need to iterate through all the clients in the list and potentially lock their mutex, which is the same class as the lock protecting the entire list. Cc: stable@vger.kernel.org # 5.4+ Signed-off-by: Jeremy Cline Reviewed-by: Lyude Paul Reviewed-by: Ben Skeggs Tested-by: Karol Herbst Signed-off-by: Karol Herbst Link: https://patchwork.freedesktop.org/patch/msgid/20201125202648.5220-3-jcline@redhat.com Link: https://gitlab.freedesktop.org/drm/nouveau/-/merge_requests/14 (backported from commit abae9164a421bc4a41a3769f01ebcd1f9d955e0e) [bzolnier: backported due to different context in nouveau_drm_device_fini()] CVE-2020-27820 Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/gpu/drm/nouveau/nouveau_drm.c | 10 ++++++---- drivers/gpu/drm/nouveau/nouveau_drv.h | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index e18938972a89..c9d2be16ee6f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -520,6 +520,7 @@ nouveau_drm_device_init(struct drm_device *dev) nvkm_dbgopt(nouveau_debug, "DRM"); INIT_LIST_HEAD(&drm->clients); + mutex_init(&drm->clients_lock); spin_lock_init(&drm->tile.lock); /* workaround an odd issue on nvc1 by disabling the device's @@ -615,6 +616,7 @@ nouveau_drm_device_fini(struct drm_device *dev) nouveau_cli_fini(&drm->client); nouveau_cli_fini(&drm->master); + mutex_destroy(&drm->clients_lock); kfree(drm); } @@ -1073,9 +1075,9 @@ nouveau_drm_open(struct drm_device *dev, struct drm_file *fpriv) fpriv->driver_priv = cli; - mutex_lock(&drm->client.mutex); + mutex_lock(&drm->clients_lock); list_add(&cli->head, &drm->clients); - mutex_unlock(&drm->client.mutex); + mutex_unlock(&drm->clients_lock); done: if (ret && cli) { @@ -1101,9 +1103,9 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) nouveau_abi16_fini(cli->abi16); mutex_unlock(&cli->mutex); - mutex_lock(&drm->client.mutex); + mutex_lock(&drm->clients_lock); list_del(&cli->head); - mutex_unlock(&drm->client.mutex); + mutex_unlock(&drm->clients_lock); nouveau_cli_fini(cli); kfree(cli); diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h index 8104e3806499..8138ac851d94 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drv.h +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h @@ -138,6 +138,11 @@ struct nouveau_drm { struct list_head clients; + /** + * @clients_lock: Protects access to the @clients list of &struct nouveau_cli. + */ + struct mutex clients_lock; + u8 old_pm_cap; struct { From patchwork Wed Mar 23 17:23:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bartlomiej Zolnierkiewicz X-Patchwork-Id: 1608751 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=SRNjIY/t; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4KNwDf4hpXz9sFh for ; Thu, 24 Mar 2022 04:23:30 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1nX4hg-0005UT-GO; Wed, 23 Mar 2022 17:23:24 +0000 Received: from smtp-relay-internal-1.internal ([10.131.114.114] helo=smtp-relay-internal-1.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1nX4hc-0005TF-SA for kernel-team@lists.ubuntu.com; Wed, 23 Mar 2022 17:23:20 +0000 Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id A5FA23F1CA for ; Wed, 23 Mar 2022 17:23:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1648056200; bh=3Mq5IG5btpKKlmnq5DEvVhEBGyV4zPGZ69aTm0IPg6A=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=SRNjIY/t93SXUFljqR/vHPeiQHSwDkoAAVqgQnLhZbuq4nQ4wQlLAY8oz4k9mngdL yFEaKvV5x6ms7CEEIYK3YBL2f9mDL+2bGQS6aOHBvKSh1oR+WXXGUN68T86P4w4iVU X7SSJ1W0iO71WZ9RAdxjXYluq/2osbnhahbUWu3g0KlnqW9jOD95F54FbHDCN103yn 0Gi+HrsPv35MYcPAlAN+Hc2tuF0NMFqeZLVs03D0NH9GVnbe/keg+j1CXqnTnZVaZ/ EpzfwCe0Zt60flwStXF6Xe8qgeHnFGG7XvyAGoX94RZ26XJpdsKedJQb28eTqAc8g7 xeHvLZYBjKL+Q== Received: by mail-ed1-f72.google.com with SMTP id x1-20020a50f181000000b00418f6d4bccbso1446903edl.12 for ; Wed, 23 Mar 2022 10:23:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=3Mq5IG5btpKKlmnq5DEvVhEBGyV4zPGZ69aTm0IPg6A=; b=XbWsw1cvjGOYRujyK/EPTI9nvZPfDf1wXHPQwgYCjqAxxek7zEe+A1m3RY6weQVuV9 YS92c2YXcIF2e1e9p2l8+/ekraKurq7giUZNPOd0kpFRdKGV8nFenZ+qGSnXTbjjA4Yw j7OSNVMRena10vw35SyrD+bBMaQmnc6KLLsR96XYm94cHZemtTij5JHv0mStlwNJ/THY 4mVDeBA8llSjEkHhGZc8CzIaUKsRwyVbILJrFJhEcC9Wd6HKmp4gR/+xgAg2yjj/O+E2 s2yxDyA5PRD5IdGpnYA8iMa/2R60EpWAhSa1OSRQfaXVLLL8Pe434xJd1nFJEq6oP4MS ZoMA== X-Gm-Message-State: AOAM530TeRZthxok1Deaxm7nHO58qijCpfAiKfahQfrFox0AoZxaQEWm IifVJ4+EyDdv1eWL3FW3CYlqG18K1Q8wJCK7yEfy8h2QnxBQ6RlfDnY8OLaPR8Qe7o5a9GAEep8 h7rKBAnQSmhooNQxEpgv6myEZvniBeVUUYzNFvM3c0w== X-Received: by 2002:a05:6402:27d4:b0:419:5105:f7e7 with SMTP id c20-20020a05640227d400b004195105f7e7mr1505666ede.356.1648056199359; Wed, 23 Mar 2022 10:23:19 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz0yoEHcXiGTZmuLMe1lV+eUOMUBjQFJo2NMzpF8q2v6iZ59z2NWC5lKnndTxHVwof54KwCVg== X-Received: by 2002:a05:6402:27d4:b0:419:5105:f7e7 with SMTP id c20-20020a05640227d400b004195105f7e7mr1505627ede.356.1648056198992; Wed, 23 Mar 2022 10:23:18 -0700 (PDT) Received: from localhost.localdomain (89-65-217-23.dynamic.chello.pl. [89.65.217.23]) by smtp.gmail.com with ESMTPSA id l2-20020aa7cac2000000b003f9b3ac68d6sm257463edt.15.2022.03.23.10.23.18 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Mar 2022 10:23:18 -0700 (PDT) From: Bartlomiej Zolnierkiewicz To: kernel-team@lists.ubuntu.com Subject: [SRU][Focal][PATCH 2/2] drm/nouveau: clean up all clients on device removal Date: Wed, 23 Mar 2022 18:23:13 +0100 Message-Id: <20220323172313.32899-3-bartlomiej.zolnierkiewicz@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220323172313.32899-1-bartlomiej.zolnierkiewicz@canonical.com> References: <20220323172313.32899-1-bartlomiej.zolnierkiewicz@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jeremy Cline The postclose handler can run after the device has been removed (or the driver has been unbound) since userspace clients are free to hold the file open as long as they want. Because the device removal callback frees the entire nouveau_drm structure, any reference to it in the postclose handler will result in a use-after-free. To reproduce this, one must simply open the device file, unbind the driver (or physically remove the device), and then close the device file. This was found and can be reproduced easily with the IGT core_hotunplug tests. To avoid this, all clients are cleaned up in the device finalization rather than deferring it to the postclose handler, and the postclose handler is protected by a critical section which ensures the drm_dev_unplug() and the postclose handler won't race. This is not an ideal fix, since as I understand the proposed plan for the kernel<->userspace interface for hotplug support, destroying the client before the file is closed will cause problems. However, I believe to properly fix this issue, the lifetime of the nouveau_drm structure needs to be extended to match the drm_device, and this proved to be a rather invasive change. Thus, I've broken this out so the fix can be easily backported. This fixes with the two previous commits CVE-2020-27820 (Karol). Cc: stable@vger.kernel.org # 5.4+ Signed-off-by: Jeremy Cline Reviewed-by: Lyude Paul Reviewed-by: Ben Skeggs Tested-by: Karol Herbst Signed-off-by: Karol Herbst Link: https://patchwork.freedesktop.org/patch/msgid/20201125202648.5220-4-jcline@redhat.com Link: https://gitlab.freedesktop.org/drm/nouveau/-/merge_requests/14 (cherry picked from commit f55aaf63bde0d0336c3823bb3713bd4a464abbcf) CVE-2020-27820 Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/gpu/drm/nouveau/nouveau_drm.c | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index c9d2be16ee6f..43a04d9b9925 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -590,6 +590,7 @@ nouveau_drm_device_init(struct drm_device *dev) static void nouveau_drm_device_fini(struct drm_device *dev) { + struct nouveau_cli *cli, *temp_cli; struct nouveau_drm *drm = nouveau_drm(dev); if (nouveau_pmops_runtime()) { @@ -614,6 +615,24 @@ nouveau_drm_device_fini(struct drm_device *dev) nouveau_ttm_fini(drm); nouveau_vga_fini(drm); + /* + * There may be existing clients from as-yet unclosed files. For now, + * clean them up here rather than deferring until the file is closed, + * but this likely not correct if we want to support hot-unplugging + * properly. + */ + mutex_lock(&drm->clients_lock); + list_for_each_entry_safe(cli, temp_cli, &drm->clients, head) { + list_del(&cli->head); + mutex_lock(&cli->mutex); + if (cli->abi16) + nouveau_abi16_fini(cli->abi16); + mutex_unlock(&cli->mutex); + nouveau_cli_fini(cli); + kfree(cli); + } + mutex_unlock(&drm->clients_lock); + nouveau_cli_fini(&drm->client); nouveau_cli_fini(&drm->master); mutex_destroy(&drm->clients_lock); @@ -1095,6 +1114,16 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) { struct nouveau_cli *cli = nouveau_cli(fpriv); struct nouveau_drm *drm = nouveau_drm(dev); + int dev_index; + + /* + * The device is gone, and as it currently stands all clients are + * cleaned up in the removal codepath. In the future this may change + * so that we can support hot-unplugging, but for now we immediately + * return to avoid a double-free situation. + */ + if (!drm_dev_enter(dev, &dev_index)) + return; pm_runtime_get_sync(dev->dev); @@ -1111,6 +1140,7 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) kfree(cli); pm_runtime_mark_last_busy(dev->dev); pm_runtime_put_autosuspend(dev->dev); + drm_dev_exit(dev_index); } static const struct drm_ioctl_desc