spi: fix explicit controller deregistration
Subsystems (2)
cover spi: fix explicit controller deregistration
Turns out we have a few drivers that get the tear down ordering wrong also when not using device managed registration (cf. [1] and [2]).
Fix this to avoid issues like system errors due to unclocked accesses, NULL-pointer dereferences, hangs or failed I/O during during deregistration (e.g. when powering down devices).
Johan
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
Johan Hovold (8):
spi: cadence: fix controller deregistration
spi: cadence-quadspi: fix controller deregistration
spi: mpc52xx: fix controller deregistration
spi: mpc52xx: fix use-after-free on unbind
spi: mxic: fix controller deregistration
spi: orion: fix controller deregistration
spi: topcliff-pch: fix controller deregistration
spi: topcliff-pch: fix use-after-free on unbind
drivers/spi/spi-cadence-quadspi.c | 4 ++-- drivers/spi/spi-cadence.c | 6 +++++- drivers/spi/spi-mpc52xx.c | 6 ++++-- drivers/spi/spi-mxic.c | 3 ++- drivers/spi/spi-orion.c | 7 ++++++- drivers/spi/spi-topcliff-pch.c | 11 ++++++++--- 6 files changed, 27 insertions(+), 10 deletions(-)
Signature · 1 line
2.52.0
PATCH 1/8 spi: cadence: fix controller deregistration
Make sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-cadence.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/spi/spi-cadence.c +5 -1
@@ -777,6 +777,10 @@ static void cdns_spi_remove(struct platform_device *pdev)struct spi_controller *ctlr =platform_get_drvdata (pdev);struct cdns_spi *xspi =spi_controller_get_devdata (ctlr); +spi_controller_get (ctlr); + +spi_unregister_controller (ctlr); +cdns_spi_write (xspi,CDNS_SPI_ER ,CDNS_SPI_ER_DISABLE );if (!spi_controller_is_target (ctlr)) {
@@ -784,7 +788,7 @@ static void cdns_spi_remove(struct platform_device *pdev)pm_runtime_set_suspended (&pdev->dev); } -spi_unregister_controller (ctlr); +spi_controller_put (ctlr); }/**
Signature · 1 line
2.52.0
PATCH 2/8 spi: cadence-quadspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before dropping the reference count that allows new operations to start to allow SPI drivers to do I/O during deregistration.
--- drivers/spi/spi-cadence-quadspi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/spi/spi-cadence-quadspi.c +2 -2
@@ -2020,13 +2020,13 @@ static void cqspi_remove(struct platform_device *pdev) ddata =of_device_get_match_data (dev); +spi_unregister_controller (cqspi->host); +refcount_set (&cqspi->refcount,0 );if (!refcount_dec_and_test (&cqspi->inflight_ops))cqspi_wait_idle (cqspi); -spi_unregister_controller (cqspi->host); -if (cqspi->rx_chan)dma_release_channel (cqspi->rx_chan);
Signature · 1 line
2.52.0
PATCH 3/8 spi: mpc52xx: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling and releasing underlying resources like interrupts and gpios during driver unbind.
--- drivers/spi/spi-mpc52xx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/spi/spi-mpc52xx.c +2 -1
@@ -517,6 +517,8 @@ static void mpc52xx_spi_remove(struct platform_device *op)struct mpc52xx_spi *ms =spi_controller_get_devdata (host);int i; +spi_unregister_controller (host); +cancel_work_sync (&ms->work);free_irq (ms->irq0, ms);free_irq (ms->irq1, ms);
@@ -525,7 +527,6 @@ static void mpc52xx_spi_remove(struct platform_device *op)gpiod_put (ms->gpio_cs[i]);kfree (ms->gpio_cs); -spi_unregister_controller (host);iounmap (ms->regs);spi_controller_put (host); }
Signature · 1 line
2.52.0
PATCH 4/8 spi: mpc52xx: fix use-after-free on unbind
In reply to Johan HovoldThe state machine work is scheduled by the interrupt handler and therefore needs to be cancelled after disabling interrupts to avoid a potential use-after-free.
--- drivers/spi/spi-mpc52xx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/spi/spi-mpc52xx.c +2 -1
@@ -519,10 +519,11 @@ static void mpc52xx_spi_remove(struct platform_device *op)spi_unregister_controller (host); -cancel_work_sync (&ms->work);free_irq (ms->irq0, ms);free_irq (ms->irq1, ms); +cancel_work_sync (&ms->work); +for (i =0 ; i < ms->gpio_cs_count; i++)gpiod_put (ms->gpio_cs[i]);
Signature · 1 line
2.52.0
PATCH 5/8 spi: mxic: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks (via runtime pm) during driver unbind.
--- drivers/spi/spi-mxic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/spi/spi-mxic.c +2 -1
@@ -832,9 +832,10 @@ static void mxic_spi_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct mxic_spi *mxic =spi_controller_get_devdata (host); +spi_unregister_controller (host); +pm_runtime_disable (&pdev->dev);mxic_spi_mem_ecc_remove (mxic); -spi_unregister_controller (host); }static const struct of_device_id mxic_spi_of_ids[] = {
Signature · 1 line
2.52.0
PATCH 6/8 spi: orion: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-orion.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/spi/spi-orion.c +6 -1
@@ -801,10 +801,15 @@ static void orion_spi_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct orion_spi *spi =spi_controller_get_devdata (host); +spi_controller_get (host); + +spi_unregister_controller (host); +pm_runtime_get_sync (&pdev->dev);clk_disable_unprepare (spi->axi_clk); -spi_unregister_controller (host); +spi_controller_put (host); +pm_runtime_disable (&pdev->dev); }
Signature · 1 line
2.52.0
PATCH 7/8 spi: topcliff-pch: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling and releasing underlying resources like interrupts and DMA during driver unbind.
--- drivers/spi/spi-topcliff-pch.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/spi/spi-topcliff-pch.c +6 -1
@@ -1406,6 +1406,10 @@ static void pch_spi_pd_remove(struct platform_device *plat_dev)dev_dbg (&plat_dev->dev,"%s:[ch%d] irq=%d\n" , __func__, plat_dev->id, board_dat->pdev->irq); +spi_controller_get (data->host); + +spi_unregister_controller (data->host); +if (use_dma)pch_free_dma_buf (board_dat, data);
@@ -1433,7 +1437,8 @@ static void pch_spi_pd_remove(struct platform_device *plat_dev) }pci_iounmap (board_dat->pdev, data->io_remap_addr); -spi_unregister_controller (data->host); + +spi_controller_put (data->host); }#ifdef CONFIG_PM static int pch_spi_pd_suspend(struct platform_device *pd_dev,
Signature · 1 line
2.52.0
PATCH 8/8 spi: topcliff-pch: fix use-after-free on unbind
In reply to Johan HovoldGive the driver a chance to flush its queue before releasing the DMA buffers on driver unbind
--- drivers/spi/spi-topcliff-pch.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/spi/spi-topcliff-pch.c +3 -3
@@ -1410,9 +1410,6 @@ static void pch_spi_pd_remove(struct platform_device *plat_dev)spi_unregister_controller (data->host); -if (use_dma) -pch_free_dma_buf (board_dat, data); -/* check for any pending messages; no action is taken if the queue * is still full; but at least we tried. Unload anyway */ count =500 ;
@@ -1436,6 +1433,9 @@ static void pch_spi_pd_remove(struct platform_device *plat_dev)free_irq (board_dat->pdev->irq, data); } +if (use_dma) +pch_free_dma_buf (board_dat, data); +pci_iounmap (board_dat->pdev, data->io_remap_addr);spi_controller_put (data->host);
Signature · 1 line
2.52.0
Mark Brown
maintainerQuoted 10 lines · Johan Hovold
spi: fix explicit controller deregistration
Turns out we have a few drivers that get the tear down ordering wrong also when not using device managed registration (cf. [1] and [2]).
Fix this to avoid issues like system errors due to unclocked accesses, NULL-pointer dereferences, hangs or failed I/O during during deregistration (e.g. when powering down devices).
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.1
Thanks!
[1/8] spi: cadence: fix controller deregistration
https://git.kernel.org/broonie/spi/c/666fa7e9ca98 [2/8] spi: cadence-quadspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/964ee9793760 [3/8] spi: mpc52xx: fix controller deregistration https://git.kernel.org/broonie/spi/c/0f997fdae819 [4/8] spi: mpc52xx: fix use-after-free on unbind https://git.kernel.org/broonie/spi/c/706b3dc2ac7a [5/8] spi: mxic: fix controller deregistration https://git.kernel.org/broonie/spi/c/adbc595e2720 [6/8] spi: orion: fix controller deregistration https://git.kernel.org/broonie/spi/c/220f4f11104a [7/8] spi: topcliff-pch: fix controller deregistration https://git.kernel.org/broonie/spi/c/5d6f477d6fc0 [8/8] spi: topcliff-pch: fix use-after-free on unbind https://git.kernel.org/broonie/spi/c/9d72732fe70c
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks,
Mark