spi: fix resource leaks on device setup failure
Subsystems (2)
cover spi: fix resource leaks on device setup failure
Make sure to call controller cleanup() if spi_setup() fails while registering a device to avoid leaking any resources allocated by setup().
Johan
Changes in v2:
- only call cleanup() if spi_setup() fails during registration
- use spi_cleanup() helper
- fix controller cleanup() documentation (new patch)
Johan Hovold (2):
spi: fix resource leaks on device setup failure
spi: fix controller cleanup() documentation
drivers/spi/spi.c | 61 +++++++++++++++++++++++++---------------- include/linux/spi/spi.h | 2 +- 2 files changed, 38 insertions(+), 25 deletions(-)
Signature · 1 line
2.52.0
PATCH 1/2 spi: fix resource leaks on device setup failure
Make sure to call controller cleanup() if spi_setup() fails while registering a device to avoid leaking any resources allocated by setup().
--- drivers/spi/spi.c | 61 ++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 24 deletions(-)
drivers/spi/spi.c +37 -24
@@ -43,6 +43,8 @@ EXPORT_TRACEPOINT_SYMBOL(spi_transfer_stop);#include "internals.h" +static int __spi_setup (struct spi_device *spi,bool initial_setup); +static DEFINE_IDR (spi_controller_idr);static void spidev_release (struct device *dev)
@@ -726,7 +728,7 @@ static int __spi_add_device(struct spi_device *spi)* normally rely on the device being setup. Devices * using SPI_CS_HIGH can't coexist well otherwise... */ - status =spi_setup (spi); + status =__spi_setup (spi, true);if (status <0 ) {dev_err (dev,"can't setup %s, status %d\n" ,dev_name (&spi->dev), status);
@@ -3992,27 +3994,7 @@ static int spi_set_cs_timing(struct spi_device *spi)return status; } -/** -* spi_setup - setup SPI mode and clock rate -* @spi: the device whose settings are being modified -* Context: can sleep, and no requests are queued to the device -* -* SPI protocol drivers may need to update the transfer mode if the -* device doesn't work with its default. They may likewise need -* to update clock rates or word sizes from initial values. This function -* changes those settings, and must be called from a context that can sleep. -* Except for SPI_CS_HIGH, which takes effect immediately, the changes take -* effect the next time the device is selected and data is transferred to -* or from it. When this function returns, the SPI device is deselected. -* -* Note that this call will fail if the protocol driver specifies an option -* that the underlying controller or its driver does not support. For -* example, not all hardware supports wire transfers using nine bit words, -* LSB-first wire encoding, or active-high chipselects. -* -* Return: zero on success, else a negative error code. -*/ -int spi_setup (struct spi_device *spi) +static int __spi_setup (struct spi_device *spi,bool initial_setup) {unsigned bad_bits, ugly_bits;int status;
@@ -4097,7 +4079,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) {
@@ -4106,7 +4088,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; }/*
@@ -4142,6 +4124,37 @@ int spi_setup(struct spi_device *spi) status);return status; + +err_cleanup: +if (initial_setup) +spi_cleanup (spi); + +return status; +} + +/** +* spi_setup - setup SPI mode and clock rate +* @spi: the device whose settings are being modified +* Context: can sleep, and no requests are queued to the device +* +* SPI protocol drivers may need to update the transfer mode if the +* device doesn't work with its default. They may likewise need +* to update clock rates or word sizes from initial values. This function +* changes those settings, and must be called from a context that can sleep. +* Except for SPI_CS_HIGH, which takes effect immediately, the changes take +* effect the next time the device is selected and data is transferred to +* or from it. When this function returns, the SPI device is deselected. +* +* Note that this call will fail if the protocol driver specifies an option +* that the underlying controller or its driver does not support. For +* example, not all hardware supports wire transfers using nine bit words, +* LSB-first wire encoding, or active-high chipselects. +* +* Return: zero on success, else a negative error code. +*/ +int spi_setup (struct spi_device *spi) +{ +return __spi_setup (spi, false); }EXPORT_SYMBOL_GPL (spi_setup);
Signature · 1 line
2.52.0
PATCH 2/2 spi: fix controller cleanup() documentation
In reply to Johan HovoldThe controller cleanup() callback is no longer called when releasing a device, but rather when deregistering it (and on registration failures).
--- include/linux/spi/spi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/spi/spi.h +1 -1
@@ -700,7 +700,7 @@ struct spi_controller {int (*transfer)(struct spi_device *spi,struct spi_message *mesg); -/* Called on release() to free memory provided by spi_controller */ +/* Called on deregistration to free memory provided by spi_controller */ void (*cleanup)(struct spi_device *spi);/*
Signature · 1 line
2.52.0
Quoted 8 lines · Johan Hovold
Make sure to call controller
cleanup()ifspi_setup()fails while registering a device to avoid leaking any resources allocated bysetup().Changes in v2:
- only callcleanup()ifspi_setup()fails during registration
- usespi_cleanup()helper
- fix controllercleanup()documentation (new patch)
Bah, I missed to add "v2" to Subject. Let me know if you prefer me to send a v3...
Johan
Mark Brown
maintainerQuoted 9 lines · Johan Hovold
spi: fix resource leaks on device setup failure
Make sure to call controller
cleanup()ifspi_setup()fails while registering a device to avoid leaking any resources allocated bysetup().Johan
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.1
Thanks!
[1/2] spi: fix resource leaks on device setup failure
https://git.kernel.org/broonie/spi/c/db357034f7e0 [2/2] spi: fix controller cleanup() documentation https://git.kernel.org/broonie/spi/c/a6e23843e949
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