linux-kernel - Re: Re: [PATCH] tpm: fix potential NULL pointer access in tpm_del_char

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 29 Sep 2021 10:58:25 +0200
From:   Lino Sanfilippo <LinoSanfilippo@....de>
To:     Jason Gunthorpe <jgg@...pe.ca>
Cc:     Jarkko Sakkinen <jarkko@...nel.org>, peterhuewe@....de,
        p.rosenberger@...bus.com, linux-integrity@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: Re: [PATCH] tpm: fix potential NULL pointer access in
 tpm_del_char_device()

Hi,

> Gesendet: Freitag, 24. September 2021 um 16:20 Uhr
> Von: "Jason Gunthorpe" <jgg@...pe.ca>
> An: "Lino Sanfilippo" <LinoSanfilippo@....de>
> Cc: "Jarkko Sakkinen" <jarkko@...nel.org>, peterhuewe@....de, p.rosenberger@...bus.com, linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org
> 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