mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF)
@ 2026-03-19 18:38 Marc Kleine-Budde
  2026-03-24 15:05 ` Frank Li
  2026-03-26 12:27 ` Mark Brown
  0 siblings, 2 replies; 5+ messages in thread
From: Marc Kleine-Budde @ 2026-03-19 18:38 UTC (permalink / raw)
  To: Frank Li, Mark Brown, Gao Pan, Fugang Duan
  Cc: kernel, linux-spi, imx, linux-kernel, Marc Kleine-Budde

There is a teardown order issue in the driver. The SPI controller is
registered using devm_spi_register_controller(), which delays
unregistration of the SPI controller until after the fsl_lpspi_remove()
function returns.

As the fsl_lpspi_remove() function synchronously tears down the DMA
channels, a running SPI transfer triggers the following NULL pointer
dereference due to use after free:

| fsl_lpspi 42550000.spi: I/O Error in DMA RX
| Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[...]
| Call trace:
|  fsl_lpspi_dma_transfer+0x260/0x340 [spi_fsl_lpspi]
|  fsl_lpspi_transfer_one+0x198/0x448 [spi_fsl_lpspi]
|  spi_transfer_one_message+0x49c/0x7c8
|  __spi_pump_transfer_message+0x120/0x420
|  __spi_sync+0x2c4/0x520
|  spi_sync+0x34/0x60
|  spidev_message+0x20c/0x378 [spidev]
|  spidev_ioctl+0x398/0x750 [spidev]
[...]

Switch from devm_spi_register_controller() to spi_register_controller() in
fsl_lpspi_probe() and add the corresponding spi_unregister_controller() in
fsl_lpspi_remove().

Fixes: 5314987de5e5 ("spi: imx: add lpspi bus driver")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/spi/spi-fsl-lpspi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index b361c1bb3e43..45390e9b8cae 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -1009,7 +1009,7 @@ static int fsl_lpspi_probe(struct platform_device *pdev)
 		enable_irq(irq);
 	}
 
-	ret = devm_spi_register_controller(&pdev->dev, controller);
+	ret = spi_register_controller(controller);
 	if (ret < 0) {
 		dev_err_probe(&pdev->dev, ret, "spi_register_controller error\n");
 		goto free_dma;
@@ -1035,6 +1035,7 @@ static void fsl_lpspi_remove(struct platform_device *pdev)
 	struct fsl_lpspi_data *fsl_lpspi =
 				spi_controller_get_devdata(controller);
 
+	spi_unregister_controller(controller);
 	fsl_lpspi_dma_exit(controller);
 
 	pm_runtime_dont_use_autosuspend(fsl_lpspi->dev);

---
base-commit: 8c89a077ca796a2fe248c584e9d7e66cff0388c8
change-id: 20260319-spi-fsl-lpspi-fixes-725c6653c745

Best regards,
--  
Marc Kleine-Budde <mkl@pengutronix.de>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF)
  2026-03-19 18:38 [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF) Marc Kleine-Budde
@ 2026-03-24 15:05 ` Frank Li
  2026-03-24 15:22   ` Marc Kleine-Budde
  2026-03-26 12:27 ` Mark Brown
  1 sibling, 1 reply; 5+ messages in thread
From: Frank Li @ 2026-03-24 15:05 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Mark Brown, Gao Pan, Fugang Duan, kernel, linux-spi, imx, linux-kernel

On Thu, Mar 19, 2026 at 07:38:12PM +0100, Marc Kleine-Budde wrote:
> There is a teardown order issue in the driver. The SPI controller is
> registered using devm_spi_register_controller(), which delays
> unregistration of the SPI controller until after the fsl_lpspi_remove()
> function returns.
>
> As the fsl_lpspi_remove() function synchronously tears down the DMA
> channels, a running SPI transfer triggers the following NULL pointer
> dereference due to use after free:
>
> | fsl_lpspi 42550000.spi: I/O Error in DMA RX
> | Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [...]
> | Call trace:
> |  fsl_lpspi_dma_transfer+0x260/0x340 [spi_fsl_lpspi]
> |  fsl_lpspi_transfer_one+0x198/0x448 [spi_fsl_lpspi]
> |  spi_transfer_one_message+0x49c/0x7c8
> |  __spi_pump_transfer_message+0x120/0x420
> |  __spi_sync+0x2c4/0x520
> |  spi_sync+0x34/0x60
> |  spidev_message+0x20c/0x378 [spidev]
> |  spidev_ioctl+0x398/0x750 [spidev]
> [...]
>
> Switch from devm_spi_register_controller() to spi_register_controller() in
> fsl_lpspi_probe() and add the corresponding spi_unregister_controller() in
> fsl_lpspi_remove().

Feel like it is not correct's fix. devm_spi_register_controller() is quite
common, other SPI controller should have similar problem.

Frank
>
> Fixes: 5314987de5e5 ("spi: imx: add lpspi bus driver")
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/spi/spi-fsl-lpspi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> index b361c1bb3e43..45390e9b8cae 100644
> --- a/drivers/spi/spi-fsl-lpspi.c
> +++ b/drivers/spi/spi-fsl-lpspi.c
> @@ -1009,7 +1009,7 @@ static int fsl_lpspi_probe(struct platform_device *pdev)
>  		enable_irq(irq);
>  	}
>
> -	ret = devm_spi_register_controller(&pdev->dev, controller);
> +	ret = spi_register_controller(controller);
>  	if (ret < 0) {
>  		dev_err_probe(&pdev->dev, ret, "spi_register_controller error\n");
>  		goto free_dma;
> @@ -1035,6 +1035,7 @@ static void fsl_lpspi_remove(struct platform_device *pdev)
>  	struct fsl_lpspi_data *fsl_lpspi =
>  				spi_controller_get_devdata(controller);
>
> +	spi_unregister_controller(controller);
>  	fsl_lpspi_dma_exit(controller);
>
>  	pm_runtime_dont_use_autosuspend(fsl_lpspi->dev);
>
> ---
> base-commit: 8c89a077ca796a2fe248c584e9d7e66cff0388c8
> change-id: 20260319-spi-fsl-lpspi-fixes-725c6653c745
>
> Best regards,
> --
> Marc Kleine-Budde <mkl@pengutronix.de>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF)
  2026-03-24 15:05 ` Frank Li
