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

[Lino Sanfilippo]

Hi,

> Gesendet: Freitag, 24. September 2021 um 16:20 Uhr
> Von: "Jason Gunthorpe" <jgg@xxxxxxxx>
> An: "Lino Sanfilippo" <LinoSanfilippo@xxxxxx>
> Cc: "Jarkko Sakkinen" <jarkko@xxxxxxxxxx>, peterhuewe@xxxxxx, p.rosenberger@xxxxxxxxxx, linux-integrity@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, stable@xxxxxxxxxxxxxxx
> Betreff: Re: [PATCH] tpm: fix potential NULL pointer access in tpm_del_char_device()
>
> On Fri, Sep 24, 2021 at 04:17:52PM +0200, Lino Sanfilippo wrote:
> > On 24.09.21 at 15:33, Jason Gunthorpe wrote:
> > > On Fri, Sep 24, 2021 at 03:29:46PM +0200, Lino Sanfilippo wrote:
> > >
> > >> So this bug is triggered when the bcm2835 drivers shutdown() function is called since this
> > >> driver does something quite unusual: it unregisters the spi controller in its shutdown()
> > >> handler.
> > >
> > > This seems wrong
> > >
> > > Jason
> > >
> >
> >
> > Unregistering the SPI controller during shutdown is only a side-effect of calling
> > bcm2835_spi_remove() in the shutdown handler:
> >
> > static void bcm2835_spi_shutdown(struct platform_device *pdev)
> > {
> > 	int ret;
> >
> > 	ret = bcm2835_spi_remove(pdev);
> > 	if (ret)
> > 		dev_err(&pdev->dev, "failed to shutdown\n");
> > }
>
> That's wrong, the shutdown handler is only supposed to make the HW
> stop doing DMA and interrupts so we can have a clean transition to
> kexec/etc
>
> It should not be manipulating other state.


I created another patch that fixes the issue in the BCM2835 driver instead
(see https://marc.info/?l=linux-spi&m=163285906725366&w=2).

However I still think that the fix I proposed for TPM is valueable, because
it saves us from any SPI controller driver that does not know/care about the
issue that is caused in TPM by unregistering the controller in the shutdown
handler. Note that the freescale DSPI driver is another candidate that behaves
errorneous in this way.

Regards,
Lino