* [PATCH] leds: lm3601x: Don't use mutex after it was destroyed
@ 2022-05-13 8:18 Uwe Kleine-König
2022-05-13 14:02 ` Pavel Machek
0 siblings, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2022-05-13 8:18 UTC (permalink / raw)
To: Pavel Machek, Linus Walleij, Dan Murphy; +Cc: linux-leds, kernel
The mutex might still be in use until the devm cleanup callback
devm_led_classdev_flash_release() is called. This only happens some time
after lm3601x_remove() completed.
Fixes: e63a744871a3 ("leds: lm3601x: Convert class registration to device managed")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/leds/flash/leds-lm3601x.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/leds/flash/leds-lm3601x.c b/drivers/leds/flash/leds-lm3601x.c
index d0e1d4814042..3d1272748201 100644
--- a/drivers/leds/flash/leds-lm3601x.c
+++ b/drivers/leds/flash/leds-lm3601x.c
@@ -444,8 +444,6 @@ static int lm3601x_remove(struct i2c_client *client)
{
struct lm3601x_led *led = i2c_get_clientdata(client);
- mutex_destroy(&led->lock);
-
return regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
LM3601X_ENABLE_MASK,
LM3601X_MODE_STANDBY);
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: lm3601x: Don't use mutex after it was destroyed
2022-05-13 8:18 [PATCH] leds: lm3601x: Don't use mutex after it was destroyed Uwe Kleine-König
@ 2022-05-13 14:02 ` Pavel Machek
2022-05-13 14:36 ` Uwe Kleine-König
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Machek @ 2022-05-13 14:02 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Linus Walleij, Dan Murphy, linux-leds, kernel
[-- Attachment #1: Type: text/plain, Size: 789 bytes --]
Hi!
> The mutex might still be in use until the devm cleanup callback
> devm_led_classdev_flash_release() is called. This only happens some time
> after lm3601x_remove() completed.
I'm sure lots of "use after free" can be fixed by simply removing the
resource freeing... but lets fix this properly.
Best regards,
Pavel
> +++ b/drivers/leds/flash/leds-lm3601x.c
> @@ -444,8 +444,6 @@ static int lm3601x_remove(struct i2c_client *client)
> {
> struct lm3601x_led *led = i2c_get_clientdata(client);
>
> - mutex_destroy(&led->lock);
> -
> return regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> LM3601X_ENABLE_MASK,
> LM3601X_MODE_STANDBY);
> --
> 2.35.1
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: lm3601x: Don't use mutex after it was destroyed
2022-05-13 14:02 ` Pavel Machek
@ 2022-05-13 14:36 ` Uwe Kleine-König
2022-05-23 7:35 ` Uwe Kleine-König
0 siblings, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2022-05-13 14:36 UTC (permalink / raw)
To: Pavel Machek; +Cc: kernel, Linus Walleij, linux-leds
[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]
Hello Pavel,
[dropped Dan Murphy from Cc:, the email address doesn't work]
On Fri, May 13, 2022 at 04:02:55PM +0200, Pavel Machek wrote:
> Hi!
>
> > The mutex might still be in use until the devm cleanup callback
> > devm_led_classdev_flash_release() is called. This only happens some time
> > after lm3601x_remove() completed.
>
> I'm sure lots of "use after free" can be fixed by simply removing the
> resource freeing...
I agree in general. Mutexes are a bit special here and often are not
destroyed. mutex_destroy() is a no-op usually and with debugging enabled
only does
lock->magic = NULL;
which catches use-after-destroy. So IMHO just dropping the mutex_destroy
is fine.
> but lets fix this properly.
I don't understand that part. Does that mean you pick up my patch, or
that you create a better fix?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: lm3601x: Don't use mutex after it was destroyed
2022-05-13 14:36 ` Uwe Kleine-König
@ 2022-05-23 7:35 ` Uwe Kleine-König
2022-06-07 6:46 ` Uwe Kleine-König
0 siblings, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2022-05-23 7:35 UTC (permalink / raw)
To: Pavel Machek; +Cc: Linus Walleij, linux-leds, kernel
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
Hello Pavel,
On Fri, May 13, 2022 at 04:36:57PM +0200, Uwe Kleine-König wrote:
> On Fri, May 13, 2022 at 04:02:55PM +0200, Pavel Machek wrote:
> > > The mutex might still be in use until the devm cleanup callback
> > > devm_led_classdev_flash_release() is called. This only happens some time
> > > after lm3601x_remove() completed.
> >
> > I'm sure lots of "use after free" can be fixed by simply removing the
> > resource freeing...
>
> I agree in general. Mutexes are a bit special here and often are not
> destroyed. mutex_destroy() is a no-op usually and with debugging enabled
> only does
>
> lock->magic = NULL;
>
> which catches use-after-destroy. So IMHO just dropping the mutex_destroy
> is fine.
>
> > but lets fix this properly.
>
> I don't understand that part. Does that mean you pick up my patch, or
> that you create a better fix?
You didn't pick up this patch up to now and also I didn't see a better
fix.
So I wonder what is your plan/vision here. The obvious alternatives are
to call mutex_destroy only in a devm callback that is registered before
calling lm3601x_register_leds(), or don't used devm to register the LED.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: lm3601x: Don't use mutex after it was destroyed
2022-05-23 7:35 ` Uwe Kleine-König
@ 2022-06-07 6:46 ` Uwe Kleine-König
0 siblings, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2022-06-07 6:46 UTC (permalink / raw)
To: Pavel Machek; +Cc: Linus Walleij, kernel, linux-leds
[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]
Hello Pavel,
On Mon, May 23, 2022 at 09:35:10AM +0200, Uwe Kleine-König wrote:
> On Fri, May 13, 2022 at 04:36:57PM +0200, Uwe Kleine-König wrote:
> > On Fri, May 13, 2022 at 04:02:55PM +0200, Pavel Machek wrote:
> > > > The mutex might still be in use until the devm cleanup callback
> > > > devm_led_classdev_flash_release() is called. This only happens some time
> > > > after lm3601x_remove() completed.
> > >
> > > I'm sure lots of "use after free" can be fixed by simply removing the
> > > resource freeing...
> >
> > I agree in general. Mutexes are a bit special here and often are not
> > destroyed. mutex_destroy() is a no-op usually and with debugging enabled
> > only does
> >
> > lock->magic = NULL;
> >
> > which catches use-after-destroy. So IMHO just dropping the mutex_destroy
> > is fine.
> >
> > > but lets fix this properly.
> >
> > I don't understand that part. Does that mean you pick up my patch, or
> > that you create a better fix?
>
> You didn't pick up this patch up to now and also I didn't see a better
> fix.
>
> So I wonder what is your plan/vision here. The obvious alternatives are
> to call mutex_destroy only in a devm callback that is registered before
> calling lm3601x_register_leds(), or don't used devm to register the LED.
Any news on this? If you're waiting for a better fix from me, please
tell my your expectations. A devm variant of mutex_init would be an
option, but that feels like a very big hammer for a small nail. I still
consider my patch fine.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-07 6:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13 8:18 [PATCH] leds: lm3601x: Don't use mutex after it was destroyed Uwe Kleine-König
2022-05-13 14:02 ` Pavel Machek
2022-05-13 14:36 ` Uwe Kleine-König
2022-05-23 7:35 ` Uwe Kleine-König
2022-06-07 6:46 ` Uwe Kleine-König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.