linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] spi: fix use-after-free of the add_lock mutex
@ 2021-11-11  8:37 Michael Walle
  2021-11-11 12:37 ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Walle @ 2021-11-11  8:37 UTC (permalink / raw)
  To: linux-spi, linux-kernel
  Cc: Mark Brown, Andy Shevchenko, Michael Walle,
	Uwe Kleine-König, Lukas Wunner, stable

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] spi: fix use-after-free of the add_lock mutex
  2021-11-11  8:37 [PATCH] spi: fix use-after-free of the add_lock mutex Michael Walle
@ 2021-11-11 12:37 ` Mark Brown
  2021-11-11 12:46   ` Michael Walle
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Brown @ 2021-11-11 12:37 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-spi, linux-kernel, Andy Shevchenko, Uwe Kleine-König,
	Lukas Wunner, stable

[-- 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 --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spi: fix use-after-free of the add_lock mutex
  2021-11-11 12:37 ` Mark Brown
@ 2021-11-11 12:46   ` Michael Walle
  2021-11-11 13:47     ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Walle @ 2021-11-11 12:46 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, linux-kernel, Andy Shevchenko, Uwe Kleine-König,
	Lukas Wunner, stable

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spi: fix use-after-free of the add_lock mutex
  2021-11-11 12:46   ` Michael Walle
@ 2021-11-11 13:47     ` Mark Brown
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2021-11-11 13:47 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-spi, linux-kernel, Andy Shevchenko, Uwe Kleine-König,
	Lukas Wunner, stable

[-- 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 --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-11 13:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11  8:37 [PATCH] spi: fix use-after-free of the add_lock mutex Michael Walle
2021-11-11 12:37 ` Mark Brown
2021-11-11 12:46   ` Michael Walle
2021-11-11 13:47     ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).