The atcspi_exec_mem_op() function may call mutex_lock() on the
driver's mutex before it is properly initialized if a SPI memory
operation is initiated immediately after devm_spi_register_controller()
is called. The mutex initialization currently occurs after the
controller registration, which leaves a window where the mutex could
be used uninitialized.
Move the mutex initialization to the beginning of the probe function,
before any registration or resource allocation. Also replace the
plain mutex_init() with devm_mutex_init() to benefit from automatic
cleanup on driver unbind, preventing potential resource leaks.
Fixes: 34e3815ea459 ("spi: atcspi200: Add ATCSPI200 SPI controller driver")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
drivers/spi/spi-atcspi200.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-atcspi200.c b/drivers/spi/spi-atcspi200.c
index c0607193be8d..dd1292c5bb98 100644
--- a/drivers/spi/spi-atcspi200.c
+++ b/drivers/spi/spi-atcspi200.c
@@ -551,6 +551,10 @@ static int atcspi_probe(struct platform_device *pdev)
spi->dev = &pdev->dev;
dev_set_drvdata(&pdev->dev, host);
+ ret = devm_mutex_init(spi->dev, &spi->mutex_lock);
+ if (ret)
+ goto free_controller;
+
ret = atcspi_init_resources(pdev, spi, &mem_res);
if (ret)
goto free_controller;
@@ -580,7 +584,6 @@ static int atcspi_probe(struct platform_device *pdev)
else
spi->use_dma = true;
}
- mutex_init(&spi->mutex_lock);
return 0;
--
2.25.1
On Wed, Mar 11, 2026 at 09:55:14AM +0800, Pei Xiao wrote: > @@ -551,6 +551,10 @@ static int atcspi_probe(struct platform_device *pdev) > spi->dev = &pdev->dev; > dev_set_drvdata(&pdev->dev, host); > > + ret = devm_mutex_init(spi->dev, &spi->mutex_lock); > + if (ret) > + goto free_controller; > + This means we're now using devm to free the mutex that is in spi, but when we pass spi into devm_spi_register_controller() we'll cause spi to be freed before we free the mutex. This pattern gets a bit clunky with devm...
在 2026/3/11 20:29, Mark Brown 写道: > On Wed, Mar 11, 2026 at 09:55:14AM +0800, Pei Xiao wrote: > >> @@ -551,6 +551,10 @@ static int atcspi_probe(struct platform_device *pdev) >> spi->dev = &pdev->dev; >> dev_set_drvdata(&pdev->dev, host); >> >> + ret = devm_mutex_init(spi->dev, &spi->mutex_lock); >> + if (ret) >> + goto free_controller; >> + > This means we're now using devm to free the mutex that is in spi, but > when we pass spi into devm_spi_register_controller() we'll cause spi to > be freed before we free the mutex. This pattern gets a bit clunky with > devm... Thanks for review! I will use mutex_init() for V3 version.
On Thu, Mar 12, 2026 at 10:25:11AM +0800, Pei Xiao wrote: > 在 2026/3/11 20:29, Mark Brown 写道: > > On Wed, Mar 11, 2026 at 09:55:14AM +0800, Pei Xiao wrote: > >> + ret = devm_mutex_init(spi->dev, &spi->mutex_lock); > >> + if (ret) > >> + goto free_controller; > >> + > > This means we're now using devm to free the mutex that is in spi, but > > when we pass spi into devm_spi_register_controller() we'll cause spi to > > be freed before we free the mutex. This pattern gets a bit clunky with > > devm... > Thanks for review! I will use mutex_init() for V3 version. That's still going to have trouble because if you do the free in the removal function the lock will go away before the controller which is it's own issue (this is what I mean about clunky). It's especially annoying as it's only an issue for debugging builds. I can't actually see a good way of dealing with this, perhaps just a plain mutex_init() without a free is the least bad option.
© 2016 - 2026 Red Hat, Inc.