Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Message-ID: <trinity-04f4aedd-514d-47bc-8622-cf6b1a264d52-1632905905528@3c-app-gmx-bs21>
From:   Lino Sanfilippo <LinoSanfilippo@gmx.de>
To:     Jason Gunthorpe <jgg@ziepe.ca>
Cc:     Jarkko Sakkinen <jarkko@kernel.org>, peterhuewe@gmx.de,
        p.rosenberger@kunbus.com, linux-integrity@vger.kernel.org,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: Re: [PATCH] tpm: fix potential NULL pointer access in
 tpm_del_char_device()
Content-Type: text/plain; charset=UTF-8
Date:   Wed, 29 Sep 2021 10:58:25 +0200
Importance: normal
Sensitivity: Normal
In-Reply-To: <20210924142032.GY3544071@ziepe.ca>
References: <20210910180451.19314-1-LinoSanfilippo@gmx.de>
 <204a438b6db54060d03689389d6663b0d4ca815d.camel@kernel.org>
 <trinity-27f56ffd-504a-4c34-9cda-0953ccc459a3-1631566430623@3c-app-gmx-bs69>
 <c22d2878f9816000c33f5349e7256cadae22b400.camel@kernel.org>
 <50bd6224-0f01-ca50-af0e-f79b933e7998@gmx.de>
 <20210924133321.GX3544071@ziepe.ca>
 <b49f4b52-44c4-8cb8-a102-689e9f788177@gmx.de>
 <20210924142032.GY3544071@ziepe.ca>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

> Gesendet: Freitag, 24. September 2021 um 16:20 Uhr
> Von: "Jason Gunthorpe" <jgg@ziepe.ca>
> An: "Lino Sanfilippo" <LinoSanfilippo@gmx.de>
> Cc: "Jarkko Sakkinen" <jarkko@kernel.org>, peterhuewe@gmx.de, p.rosenber=
ger@kunbus.com, linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.=
org, stable@vger.kernel.org
> Betreff: Re: [PATCH] tpm: fix potential NULL pointer access in tpm_del_c=
har_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() functi=
on is called since this
> > >> driver does something quite unusual: it unregisters the spi control=
ler 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 =3D 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=3Dlinux-spi&m=3D163285906725366&w=3D2).

However I still think that the fix I proposed for TPM is valueable, becaus=
e
it saves us from any SPI controller driver that does not know/care about t=
he
issue that is caused in TPM by unregistering the controller in the shutdow=
n
handler. Note that the freescale DSPI driver is another candidate that beh=
aves
errorneous in this way.

Regards,
Lino