Re: [PATCH] spi: fix resource leaks on device setup failure

[Johan Hovold <johan@kernel.org>]

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

On Tue, Mar 24, 2026 at 01:23:56PM +0000, Mark Brown wrote:
> On Tue, Mar 24, 2026 at 11:30:42AM +0100, Johan Hovold wrote:
> 
> > Make sure to call controller cleanup() on late device setup failures to
> > avoid leaking resources allocated by setup().
> 
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -4091,7 +4091,7 @@ int spi_setup(struct spi_device *spi)
> 
> This is specifically setup().  This is called repeatedly at runtime,
> it's not part of the device registration flow like the issues that were
> addressed by the commit you tagged as a fix.  Can you be more specific
> about the leak you're trying to fix here?

It's when spi_add_device() device fails in spi_setup() that cleanup()
may no longer be called.

	spi_add_device()
	  __spi_add_device()
	    spi_setup()
	      controller->setup()
	      return -<errno>;

And there are drivers that allocate memory in the controller->setup()
callback which is released in ->cleanup() (e.g. atmel_spi_setup() and
atmel_spi_cleanup()).

From a quick look at some of the drivers it did not seem limited to
memory allocations.

But I see now that not all of them can handle cleanup() being called
more than once, which could indeed happen if a later call to spi_setup()
fails after the device has been added.

We could set a flag to track whether setup() has ever been called
successfully, or make sure that cleanup() can be called more than once
for all drivers.

> > +err_cleanup:
> > +       if (spi->controller->cleanup)
> > +               spi->controller->cleanup(spi);
> 
> spi_cleanup()

I open-coded as I'm matching the call to ->setup() inside spi_setup()
with ->cleanup() (i.e. not spi_setup() with spi_cleanup()).

Johan

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