2021-11-11 08:37:56

by Michael Walle

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

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 <[email protected]>
Reviewed-by: Uwe Kleine-König <[email protected]>
Reviewed-by: Lukas Wunner <[email protected]>
Cc: [email protected] # 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



2021-11-11 12:37:23

by Mark Brown

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

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.


Attachments:
(No filename) (328.00 B)
signature.asc (488.00 B)
Download all attachments

2021-11-11 12:46:06

by Michael Walle

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

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

2021-11-11 13:47:24

by Mark Brown

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

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.


Attachments:
(No filename) (690.00 B)
signature.asc (488.00 B)
Download all attachments