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;
Date:   Fri, 24 Sep 2021 11:20:32 -0300
From:   Jason Gunthorpe <jgg@ziepe.ca>
To:     Lino Sanfilippo <LinoSanfilippo@gmx.de>
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: [PATCH] tpm: fix potential NULL pointer access in
 tpm_del_char_device()
Message-ID: <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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b49f4b52-44c4-8cb8-a102-689e9f788177@gmx.de>
Precedence: bulk

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.

Jason