spi: Cleanup on failure of initial setup
[ Upstream commit 2ec6f20b33eb4f62ab90bdcd620436c883ec3af6 ] Commit c7299fea6769 ("spi: Fix spi device unregister flow") changed the SPI core's behavior if the ->setup() hook returns an error upon adding an spi_device: Before, the ->cleanup() hook was invoked to free any allocations that were made by ->setup(). With the commit, that's no longer the case, so the ->setup() hook is expected to free the allocations itself. I've identified 5 drivers which depend on the old behavior and am fixing them up hereinafter: spi-bitbang.c spi-fsl-spi.c spi-omap-uwire.c spi-omap2-mcspi.c spi-pxa2xx.c Importantly, ->setup() is not only invoked on spi_device *addition*: It may subsequently be called to *change* SPI parameters. If changing these SPI parameters fails, freeing memory allocations would be wrong. That should only be done if the spi_device is finally destroyed. I am therefore using a bool "initial_setup" in 4 of the affected drivers to differentiate between the invocation on *adding* the spi_device and any subsequent invocations: spi-bitbang.c spi-fsl-spi.c spi-omap-uwire.c spi-omap2-mcspi.c In spi-pxa2xx.c, it seems the ->setup() hook can only fail on spi_device addition, not any subsequent calls. It therefore doesn't need the bool. It's worth noting that 5 other drivers already perform a cleanup if the ->setup() hook fails. Before c7299fea6769, they caused a double-free if ->setup() failed on spi_device addition. Since the commit, they're fine. These drivers are: spi-mpc512x-psc.c spi-pl022.c spi-s3c64xx.c spi-st-ssc4.c spi-tegra114.c (spi-pxa2xx.c also already performs a cleanup, but only in one of several error paths.) Fixes: c7299fea6769 ("spi: Fix spi device unregister flow") Signed-off-by: Lukas Wunner <lukas@wunner.de> Cc: Saravana Kannan <saravanak@google.com> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # pxa2xx Link: https://lore.kernel.org/r/f76a0599469f265b69c371538794101fa37b5536.1622149321.git.lukas@wunner.de Signed-off-by: Mark Brown <broonie@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:

committed by
Greg Kroah-Hartman

parent
0c4d4de2da
commit
fa05ba6196
@@ -181,6 +181,8 @@ int spi_bitbang_setup(struct spi_device *spi)
|
||||
{
|
||||
struct spi_bitbang_cs *cs = spi->controller_state;
|
||||
struct spi_bitbang *bitbang;
|
||||
bool initial_setup = false;
|
||||
int retval;
|
||||
|
||||
bitbang = spi_master_get_devdata(spi->master);
|
||||
|
||||
@@ -189,22 +191,30 @@ int spi_bitbang_setup(struct spi_device *spi)
|
||||
if (!cs)
|
||||
return -ENOMEM;
|
||||
spi->controller_state = cs;
|
||||
initial_setup = true;
|
||||
}
|
||||
|
||||
/* per-word shift register access, in hardware or bitbanging */
|
||||
cs->txrx_word = bitbang->txrx_word[spi->mode & (SPI_CPOL|SPI_CPHA)];
|
||||
if (!cs->txrx_word)
|
||||
return -EINVAL;
|
||||
if (!cs->txrx_word) {
|
||||
retval = -EINVAL;
|
||||
goto err_free;
|
||||
}
|
||||
|
||||
if (bitbang->setup_transfer) {
|
||||
int retval = bitbang->setup_transfer(spi, NULL);
|
||||
retval = bitbang->setup_transfer(spi, NULL);
|
||||
if (retval < 0)
|
||||
return retval;
|
||||
goto err_free;
|
||||
}
|
||||
|
||||
dev_dbg(&spi->dev, "%s, %u nsec/bit\n", __func__, 2 * cs->nsecs);
|
||||
|
||||
return 0;
|
||||
|
||||
err_free:
|
||||
if (initial_setup)
|
||||
kfree(cs);
|
||||
return retval;
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(spi_bitbang_setup);
|
||||
|
||||
|
Reference in New Issue
Block a user