From patchwork Mon Dec 20 15:06:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lino Sanfilippo X-Patchwork-Id: 12688059 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 3F68BC433F5 for ; Mon, 20 Dec 2021 15:08:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237972AbhLTPI4 (ORCPT ); Mon, 20 Dec 2021 10:08:56 -0500 Received: from mout.gmx.net ([212.227.17.22]:49909 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239460AbhLTPHH (ORCPT ); Mon, 20 Dec 2021 10:07:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1640012819; bh=ExqcfdSeorMvGLnSOWloYS8Zi49Z7vZRjhyfXFGBUvQ=; h=X-UI-Sender-Class:From:To:Cc:Subject:Date; b=ki6ZglJkBXGNDhH1DL2MepnpTq1ZldsmvUhJvaWo4tqABiQhBCYT7V96Xy1gwhLnE lS2fIdP95sff+ES3r/Nn13QVihXe7AdOr/S0uM2mMZls2ql5Weok6Jgq3Lf58j9ki2 xk1WZmxMs2isE9foxsMP0CwSkg4VIxLdgPDTs2vo= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from Venus.fritz.box ([46.223.119.124]) by mail.gmx.net (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MY68T-1myZiw1Xzx-00YRJE; Mon, 20 Dec 2021 16:06:59 +0100 From: Lino Sanfilippo To: peterhuewe@gmx.de, jarkko@kernel.org, jgg@ziepe.ca Cc: p.rosenberger@kunbus.com, linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org, Lino Sanfilippo , stable@vger.kernel.org Subject: [PATCH v2] tpm: fix potential NULL pointer access in tpm_del_char_device Date: Mon, 20 Dec 2021 16:06:35 +0100 Message-Id: <20211220150635.8545-1-LinoSanfilippo@gmx.de> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Provags-ID: V03:K1:VSpIHCjSucsOxWy7hG0cnMrBZMjWeQPXOCuFT7kbNWhNYgw+i0X +CaUAC6eG6MOYpZO2RTbi6VG04Qs0myIADbBdMpcyIVMp7DyhCg/lkKgWv/Y6kdCbNcJRNy 6GXdiutVvQapWqR47twaTVx6tudTQfhH6+3FQ7rLmrNlkHwF6FxyRCz9HtGdJX8OiMQ+d1/ 1FHXhg8EOqzfZiJR3TCng== X-UI-Out-Filterresults: notjunk:1;V03:K0:TctFUmfyLJY=:eLT2w9B4HdIdxDpih17snI Kw/327TQhPfCfUDPlYBo/sNKjCXvpBrTeU7Hf/qFy7zqBloKfsIOmdciwkjbODARVvXwoFNd8 T64ICHmyPYJuBWwohgZd0BEZt/gnVQxbESWZtTYzYD2rsaWKtUtZFYDBWVMFyGzllbhOTkeYE JXZq0JB7OU2BnQT5BZPHhnXX/8LbPesny6KqSFXheld463Wc5Us8HRDjlvsfwqF9D8iU8f/Am y8OvKdEGW6xqgR2pJJ2lwnYZvCxxCqLiaVm6Q04k1Z4gUFwqY2sPoSj67Z0q8i1IKHUumuZhw 2bJ5sdbr3S67WGxblzo7KMen85AuH1tCuSFfSS/lxLWPSBkqYla1vpSBnux82ifU94mSuEbsD RWc1AdkI0QO523OcyMTFiEkoHIqEpJpox4mL5okmPDRi8isKp2PPYG01ZyWXUeqEcjefI0Tob 7r8h3vZ96Q7e0a8B/fAPU8MWwtnF2+ZcBo/NY7mLoobioizud9QigSwhboRdt4B4nDyqJoOLD rE3YwNVNrUGTEI9jH1ylJllYplhhSHX7lH89XhoVCyer+ArebEUSgm0YIS6B0OYwc9WvLu0Pf hOTd/0UgapWrniar/FRbxtsJ66p+DgATq/peUMo6JB4pfnjust4IF73N0KSWJBf8XNxBXZZuf aoBFtmFSCnxQqi81pYtJrIRn6BrUe8Xl1NU+t7ls21BkLXS1d7XJpkkjF6wsjnuPxT4i22V08 UmQVywGtvvA6xbPdNTmeSHmpzB6uBINzHUoQfp0XontgFpZItmgy1My4pwoWqwB9iIKiByWa3 XWfvvrdbajupHAQv5fJir2VuR5uEtyTqH1hprpTPE1rjA1vmQQI3AhxN4GJPtNdrIVC6xE7v6 NxckYr3pRAFNJw9wi2WNI6hu3sCvmKrjRVsSgd+m2GkhMvS5Eh9XHy4rsHzcCDw1exAPF18i8 xDoGeASVeOGWN0d3lsAMuu9RBhnIF7NmMS86Q8aL/m9Zm/YCsNkGRxeNRLhNIO7qP3blQ6Rjb 2Mo105fdkYslT6nUdhU36CPg5zZvXPEFK+5q7muIABnwoxPbiHhNI6fz0hy+IR2i1euZT/iNT fbZkdEdrCceDTo= 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") Cc: stable@vger.kernel.org Signed-off-by: Lino Sanfilippo Reviewed-by: Stefan Berger Tested-by: Stefan Berger Reviewed-by: Jarkko Sakkinen --- Changes to 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: a7904a538933c525096ca2ccde1e60d0ee62c08e diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index ddaeceb7e109..7960da490e72 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); + /* Check if chip->ops is still valid: In case that the controller + * drivers shutdown handler unregisters the controller in its + * shutdown handler we are called twice and chip->ops to NULL. + */ + 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); }