Aw: Re: [PATCH] spi: bcm2835: do not unregister controller in shutdown handler

Lino Sanfilippo
2021-09-29T08:38:52Z

Hi,

> Gesendet: Dienstag, 28. September 2021 um 22:08 Uhr
> Von: "Mark Brown" <broonie@kernel.org>
> An: "Lino Sanfilippo" <LinoSanfilippo@gmx.de>
> Cc: f.fainelli@gmail.com, rjui@broadcom.com, sbranden@broadcom.com, bcm-kernel-feedback-list@broadcom.com, nsaenz@kernel.org, linux-spi@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, jgg@ziepe.ca, p.rosenberger@kunbus.com, linux-integrity@vger.kernel.org, stable@vger.kernel.org
> Betreff: Re: [PATCH] spi: bcm2835: do not unregister controller in shutdown handler
>
> On Tue, Sep 28, 2021 at 09:56:57PM +0200, Lino Sanfilippo wrote:
> > Do not unregister the SPI controller in the shutdown handler. The reason
> > to avoid this is that controller unregistration results in the slave
> > devices remove() handler being called which may be unexpected for slave
> > drivers at system shutdown.
> >
> > One example is if the BCM2835 driver is used together with the TPM SPI
> > driver:
> > At system shutdown first the TPM chip devices (pre) shutdown handler
> > (tpm_class_shutdown) is called, stopping the chip and setting an operations
> > pointer to NULL.
> > Then since the BCM2835 shutdown handler unregisters the SPI controller the
> > TPM SPI remove function (tpm_tis_spi_remove) is also called. In case of
> > TPM 2 this function accesses the now nullified operations pointer,
> > resulting in the following NULL pointer access:
> >
> > [ 174.078277] 8<--- cut here ---
> > [ 174.078288] Unable to handle kernel NULL pointer dereference at virtual address 00000034
> > [ 174.078293] pgd = 557a5fc9
> > [ 174.078300] [00000034] *pgd=031cf003, *pmd=00000000
> > [ 174.078317] Internal error: Oops: 206 [#1] SMP ARM
> > [ 174.078323] Modules linked in: tpm_tis_spi tpm_tis_core tpm spidev gpio_pca953x mcp320x rtc_pcf2127 industrialio regmap_i2c regmap_spi 8021q garp stp llc ftdi_sio6
>
> Please think hard before including complete backtraces in upstream
> reports, they are very large and contain almost no useful information
> relative to their size so often obscure the relevant content in your
> message. If part of the backtrace is usefully illustrative (it often is
> for search engines if nothing else) then it's usually better to pull out
> the relevant sections.
>

Thank you for the feedback, I will omit the stack trace in the next version.

Regards,
Lino