lwn.git/drivers/spi, branch v5.10-rc5

spi: npcm-fiu: Don't leak SPI master in probe error path

2020-11-17T17:08:39+00:00

If the calls to of_match_device(), of_alias_get_id(), devm_ioremap_resource(), devm_regmap_init_mmio() or devm_clk_get() fail on probe of the NPCM FIU SPI driver, the spi_controller struct is erroneously not freed. Fix by switching over to the new devm_spi_alloc_master() helper. Fixes: ace55c411b11 ("spi: npcm-fiu: add NPCM FIU controller driver") Signed-off-by: Lukas Wunner Cc: # v5.4+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation Cc: # v5.4+ Cc: Tomer Maimon Link: https://lore.kernel.org/r/a420c23a363a3bc9aa684c6e790c32a8af106d17.1605512876.git.lukas@wunner.de Signed-off-by: Mark Brown

spi: dw: Set transfer handler before unmasking the IRQs

2020-11-17T17:08:39+00:00

It turns out the IRQs most like can be unmasked before the controller is enabled with no problematic consequences. The manual doesn't explicitly state that, but the examples perform the controller initialization procedure in that order. So the commit da8f58909e7e ("spi: dw: Unmask IRQs after enabling the chip") hasn't been that required as I thought. But anyway setting the IRQs up after the chip enabling still worth adding since it has simplified the code a bit. The problem is that it has introduced a potential bug. The transfer handler pointer is now initialized after the IRQs are enabled. That may and eventually will cause an invalid or uninitialized callback invocation. Fix that just by performing the callback initialization before the IRQ unmask procedure. Fixes: da8f58909e7e ("spi: dw: Unmask IRQs after enabling the chip") Signed-off-by: Serge Semin Link: https://lore.kernel.org/r/20201117094054.4696-1-Sergey.Semin@baikalelectronics.ru Signed-off-by: Mark Brown

spi: cadence-quadspi: Fix error return code in cqspi_probe

2020-11-16T20:02:42+00:00

Fix to return the error code from devm_reset_control_get_optional_exclusive() instaed of 0 in cqspi_probe(). Fixes: 31fb632b5d43ca ("spi: Move cadence-quadspi driver to drivers/spi/") Reported-by: Hulk Robot Signed-off-by: Zhihao Cheng Reviewed-by: Philipp Zabel Link: https://lore.kernel.org/r/20201116141836.2970579-1-chengzhihao1@huawei.com Signed-off-by: Mark Brown

spi: bcm2835aux: Restore err assignment in bcm2835aux_spi_probe

2020-11-13T18:22:24+00:00

Clang warns: drivers/spi/spi-bcm2835aux.c:532:50: warning: variable 'err' is uninitialized when used here [-Wuninitialized] dev_err(&pdev->dev, "could not get clk: %d\n", err); ^~~ ./include/linux/dev_printk.h:112:32: note: expanded from macro 'dev_err' _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__) ^~~~~~~~~~~ drivers/spi/spi-bcm2835aux.c:495:9: note: initialize the variable 'err' to silence this warning int err; ^ = 0 1 warning generated. Restore the assignment so that the error value can be used in the dev_err statement and there is no uninitialized memory being leaked. Fixes: e13ee6cc4781 ("spi: bcm2835aux: Fix use-after-free on unbind") Link: https://github.com/ClangBuiltLinux/linux/issues/1199 Signed-off-by: Nathan Chancellor Link: https://lore.kernel.org/r/20201113180701.455541-1-natechancellor@gmail.com Signed-off-by: Mark Brown

Merge series "Use-after-free be gone" from Lukas Wunner :

2020-11-12T19:34:52+00:00