@ 2026-03-24 15:22   ` Marc Kleine-Budde
  2026-03-24 15:42     ` Mark Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Marc Kleine-Budde @ 2026-03-24 15:22 UTC (permalink / raw)
  To: Frank Li; +Cc: imx, Gao Pan, linux-kernel, linux-spi, Mark Brown, kernel

[-- Attachment #1: Type: text/plain, Size: 2464 bytes --]

On 24.03.2026 11:05:11, Frank Li wrote:
> On Thu, Mar 19, 2026 at 07:38:12PM +0100, Marc Kleine-Budde wrote:
> > There is a teardown order issue in the driver. The SPI controller is
> > registered using devm_spi_register_controller(), which delays
> > unregistration of the SPI controller until after the fsl_lpspi_remove()
> > function returns.
> >
> > As the fsl_lpspi_remove() function synchronously tears down the DMA
> > channels, a running SPI transfer triggers the following NULL pointer
> > dereference due to use after free:
> >
> > | fsl_lpspi 42550000.spi: I/O Error in DMA RX
> > | Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> > [...]
> > | Call trace:
> > |  fsl_lpspi_dma_transfer+0x260/0x340 [spi_fsl_lpspi]
> > |  fsl_lpspi_transfer_one+0x198/0x448 [spi_fsl_lpspi]
> > |  spi_transfer_one_message+0x49c/0x7c8
> > |  __spi_pump_transfer_message+0x120/0x420
> > |  __spi_sync+0x2c4/0x520
> > |  spi_sync+0x34/0x60
> > |  spidev_message+0x20c/0x378 [spidev]
> > |  spidev_ioctl+0x398/0x750 [spidev]
> > [...]
> >
> > Switch from devm_spi_register_controller() to spi_register_controller() in
> > fsl_lpspi_probe() and add the corresponding spi_unregister_controller() in
> > fsl_lpspi_remove().
>
> Feel like it is not correct's fix. devm_spi_register_controller() is quite
> common, other SPI controller should have similar problem.

Have a look at the remove function:

| static void fsl_lpspi_remove(struct platform_device *pdev)
| {
| 	struct spi_controller *controller = platform_get_drvdata(pdev);
| 	struct fsl_lpspi_data *fsl_lpspi =
| 				spi_controller_get_devdata(controller);
|
| 	fsl_lpspi_dma_exit(controller);

It first cleans up the DMA support, but the controller is still
registered. In my use case the /dev/spiX.Y is still active and my test
program can still issue a SPI transfer.

|
| 	pm_runtime_dont_use_autosuspend(fsl_lpspi->dev);
| 	pm_runtime_disable(fsl_lpspi->dev);
| }

The patch unregisters the SPI controller prior the tearing down thee DMA
support.

Another options would be to push everything in the fsl_lpspi_remove() to
devm, too.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF)
  2026-03-24 15:22   ` Marc Kleine-Budde
@ 2026-03-24 15:42     ` Mark Brown
  0 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2026-03-24 15:42 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Frank Li, imx, Gao Pan, linux-kernel, linux-spi, kernel

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On Tue, Mar 24, 2026 at 04:22:30PM +0100, Marc Kleine-Budde wrote:
> On 24.03.2026 11:05:11, Frank Li wrote:
> > On Thu, Mar 19, 2026 at 07:38:12PM +0100, Marc Kleine-Budde wrote:

> > > fsl_lpspi_probe() and add the corresponding spi_unregister_controller() in
> > > fsl_lpspi_remove().

> > Feel like it is not correct's fix. devm_spi_register_controller() is quite
> > common, other SPI controller should have similar problem.

...

> The patch unregisters the SPI controller prior the tearing down thee DMA
> support.

> Another options would be to push everything in the fsl_lpspi_remove() to
> devm, too.

Right, this is the thing with using devm_ for the controller - either
the driver needs to fully use devm_ or it can't use it for registering
the controller itself since you get unwind ordering issues.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF)
  2026-03-19 18:38 [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF) Marc Kleine-Budde
  2026-03-24 15:05 ` Frank Li
@ 2026-03-26 12:27 ` Mark Brown
  1 sibling, 0 replies; 5+ messages in thread
From: Mark Brown @ 2026-03-26 12:27 UTC (permalink / raw)
  To: Frank Li, Gao Pan, Fugang Duan, Marc Kleine-Budde
  Cc: kernel, linux-spi, imx, linux-kernel

On Thu, 19 Mar 2026 19:38:12 +0100, Marc Kleine-Budde wrote:
> spi: spi-fsl-lpspi: fix teardown order issue (UAF)

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.0

Thanks!

[1/1] spi: spi-fsl-lpspi: fix teardown order issue (UAF)
      https://git.kernel.org/broonie/spi/c/b341c1176f2e

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-26 16:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-19 18:38 [PATCH spi] spi: spi-fsl-lpspi: fix teardown order issue (UAF) Marc Kleine-Budde
2026-03-24 15:05 ` Frank Li
2026-03-24 15:22   ` Marc Kleine-Budde
2026-03-24 15:42     ` Mark Brown
2026-03-26 12:27 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®