spi: fix resource leaks on device setup failure
Subsystems (2)
PATCH 1/1 spi: fix resource leaks on device setup failure
Make sure to call controller cleanup() on late device setup failures to avoid leaking resources allocated by setup().
--- drivers/spi/spi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/spi/spi.c +8 -2
@@ -4091,7 +4091,7 @@ int spi_setup(struct spi_device *spi) status =spi_set_cs_timing (spi);if (status) {mutex_unlock (&spi->controller->io_mutex); -return status; + goto err_cleanup; }if (spi->controller->auto_runtime_pm && spi->controller->set_cs) {
@@ -4100,7 +4100,7 @@ int spi_setup(struct spi_device *spi)mutex_unlock (&spi->controller->io_mutex);dev_err (&spi->controller->dev,"Failed to power device: %d\n" , status); -return status; + goto err_cleanup; }/*
@@ -4136,6 +4136,12 @@ int spi_setup(struct spi_device *spi) status);return status; + +err_cleanup: +if (spi->controller->cleanup) + spi->controller->cleanup (spi); + +return status; }EXPORT_SYMBOL_GPL (spi_setup);
Signature · 1 line
2.52.0
Mark Brown
maintainerdrivers/spi/spi.c int spi_setup(struct spi_device *spi)
Quoted 6 lines · Johan Hovold
Make sure to call controller
cleanup()on late device setup failures to avoid leaking resources allocated bysetup().drivers/spi/spi.c +0
-0@@ -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?
drivers/spi/spi.c int spi_setup(struct spi_device *spi) · line 4142
Quoted 3 lines
drivers/spi/spi.c +3
-0+err_cleanup: +if (spi->controller->cleanup) + spi->controller->cleanup (spi);
spi_cleanup()
Quoted 5 lines · Mark Brown
Quoted 2 lines · Johan Hovold
Make sure to call controller
cleanup()on late device setup failures to avoid leaking resources allocated bysetup().
Quoted 8 lines
drivers/spi/spi.c int spi_setup(struct spi_device *spi)
Quoted 3 lines
drivers/spi/spi.c +0
-0@@ -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.
Quoted 5 lines
drivers/spi/spi.c int spi_setup(struct spi_device *spi) · line 4142
Quoted 3 lines
drivers/spi/spi.c +3
-0+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