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