From patchwork Thu Dec 23 04:02:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lino Sanfilippo X-Patchwork-Id: 12697908 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8699DC433F5 for ; Thu, 23 Dec 2021 04:03:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242078AbhLWEDa (ORCPT ); Wed, 22 Dec 2021 23:03:30 -0500 Received: from mout.gmx.net ([212.227.17.21]:34501 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238699AbhLWED3 (ORCPT ); Wed, 22 Dec 2021 23:03:29 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1640232199; bh=4ILo/VJm8wgroPorTTpv5tAKucJx98LcPaR7SD+bkw8=; h=X-UI-Sender-Class:From:To:Cc:Subject:Date; b=hrf0cYhm4HEHji31aXKDeXveSfpishJEHt0cTeypZ1L0ZVeK+rKflKdvzgutBCaca winCaQv6CFuGUAhzE8wQHgGdrcnb8CmBV9ofdlbZbWI+feBq/sUtSLE+rdE21J7xjq MQfMiAUEMX01uIrjc9cCkAVE0MPs9lNLgKbE7rWQ= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from Venus.fritz.box ([46.223.119.124]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1N1fii-1mKDZ039ei-011zLn; Thu, 23 Dec 2021 05:03:18 +0100 From: Lino Sanfilippo To: peterhuewe@gmx.de, jarkko@kernel.org, jgg@ziepe.ca Cc: p.rosenberger@kunbus.com, stefanb@linux.ibm.com, linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org, Lino Sanfilippo , stable@vger.kernel.org Subject: [PATCH v3] tpm: fix potential NULL pointer access in tpm_del_char_device Date: Thu, 23 Dec 2021 05:02:46 +0100 Message-Id: <20211223040246.6575-1-LinoSanfilippo@gmx.de> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Provags-ID: V03:K1:2kDWMu0wWMr7HLmwOpt7VP3fzse20/ciSnJzNIltI1yar+OtMxB 6XrkoGlLhABtmA+RWtFcAwJr8tXhG8It642u3/HFEOHbK7LyNQCl7Wn4PdwYgcj3SujBfVk FKDKHRTllKA6kUlZcbisqn6PgsT8WBdDgxtlbGiu9QhnxKvk2zk6WjG0ZYq8FFkwVOqXyeJ QJMSvilLhUxGxh5Pedh4Q== X-UI-Out-Filterresults: notjunk:1;V03:K0:g85bodK0EJ8=:RR6KOWkI1oIuahrmq37tih 3qeasUJCSto5uPGPyYK+OZAP4MoMOupg8Sh4OTfDNqYpkXg0TRLiodBGjNQN6YMVmBHVujrQH F+AMp0X/MPE58fIBII5SEZeysswyI73FxAMTtASQTJ8vhcWeBZFVCDrdbmUPL5d1FHdEupelh x5L9QbW/OSv76T59XTMv9/DJ27hxLbTsfkR8utrpTO24eGwSZHY+oFJ/xg9ROVKo0QhAX3GmX 4mT7zRNEAtC/16e1njXD9D7CRgwWTEScCtCN0l7Lks0/zeq1mHVjz9M8xc0qoScAc9K0YgPtH SrB214JkLSJl0eCU1qrJUL9CsYCyDZ1ex3BZRzK559PuueOkFrYJy2gbz8TD2UxHgzX1iO51n /ydFMq8xW54sDYuFS5a30bwDzPwSJS8KlVNm1oN/KT0D98a7dzNb1OZ8gpKcfO8Hw3Rdo1rc/ eUIgsBdNr9dQv3OS7VwXHaDmz12Mo88rA2A67Yq2fe4+c3aKWG4rX/tEZJu3c5Sk2yLfjU9F8 bg1K0iLiY/GyKt+Ek6NjfBDV4H1sW5Ct9ZRyWYpPPtnXRa3Go9qsu4wdlAm8ZFom/BkpFX2dy EYdm2NoUwIh7Z3pRDE025q+xIrN/JCSVTI7lDVld+ReAS1/0zBXnxrIoD4m59YkFNG4VuKAzN QT6yE4I2eVNuIBCoJuW9frxc3lyi/NcyT679G+qd9pLAeNeN2KVildRqIXSyGCjAJsCcX7aSx nDXroty/i816krTerEwJNERjZALaxyr4fiS6Mxz2T3Wf7D1XSHbxbaWnyYX8lnYz762sRpsmS uBdPIynVcw5IO51lN+Taowhg7vyogFa/5FwPG8iuUrJWHoEiWyeEDPsRS5/56fKSrrqyS5GAd riYnEWr0i3anposHhWlivB7A2pX3tjopQVQ6H6eBguf/o+p06MmIc9MdkZQT+xyf2o84dIJxh I5TY/6v+hBGeIpUhqr1myCcF216EpaGkhi+8RVbJ9rs+ziTsu+PKSoebnBAhN6LEn2Nd/vd8a Jfl5w0QIUhJLZQBlsgilBokt0SmfxlZb/dBsqPM2HWympJ1ryOpgwIMFtBvBrOBXyNvM0nlN/ FGjemhUQC6yMEc= Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org Some SPI controller drivers unregister the controller in the shutdown handler (e.g. BCM2835). If such a controller is used with a TPM 2 slave chip->ops may be accessed when it is already NULL: At system shutdown the pre-shutdown handler tpm_class_shutdown() shuts down TPM 2 and sets chip->ops to NULL. Then at SPI controller unregistration tpm_tis_spi_remove() is called and eventually calls tpm_del_char_device() which tries to shut down TPM 2 again. Thereby it accesses chip->ops again: (tpm_del_char_device calls tpm_chip_start which calls tpm_clk_enable which calls chip->ops->clk_enable). Avoid the NULL pointer access by testing if chip->ops is valid and skipping the TPM 2 shutdown procedure in case it is NULL. Fixes: dcbeab1946454 ("tpm: fix crash in tpm_tis deinitialization") Fixes: 39d0099f9439 ("powerpc/pseries: Add shutdown() to vio_driver and vio_bus") Cc: stable@vger.kernel.org Tested-by: Stefan Berger Reviewed-by: Stefan Berger Signed-off-by: Lino Sanfilippo --- Changes in v3: - added tags for Stefans review and test - corrected the source code comment Changes in v2: - rephrased the commit message to clarify the circumstances under which this bug triggers (as requested by Jarkko) I was able to reproduce this issue with a SLB 9670 TPM chip controlled by a BCM2835 SPI controller. The approach to fix this issue in the BCM2835 driver was rejected after a discussion on the mailing list: https://marc.info/?l=linux-integrity&m=163285906725367&w=2 The reason for the rejection was the realization, that this issue should rather be fixed in the TPM code: https://marc.info/?l=linux-spi&m=163311087423271&w=2 So this is the reworked version of a patch that is supposed to do that. drivers/char/tpm/tpm-chip.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) base-commit: bc491fb12513e79702c6f936c838f792b5389129 diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index ddaeceb7e109..03122d624670 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -474,13 +474,19 @@ static void tpm_del_char_device(struct tpm_chip *chip) /* Make the driver uncallable. */ down_write(&chip->ops_sem); - if (chip->flags & TPM_CHIP_FLAG_TPM2) { - if (!tpm_chip_start(chip)) { - tpm2_shutdown(chip, TPM2_SU_CLEAR); - tpm_chip_stop(chip); + /* In case that the SPI master is unregistered in its drivers + * shutdown handler, tpm_class_shutdown() has already been called + * and set chip->ops to NULL. So check if it is still valid. + */ + if (chip->ops) { + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + if (!tpm_chip_start(chip)) { + tpm2_shutdown(chip, TPM2_SU_CLEAR); + tpm_chip_stop(chip); + } } + chip->ops = NULL; } - chip->ops = NULL; up_write(&chip->ops_sem); }