1. Patchew
  2. linux
  3. [PATCH 0/5] spi: imx: fix use-after-free on unbind
  4. View patch

[PATCH 1/5] spi: imx: fix use-after-free on unbind

Johan Hovold posted 5 patches 1 week, 4 days ago
There is a newer version of this series
[PATCH 1/5] spi: imx: fix use-after-free on unbind
Posted by Johan Hovold 1 week, 4 days ago 
The SPI subsystem frees the controller and any subsystem allocated
driver data as part of deregistration (unless the allocation is device
managed).

Take another reference before deregistering the controller so that the
driver data is not freed until the driver is done with it.

Fixes: 307c897db762 ("spi: spi-imx: replace struct spi_imx_data::bitbang by pointer to struct spi_controller")
Cc: stable@vger.kernel.org	# 5.19
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/spi/spi-imx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 64c6c09e1e7b..a8d90c86a8a1 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -2401,6 +2401,8 @@ static void spi_imx_remove(struct platform_device *pdev)
 	struct spi_imx_data *spi_imx = spi_controller_get_devdata(controller);
 	int ret;
 
+	spi_controller_get(controller);
+
 	spi_unregister_controller(controller);
 
 	ret = pm_runtime_get_sync(spi_imx->dev);
@@ -2414,6 +2416,8 @@ static void spi_imx_remove(struct platform_device *pdev)
 	pm_runtime_disable(spi_imx->dev);
 
 	spi_imx_sdma_exit(spi_imx);
+
+	spi_controller_put(controller);
 }
 
 static int spi_imx_runtime_resume(struct device *dev)
-- 
2.52.0
Re: [PATCH 1/5] spi: imx: fix use-after-free on unbind
Posted by Marc Kleine-Budde 1 week, 4 days ago 
On 23.03.2026 11:49:44, Johan Hovold wrote:
> The SPI subsystem frees the controller and any subsystem allocated
> driver data as part of deregistration (unless the allocation is device
> managed).
>
> Take another reference before deregistering the controller so that the
> driver data is not freed until the driver is done with it.

Would re-ordering the spi_imx_remove() function be an alternative fix?
I.e. call spi_unregister_controller() last?

regards,
Marc

>
> Fixes: 307c897db762 ("spi: spi-imx: replace struct spi_imx_data::bitbang by pointer to struct spi_controller")
> Cc: stable@vger.kernel.org	# 5.19
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/spi/spi-imx.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 64c6c09e1e7b..a8d90c86a8a1 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -2401,6 +2401,8 @@ static void spi_imx_remove(struct platform_device *pdev)
>  	struct spi_imx_data *spi_imx = spi_controller_get_devdata(controller);
>  	int ret;
>
> +	spi_controller_get(controller);
> +
>  	spi_unregister_controller(controller);
>
>  	ret = pm_runtime_get_sync(spi_imx->dev);
> @@ -2414,6 +2416,8 @@ static void spi_imx_remove(struct platform_device *pdev)
>  	pm_runtime_disable(spi_imx->dev);
>
>  	spi_imx_sdma_exit(spi_imx);
> +
> +	spi_controller_put(controller);
>  }
>
>  static int spi_imx_runtime_resume(struct device *dev)
> --
> 2.52.0
>
>

-- 
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   |
Re: [PATCH 1/5] spi: imx: fix use-after-free on unbind
Posted by Johan Hovold 1 week, 4 days ago 
On Mon, Mar 23, 2026 at 12:00:59PM +0100, Marc Kleine-Budde wrote:
> On 23.03.2026 11:49:44, Johan Hovold wrote:
> > The SPI subsystem frees the controller and any subsystem allocated
> > driver data as part of deregistration (unless the allocation is device
> > managed).
> >
> > Take another reference before deregistering the controller so that the
> > driver data is not freed until the driver is done with it.
> 
> Would re-ordering the spi_imx_remove() function be an alternative fix?
> I.e. call spi_unregister_controller() last?

No, the controller needs to be deregistered before disabling clocks and
releasing other resources.

Johan
Re: [PATCH 1/5] spi: imx: fix use-after-free on unbind
Posted by Marc Kleine-Budde 1 week, 4 days ago 
On 23.03.2026 12:20:08, Johan Hovold wrote:
> On Mon, Mar 23, 2026 at 12:00:59PM +0100, Marc Kleine-Budde wrote:
> > On 23.03.2026 11:49:44, Johan Hovold wrote:
> > > The SPI subsystem frees the controller and any subsystem allocated
> > > driver data as part of deregistration (unless the allocation is device
> > > managed).
> > >
> > > Take another reference before deregistering the controller so that the
> > > driver data is not freed until the driver is done with it.
> >
> > Would re-ordering the spi_imx_remove() function be an alternative fix?
> > I.e. call spi_unregister_controller() last?
>
> No, the controller needs to be deregistered before disabling clocks and
> releasing other resources.

I see. So the API is a bit strange to use:

Allocate with spi_alloc_host(), free with spi_controller_put() before
spi_register_controller(), the free with spi_unregister_controller()
afterwards.

But spi_unregister_controller() shuts down the SPI interface _and_ frees
the memory. Which is the culprit here.