Here's my proposal to fix the use-after-free bugs reported by Sascha Hauer and Florian Fainelli: I scrutinized all SPI drivers in the v5.10 tree: * There are 9 drivers with a use-after-free in the ->remove() hook caused by accessing driver private data after spi_unregister_controller(). * There are 8 drivers which leak the spi_controller in the ->probe() error path because of a missing spi_controller_put(). I'm introducing devm_spi_alloc_master/slave() which automatically calls spi_controller_put() on ->remove(). This fixes both classes of bugs while at the same time reducing code amount and complexity in the ->probe() hook. I propose that spi_controller_unregister() should no longer release a reference on the spi_controller. Instead, drivers need to either do it themselves or use one of the devm functions introduced herein. The vast majority of drivers can be converted to the devm functions. See the commit message of patch [1/4] for the rationale and details. Enclosed are patches for 3 Broadcom drivers. Patches for the other drivers are on this branch: https://github.com/l1k/linux/commits/spi_fixes @Florian Fainelli: Could you verify that there are no KASAN splats or leaks with these patches? Unfortunately I do not have any SPI-capable hardware at my disposal right now, so can only compile-test. You may want to augment spi_controller_release() with a printk() to log when the spi_controller is freed. @Mark Brown: Patches [2/4] to [4/4] reference the SHA-1 of patch [1/4] in their stable tags. Because the hash is unknown to me until you apply the patch, I've used "123456789abc" as a placeholder. You'll have to replace the hash if/when applying. Alternatively, only apply patch [1/4] and I'll repost the other patches with the hash fixed up. Thanks! Lukas Wunner (4): spi: Introduce device-managed SPI controller allocation spi: bcm2835: Fix use-after-free on unbind spi: bcm2835aux: Fix use-after-free on unbind spi: bcm-qspi: Fix use-after-free on unbind drivers/spi/spi-bcm-qspi.c | 34 ++++++++------------- drivers/spi/spi-bcm2835.c | 24 +++++---------- drivers/spi/spi-bcm2835aux.c | 21 +++++-------- drivers/spi/spi.c | 58 +++++++++++++++++++++++++++++++++++- include/linux/spi/spi.h | 19 ++++++++++++ 5 files changed, 103 insertions(+), 53 deletions(-) -- 2.28.0

spi: lpspi: Fix use-after-free on unbind

2020-11-12T18:45:56+00:00

Normally the last reference on an spi_controller is released by spi_unregister_controller(). In the case of the i.MX lpspi driver, the spi_controller is registered with devm_spi_register_controller(), so spi_unregister_controller() is invoked automatically after the driver has unbound. However the driver already releases the last reference in fsl_lpspi_remove() through a gratuitous call to spi_master_put(), causing a use-after-free when spi_unregister_controller() is subsequently invoked by the devres framework. Fix by dropping the superfluous spi_master_put(). Fixes: 944c01a889d9 ("spi: lpspi: enable runtime pm for lpspi") Signed-off-by: Lukas Wunner Cc: # v5.2+ Cc: Han Xu Link: https://lore.kernel.org/r/ab3c0b18bd820501a12c85e440006e09ec0e275f.1604874488.git.lukas@wunner.de Signed-off-by: Mark Brown

spi: bcm-qspi: Fix use-after-free on unbind

2020-11-12T15:05:37+00:00

bcm_qspi_remove() calls spi_unregister_master() even though bcm_qspi_probe() calls devm_spi_register_master(). The spi_master is therefore unregistered and freed twice on unbind. Moreover, since commit 0392727c261b ("spi: bcm-qspi: Handle clock probe deferral"), bcm_qspi_probe() leaks the spi_master allocation if the call to devm_clk_get_optional() fails. Fix by switching over to the new devm_spi_alloc_master() helper which keeps the private data accessible until the driver has unbound and also avoids the spi_master leak on probe. While at it, fix an ordering issue in bcm_qspi_remove() wherein spi_unregister_master() is called after uninitializing the hardware, disabling the clock and freeing an IRQ data structure. The correct order is to call spi_unregister_master() *before* those teardown steps because bus accesses may still be ongoing until that function returns. Fixes: fa236a7ef240 ("spi: bcm-qspi: Add Broadcom MSPI driver") Signed-off-by: Lukas Wunner Cc: # v4.9+: 123456789abc: spi: Introduce device-managed SPI controller allocation Cc: # v4.9+ Cc: Kamal Dasu Acked-by: Florian Fainelli Tested-by: Florian Fainelli Link: https://lore.kernel.org/r/5e31a9a59fd1c0d0b795b2fe219f25e5ee855f9d.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown

spi: bcm2835aux: Fix use-after-free on unbind

2020-11-12T15:05:36+00:00

bcm2835aux_spi_remove() accesses the driver's private data after calling spi_unregister_master() even though that function releases the last reference on the spi_master and thereby frees the private data. Fix by switching over to the new devm_spi_alloc_master() helper which keeps the private data accessible until the driver has unbound. Fixes: b9dd3f6d4172 ("spi: bcm2835aux: Fix controller unregister order") Signed-off-by: Lukas Wunner Cc: # v4.4+: 123456789abc: spi: Introduce device-managed SPI controller allocation Cc: # v4.4+: b9dd3f6d4172: spi: bcm2835aux: Fix controller unregister order Cc: # v4.4+ Link: https://lore.kernel.org/r/b290b06357d0c0bdee9cecc539b840a90630f101.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown

spi: bcm2835: Fix use-after-free on unbind

2020-11-12T15:05:35+00:00

bcm2835_spi_remove() accesses the driver's private data after calling spi_unregister_controller() even though that function releases the last reference on the spi_controller and thereby frees the private data. Fix by switching over to the new devm_spi_alloc_master() helper which keeps the private data accessible until the driver has unbound. Fixes: f8043872e796 ("spi: add driver for BCM2835") Reported-by: Sascha Hauer Reported-by: Florian Fainelli Signed-off-by: Lukas Wunner Cc: # v3.10+: 123456789abc: spi: Introduce device-managed SPI controller allocation Cc: # v3.10+ Cc: Vladimir Oltean Tested-by: Florian Fainelli Acked-by: Florian Fainelli Link: https://lore.kernel.org/r/ad66e0a0ad96feb848814842ecf5b6a4539ef35c.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown

spi: Introduce device-managed SPI controller allocation

2020-11-12T15:05:34+00:00

SPI driver probing currently comprises two steps, whereas removal comprises only one step: spi_alloc_master() spi_register_controller() spi_unregister_controller() That's because spi_unregister_controller() calls device_unregister() instead of device_del(), thereby releasing the reference on the spi_controller which was obtained by spi_alloc_master(). An SPI driver's private data is contained in the same memory allocation as the spi_controller struct. Thus, once spi_unregister_controller() has been called, the private data is inaccessible. But some drivers need to access it after spi_unregister_controller() to perform further teardown steps. Introduce devm_spi_alloc_master() and devm_spi_alloc_slave(), which release a reference on the spi_controller struct only after the driver has unbound, thereby keeping the memory allocation accessible. Change spi_unregister_controller() to not release a reference if the spi_controller was allocated by one of these new devm functions. The present commit is small enough to be backportable to stable. It allows fixing drivers which use the private data in their ->remove() hook after it's been freed. It also allows fixing drivers which neglect to release a reference on the spi_controller in the probe error path. Long-term, most SPI drivers shall be moved over to the devm functions introduced herein. The few that can't shall be changed in a treewide commit to explicitly release the last reference on the controller. That commit shall amend spi_unregister_controller() to no longer release a reference, thereby completing the migration. As a result, the behaviour will be less surprising and more consistent with subsystems such as IIO, which also includes the private data in the allocation of the generic iio_dev struct, but calls device_del() in iio_device_unregister(). Signed-off-by: Lukas Wunner Link: https://lore.kernel.org/r/272bae2ef08abd21388c98e23729886663d19192.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown