[PATCH] spi: fix use-after-free of the add_lock mutex

[PATCH] spi: fix use-after-free of the add_lock mutex

[Michael Walle]

Commit 6098475d4cb4 ("spi: Fix deadlock when adding SPI controllers on
SPI buses") introduced a per-controller mutex. But mutex_unlock() of
said lock is called after the controller is already freed:

  spi_unregister_controller(ctlr)
  -> put_device(&ctlr->dev)
    -> spi_controller_release(dev)
  -> mutex_unlock(&ctrl->add_lock)

Move the put_device() after the mutex_unlock().

Fixes: 6098475d4cb4 ("spi: Fix deadlock when adding SPI controllers on SPI buses")
Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v5.15
---
changes since RFC:
 - fix call graph indendation in commit message

 drivers/spi/spi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b23e675953e1..fdd530b150a7 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3099,12 +3099,6 @@ void spi_unregister_controller(struct spi_controller *ctlr)
 
 	device_del(&ctlr->dev);
 
-	/* Release the last reference on the controller if its driver
-	 * has not yet been converted to devm_spi_alloc_master/slave().
-	 */
-	if (!ctlr->devm_allocated)
-		put_device(&ctlr->dev);
-
 	/* free bus id */
 	mutex_lock(&board_lock);
 	if (found == ctlr)
@@ -3113,6 +3107,12 @@ void spi_unregister_controller(struct spi_controller *ctlr)
 
 	if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
 		mutex_unlock(&ctlr->add_lock);
+
+	/* Release the last reference on the controller if its driver
+	 * has not yet been converted to devm_spi_alloc_master/slave().
+	 */
+	if (!ctlr->devm_allocated)
+		put_device(&ctlr->dev);
 }
 EXPORT_SYMBOL_GPL(spi_unregister_controller);
 
-- 
2.30.2

Re: [PATCH] spi: fix use-after-free of the add_lock mutex

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

On Thu, Nov 11, 2021 at 09:37:13AM +0100, Michael Walle wrote:

> ---
> changes since RFC:
>  - fix call graph indendation in commit message

If you are sending a new version of something please flag that in the
commit message, this helps both people and automated systems identify
that this is a new version of the same thing.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] spi: fix use-after-free of the add_lock mutex

[Michael Walle]

Am 2021-11-11 13:37, schrieb Mark Brown:
> On Thu, Nov 11, 2021 at 09:37:13AM +0100, Michael Walle wrote:
> 
>> ---
>> changes since RFC:
>>  - fix call graph indendation in commit message
> 
> If you are sending a new version of something please flag that in the
> commit message, this helps both people and automated systems identify
> that this is a new version of the same thing.

Are RFC patches eligible to be picked up? I wasn't sure if I had to
resend it at all. But since there was a mistake in the commit message
anyway, I went ahead and the the first "real" version. How would
you flag that? Isn't changing the subject from "[PATCH RFC]" (ok it
was "RFC PATCH", my bad) to "[PATCH]" enough?

-michael

Re: [PATCH] spi: fix use-after-free of the add_lock mutex

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

On Thu, Nov 11, 2021 at 01:46:01PM +0100, Michael Walle wrote:
> Am 2021-11-11 13:37, schrieb Mark Brown:

> > If you are sending a new version of something please flag that in the
> > commit message, this helps both people and automated systems identify
> > that this is a new version of the same thing.

> Are RFC patches eligible to be picked up? I wasn't sure if I had to
> resend it at all. But since there was a mistake in the commit message
> anyway, I went ahead and the the first "real" version. How would
> you flag that? Isn't changing the subject from "[PATCH RFC]" (ok it
> was "RFC PATCH", my bad) to "[PATCH]" enough?

No, both people and machines are going to get confused.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]