Would using devm_spi_alloc_host() be an option here?

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   |
Re: [PATCH 1/5] spi: imx: fix use-after-free on unbind
Posted by Johan Hovold 1 week, 4 days ago 
On Mon, Mar 23, 2026 at 12:57:42PM +0100, Marc Kleine-Budde wrote:
> On 23.03.2026 12:20:08, Johan Hovold wrote:
> > On Mon, Mar 23, 2026 at 12:00:59PM +0100, Marc Kleine-Budde wrote:
> > > On 23.03.2026 11:49:44, Johan Hovold wrote:
> > > > The SPI subsystem frees the controller and any subsystem allocated
> > > > driver data as part of deregistration (unless the allocation is device
> > > > managed).
> > > >
> > > > Take another reference before deregistering the controller so that the
> > > > driver data is not freed until the driver is done with it.
> > >
> > > Would re-ordering the spi_imx_remove() function be an alternative fix?
> > > I.e. call spi_unregister_controller() last?
> >
> > No, the controller needs to be deregistered before disabling clocks and
> > releasing other resources.
> 
> I see. So the API is a bit strange to use:
> 
> Allocate with spi_alloc_host(), free with spi_controller_put() before
> spi_register_controller(), the free with spi_unregister_controller()
> afterwards.
>
> But spi_unregister_controller() shuts down the SPI interface _and_ frees
> the memory. Which is the culprit here.

Indeed, it's a known issue with the SPI API. See for example:

	68b892f1fdc4 ("spi: document odd controller reference handling")
	5e844cc37a5c ("spi: Introduce device-managed SPI controller allocation")
	f0c35a024cce ("spi: fix misleading controller deregistration kernel-doc")

> Would using devm_spi_alloc_host() be an option here?

It can also be used, but that's more intrusive so I did that as a
follow-on cleanup to the fix (see patch 2/5).

Johan
Re: [PATCH 1/5] spi: imx: fix use-after-free on unbind
Posted by Marc Kleine-Budde 1 week, 4 days ago 
On 23.03.2026 14:59:49, Johan Hovold wrote:
> On Mon, Mar 23, 2026 at 12:57:42PM +0100, Marc Kleine-Budde wrote:
> > On 23.03.2026 12:20:08, Johan Hovold wrote:
> > > On Mon, Mar 23, 2026 at 12:00:59PM +0100, Marc Kleine-Budde wrote:
> > > > On 23.03.2026 11:49:44, Johan Hovold wrote:
> > > > > The SPI subsystem frees the controller and any subsystem allocated
> > > > > driver data as part of deregistration (unless the allocation is device
> > > > > managed).
> > > > >
> > > > > Take another reference before deregistering the controller so that the
> > > > > driver data is not freed until the driver is done with it.
> > > >
> > > > Would re-ordering the spi_imx_remove() function be an alternative fix?
> > > > I.e. call spi_unregister_controller() last?
> > >
> > > No, the controller needs to be deregistered before disabling clocks and
> > > releasing other resources.
> >
> > I see. So the API is a bit strange to use:
> >
> > Allocate with spi_alloc_host(), free with spi_controller_put() before
> > spi_register_controller(), the free with spi_unregister_controller()
> > afterwards.
> >
> > But spi_unregister_controller() shuts down the SPI interface _and_ frees
> > the memory. Which is the culprit here.
>
> Indeed, it's a known issue with the SPI API. See for example:
>
> 	68b892f1fdc4 ("spi: document odd controller reference handling")
> 	5e844cc37a5c ("spi: Introduce device-managed SPI controller allocation")
> 	f0c35a024cce ("spi: fix misleading controller deregistration kernel-doc")
>
> > Would using devm_spi_alloc_host() be an option here?
>
> It can also be used, but that's more intrusive so I did that as a
> follow-on cleanup to the fix (see patch 2/5).

Ah, nice! At the time I replied to the patch, the whole series was not
available on lore, yet.

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>

And thanks for taking the time in explaining me the details :)

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   |
Re: [PATCH 1/5] spi: imx: fix use-after-free on unbind
Posted by Johan Hovold 1 week, 4 days ago 
On Mon, Mar 23, 2026 at 03:47:45PM +0100, Marc Kleine-Budde wrote:
> On 23.03.2026 14:59:49, Johan Hovold wrote:

> > Indeed, it's a known issue with the SPI API. See for example:
> >
> > 	68b892f1fdc4 ("spi: document odd controller reference handling")
> > 	5e844cc37a5c ("spi: Introduce device-managed SPI controller allocation")

> > 	f0c35a024cce ("spi: fix misleading controller deregistration kernel-doc")

This was supposed to say

	3f174274d224 ("spi: fix misleading controller deregistration kernel-doc")

> > > Would using devm_spi_alloc_host() be an option here?
> >
> > It can also be used, but that's more intrusive so I did that as a
> > follow-on cleanup to the fix (see patch 2/5).
> 
> Ah, nice! At the time I replied to the patch, the whole series was not
> available on lore, yet.

Sorry, I should have CC:ed you the whole series.

> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> And thanks for taking the time in explaining me the details :)

No worries. :)

Johan

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.