Re: [PATCH 1/5] spi: imx: fix use-after-free on unbind
From: Johan Hovold
Date: Mon Mar 23 2026 - 10:25:32 EST
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
Attachment:
signature.asc
Description: PGP signature