Re: [PATCH v2] tpm: fix potential NULL pointer access in tpm_del_char_device

On Wed, Dec 29, 2021 at 02:41:52AM +0100, Lino Sanfilippo wrote:

Hi,

On 29.12.21 at 01:13, Jarkko Sakkinen wrote:

quoted

On Mon, Dec 20, 2021 at 04:06:35PM +0100, Lino Sanfilippo wrote:

quoted

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 <LinoSanfilippo@gmx.de>

Thank you.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR,
Jarkko

Thanks a lot for the review. Please note that the latest version is v3 which
contains one more Fixes tag and also a tag for the review by Stefan. I also
adjusted the source code comment in that version which I somehow messed up in v2.

Since there is no functional difference, I rather do not swap it.

I fixed a glitch:

+
+       /*

A multi-line commit needs to have this as the very first line.
You can check if the all tags were pulled by b4.

/Jarkko