* [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
@ 2025-12-11 16:37 Hans de Goede
2025-12-12 6:49 ` Sebastian Reichel
2026-01-08 12:14 ` (subset) " Lee Jones
0 siblings, 2 replies; 8+ messages in thread
From: Hans de Goede @ 2025-12-11 16:37 UTC (permalink / raw)
To: Lee Jones, Pavel Machek
Cc: Hans de Goede, linux-leds, Sebastian Reichel, stable
Before this change the LED was added to leds_list before led_init_core()
gets called adding it the list before led_classdev.set_brightness_work gets
initialized.
This leaves a window where led_trigger_register() of a LED's default
trigger will call led_trigger_set() which calls led_set_brightness()
which in turn will end up queueing the *uninitialized*
led_classdev.set_brightness_work.
This race gets hit by the lenovo-thinkpad-t14s EC driver which registers
2 LEDs with a default trigger provided by snd_ctl_led.ko in quick
succession. The first led_classdev_register() causes an async modprobe of
snd_ctl_led to run and that async modprobe manages to exactly hit
the window where the second LED is on the leds_list without led_init_core()
being called for it, resulting in:
------------[ cut here ]------------
WARNING: CPU: 11 PID: 5608 at kernel/workqueue.c:4234 __flush_work+0x344/0x390
Hardware name: LENOVO 21N2S01F0B/21N2S01F0B, BIOS N42ET93W (2.23 ) 09/01/2025
...
Call trace:
__flush_work+0x344/0x390 (P)
flush_work+0x2c/0x50
led_trigger_set+0x1c8/0x340
led_trigger_register+0x17c/0x1c0
led_trigger_register_simple+0x84/0xe8
snd_ctl_led_init+0x40/0xf88 [snd_ctl_led]
do_one_initcall+0x5c/0x318
do_init_module+0x9c/0x2b8
load_module+0x7e0/0x998
Close the race window by moving the adding of the LED to leds_list to
after the led_init_core() call.
Cc: Sebastian Reichel <sre@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
Note no Fixes tag as this problem has been around for a long long time,
so I could not really find a good commit for the Fixes tag.
---
drivers/leds/led-class.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index f3faf37f9a08..6b9fa060c3a1 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -560,11 +560,6 @@ int led_classdev_register_ext(struct device *parent,
#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
led_cdev->brightness_hw_changed = -1;
#endif
- /* add to the list of leds */
- down_write(&leds_list_lock);
- list_add_tail(&led_cdev->node, &leds_list);
- up_write(&leds_list_lock);
-
if (!led_cdev->max_brightness)
led_cdev->max_brightness = LED_FULL;
@@ -574,6 +569,11 @@ int led_classdev_register_ext(struct device *parent,
led_init_core(led_cdev);
+ /* add to the list of leds */
+ down_write(&leds_list_lock);
+ list_add_tail(&led_cdev->node, &leds_list);
+ up_write(&leds_list_lock);
+
#ifdef CONFIG_LEDS_TRIGGERS
led_trigger_set_default(led_cdev);
#endif
--
2.52.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
2025-12-11 16:37 [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready Hans de Goede
@ 2025-12-12 6:49 ` Sebastian Reichel
2025-12-12 9:05 ` Hans de Goede
2026-01-08 12:14 ` (subset) " Lee Jones
1 sibling, 1 reply; 8+ messages in thread
From: Sebastian Reichel @ 2025-12-12 6:49 UTC (permalink / raw)
To: Hans de Goede; +Cc: Lee Jones, Pavel Machek, linux-leds, stable
[-- Attachment #1: Type: text/plain, Size: 3351 bytes --]
Hi,
On Thu, Dec 11, 2025 at 05:37:27PM +0100, Hans de Goede wrote:
> Before this change the LED was added to leds_list before led_init_core()
> gets called adding it the list before led_classdev.set_brightness_work gets
> initialized.
>
> This leaves a window where led_trigger_register() of a LED's default
> trigger will call led_trigger_set() which calls led_set_brightness()
> which in turn will end up queueing the *uninitialized*
> led_classdev.set_brightness_work.
>
> This race gets hit by the lenovo-thinkpad-t14s EC driver which registers
> 2 LEDs with a default trigger provided by snd_ctl_led.ko in quick
> succession. The first led_classdev_register() causes an async modprobe of
> snd_ctl_led to run and that async modprobe manages to exactly hit
> the window where the second LED is on the leds_list without led_init_core()
> being called for it, resulting in:
>
> ------------[ cut here ]------------
> WARNING: CPU: 11 PID: 5608 at kernel/workqueue.c:4234 __flush_work+0x344/0x390
> Hardware name: LENOVO 21N2S01F0B/21N2S01F0B, BIOS N42ET93W (2.23 ) 09/01/2025
> ...
> Call trace:
> __flush_work+0x344/0x390 (P)
> flush_work+0x2c/0x50
> led_trigger_set+0x1c8/0x340
> led_trigger_register+0x17c/0x1c0
> led_trigger_register_simple+0x84/0xe8
> snd_ctl_led_init+0x40/0xf88 [snd_ctl_led]
> do_one_initcall+0x5c/0x318
> do_init_module+0x9c/0x2b8
> load_module+0x7e0/0x998
>
> Close the race window by moving the adding of the LED to leds_list to
> after the led_init_core() call.
>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
heh, I've never hit this. But I guess that is not too surprising
considering it is a race condition. The change looks good to me:
Reviewed-by: Sebastian Reichel <sre@kernel.org>
> Note no Fixes tag as this problem has been around for a long long time,
> so I could not really find a good commit for the Fixes tag.
My suggestion would be:
Fixes: d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink")
It introduces the set_brightness_work with the INIT_WORK at the
wrong position (after the list addition).
Greetings,
-- Sebastian
> drivers/leds/led-class.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index f3faf37f9a08..6b9fa060c3a1 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -560,11 +560,6 @@ int led_classdev_register_ext(struct device *parent,
> #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
> led_cdev->brightness_hw_changed = -1;
> #endif
> - /* add to the list of leds */
> - down_write(&leds_list_lock);
> - list_add_tail(&led_cdev->node, &leds_list);
> - up_write(&leds_list_lock);
> -
> if (!led_cdev->max_brightness)
> led_cdev->max_brightness = LED_FULL;
>
> @@ -574,6 +569,11 @@ int led_classdev_register_ext(struct device *parent,
>
> led_init_core(led_cdev);
>
> + /* add to the list of leds */
> + down_write(&leds_list_lock);
> + list_add_tail(&led_cdev->node, &leds_list);
> + up_write(&leds_list_lock);
> +
> #ifdef CONFIG_LEDS_TRIGGERS
> led_trigger_set_default(led_cdev);
> #endif
> --
> 2.52.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
2025-12-12 6:49 ` Sebastian Reichel
@ 2025-12-12 9:05 ` Hans de Goede
2026-01-08 12:11 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2025-12-12 9:05 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: Lee Jones, Pavel Machek, linux-leds, stable
Hi,
On 12-Dec-25 07:49, Sebastian Reichel wrote:
> Hi,
>
> On Thu, Dec 11, 2025 at 05:37:27PM +0100, Hans de Goede wrote:
>> Before this change the LED was added to leds_list before led_init_core()
>> gets called adding it the list before led_classdev.set_brightness_work gets
>> initialized.
>>
>> This leaves a window where led_trigger_register() of a LED's default
>> trigger will call led_trigger_set() which calls led_set_brightness()
>> which in turn will end up queueing the *uninitialized*
>> led_classdev.set_brightness_work.
>>
>> This race gets hit by the lenovo-thinkpad-t14s EC driver which registers
>> 2 LEDs with a default trigger provided by snd_ctl_led.ko in quick
>> succession. The first led_classdev_register() causes an async modprobe of
>> snd_ctl_led to run and that async modprobe manages to exactly hit
>> the window where the second LED is on the leds_list without led_init_core()
>> being called for it, resulting in:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 11 PID: 5608 at kernel/workqueue.c:4234 __flush_work+0x344/0x390
>> Hardware name: LENOVO 21N2S01F0B/21N2S01F0B, BIOS N42ET93W (2.23 ) 09/01/2025
>> ...
>> Call trace:
>> __flush_work+0x344/0x390 (P)
>> flush_work+0x2c/0x50
>> led_trigger_set+0x1c8/0x340
>> led_trigger_register+0x17c/0x1c0
>> led_trigger_register_simple+0x84/0xe8
>> snd_ctl_led_init+0x40/0xf88 [snd_ctl_led]
>> do_one_initcall+0x5c/0x318
>> do_init_module+0x9c/0x2b8
>> load_module+0x7e0/0x998
>>
>> Close the race window by moving the adding of the LED to leds_list to
>> after the led_init_core() call.
>>
>> Cc: Sebastian Reichel <sre@kernel.org>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>> ---
>
> heh, I've never hit this. But I guess that is not too surprising
> considering it is a race condition. The change looks good to me:
>
> Reviewed-by: Sebastian Reichel <sre@kernel.org>
Thx.
>> Note no Fixes tag as this problem has been around for a long long time,
>> so I could not really find a good commit for the Fixes tag.
>
> My suggestion would be:
>
> Fixes: d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink")
Ack, that works for me.
Lee can you add this Fixes tag while merging ?
Also (in case it is not obvious) this is a bugfix so it would be
nice if this could go in a fixes pull-request for 6.19.
Regards,
Hans
> It introduces the set_brightness_work with the INIT_WORK at the
> wrong position (after the list addition).
>
> Greetings,
>
> -- Sebastian
>
>> drivers/leds/led-class.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index f3faf37f9a08..6b9fa060c3a1 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -560,11 +560,6 @@ int led_classdev_register_ext(struct device *parent,
>> #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>> led_cdev->brightness_hw_changed = -1;
>> #endif
>> - /* add to the list of leds */
>> - down_write(&leds_list_lock);
>> - list_add_tail(&led_cdev->node, &leds_list);
>> - up_write(&leds_list_lock);
>> -
>> if (!led_cdev->max_brightness)
>> led_cdev->max_brightness = LED_FULL;
>>
>> @@ -574,6 +569,11 @@ int led_classdev_register_ext(struct device *parent,
>>
>> led_init_core(led_cdev);
>>
>> + /* add to the list of leds */
>> + down_write(&leds_list_lock);
>> + list_add_tail(&led_cdev->node, &leds_list);
>> + up_write(&leds_list_lock);
>> +
>> #ifdef CONFIG_LEDS_TRIGGERS
>> led_trigger_set_default(led_cdev);
>> #endif
>> --
>> 2.52.0
>>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
2025-12-12 9:05 ` Hans de Goede
@ 2026-01-08 12:11 ` Lee Jones
2026-01-08 12:17 ` Hans de Goede
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2026-01-08 12:11 UTC (permalink / raw)
To: Hans de Goede; +Cc: Sebastian Reichel, Pavel Machek, linux-leds, stable
On Fri, 12 Dec 2025, Hans de Goede wrote:
> Hi,
>
> On 12-Dec-25 07:49, Sebastian Reichel wrote:
> > Hi,
> >
> > On Thu, Dec 11, 2025 at 05:37:27PM +0100, Hans de Goede wrote:
> >> Before this change the LED was added to leds_list before led_init_core()
> >> gets called adding it the list before led_classdev.set_brightness_work gets
> >> initialized.
> >>
> >> This leaves a window where led_trigger_register() of a LED's default
> >> trigger will call led_trigger_set() which calls led_set_brightness()
> >> which in turn will end up queueing the *uninitialized*
> >> led_classdev.set_brightness_work.
> >>
> >> This race gets hit by the lenovo-thinkpad-t14s EC driver which registers
> >> 2 LEDs with a default trigger provided by snd_ctl_led.ko in quick
> >> succession. The first led_classdev_register() causes an async modprobe of
> >> snd_ctl_led to run and that async modprobe manages to exactly hit
> >> the window where the second LED is on the leds_list without led_init_core()
> >> being called for it, resulting in:
> >>
> >> ------------[ cut here ]------------
> >> WARNING: CPU: 11 PID: 5608 at kernel/workqueue.c:4234 __flush_work+0x344/0x390
> >> Hardware name: LENOVO 21N2S01F0B/21N2S01F0B, BIOS N42ET93W (2.23 ) 09/01/2025
> >> ...
> >> Call trace:
> >> __flush_work+0x344/0x390 (P)
> >> flush_work+0x2c/0x50
> >> led_trigger_set+0x1c8/0x340
> >> led_trigger_register+0x17c/0x1c0
> >> led_trigger_register_simple+0x84/0xe8
> >> snd_ctl_led_init+0x40/0xf88 [snd_ctl_led]
> >> do_one_initcall+0x5c/0x318
> >> do_init_module+0x9c/0x2b8
> >> load_module+0x7e0/0x998
> >>
> >> Close the race window by moving the adding of the LED to leds_list to
> >> after the led_init_core() call.
> >>
> >> Cc: Sebastian Reichel <sre@kernel.org>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> >> ---
> >
> > heh, I've never hit this. But I guess that is not too surprising
> > considering it is a race condition. The change looks good to me:
> >
> > Reviewed-by: Sebastian Reichel <sre@kernel.org>
>
> Thx.
>
> >> Note no Fixes tag as this problem has been around for a long long time,
> >> so I could not really find a good commit for the Fixes tag.
> >
> > My suggestion would be:
> >
> > Fixes: d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink")
>
> Ack, that works for me.
>
> Lee can you add this Fixes tag while merging ?
>
> Also (in case it is not obvious) this is a bugfix so it would be
> nice if this could go in a fixes pull-request for 6.19.
Yes, I can add the Fixes: tag and no, I have no plans to send this for
-fixes. As you rightly mentioned, this issue has been around for a long
time already. I tend to only send -fixes pull-requests for things that
broke in -rc1 of the same release.
That said, however, if I end up sending one for something else, I will
try to remember to add this one to it as well.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
2026-01-08 12:11 ` Lee Jones
@ 2026-01-08 12:17 ` Hans de Goede
2026-01-09 9:35 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2026-01-08 12:17 UTC (permalink / raw)
To: Lee Jones; +Cc: Sebastian Reichel, Pavel Machek, linux-leds, stable
Hi Lee,
On 8-Jan-26 13:11, Lee Jones wrote:
> On Fri, 12 Dec 2025, Hans de Goede wrote:
>
>> Hi,
>>
>> On 12-Dec-25 07:49, Sebastian Reichel wrote:
>>> Hi,
>>>
>>> On Thu, Dec 11, 2025 at 05:37:27PM +0100, Hans de Goede wrote:
>>>> Before this change the LED was added to leds_list before led_init_core()
>>>> gets called adding it the list before led_classdev.set_brightness_work gets
>>>> initialized.
>>>>
>>>> This leaves a window where led_trigger_register() of a LED's default
>>>> trigger will call led_trigger_set() which calls led_set_brightness()
>>>> which in turn will end up queueing the *uninitialized*
>>>> led_classdev.set_brightness_work.
>>>>
>>>> This race gets hit by the lenovo-thinkpad-t14s EC driver which registers
>>>> 2 LEDs with a default trigger provided by snd_ctl_led.ko in quick
>>>> succession. The first led_classdev_register() causes an async modprobe of
>>>> snd_ctl_led to run and that async modprobe manages to exactly hit
>>>> the window where the second LED is on the leds_list without led_init_core()
>>>> being called for it, resulting in:
>>>>
>>>> ------------[ cut here ]------------
>>>> WARNING: CPU: 11 PID: 5608 at kernel/workqueue.c:4234 __flush_work+0x344/0x390
>>>> Hardware name: LENOVO 21N2S01F0B/21N2S01F0B, BIOS N42ET93W (2.23 ) 09/01/2025
>>>> ...
>>>> Call trace:
>>>> __flush_work+0x344/0x390 (P)
>>>> flush_work+0x2c/0x50
>>>> led_trigger_set+0x1c8/0x340
>>>> led_trigger_register+0x17c/0x1c0
>>>> led_trigger_register_simple+0x84/0xe8
>>>> snd_ctl_led_init+0x40/0xf88 [snd_ctl_led]
>>>> do_one_initcall+0x5c/0x318
>>>> do_init_module+0x9c/0x2b8
>>>> load_module+0x7e0/0x998
>>>>
>>>> Close the race window by moving the adding of the LED to leds_list to
>>>> after the led_init_core() call.
>>>>
>>>> Cc: Sebastian Reichel <sre@kernel.org>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>>>> ---
>>>
>>> heh, I've never hit this. But I guess that is not too surprising
>>> considering it is a race condition. The change looks good to me:
>>>
>>> Reviewed-by: Sebastian Reichel <sre@kernel.org>
>>
>> Thx.
>>
>>>> Note no Fixes tag as this problem has been around for a long long time,
>>>> so I could not really find a good commit for the Fixes tag.
>>>
>>> My suggestion would be:
>>>
>>> Fixes: d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink")
>>
>> Ack, that works for me.
>>
>> Lee can you add this Fixes tag while merging ?
>>
>> Also (in case it is not obvious) this is a bugfix so it would be
>> nice if this could go in a fixes pull-request for 6.19.
>
> Yes, I can add the Fixes: tag and no, I have no plans to send this for
> -fixes. As you rightly mentioned, this issue has been around for a long
> time already. I tend to only send -fixes pull-requests for things that
> broke in -rc1 of the same release.
Even though this has been around for a long time, it would be good
to get this in as a fix for 6.19-rc# because as described in the commit
msg the lenovo-thinkpad-t14s embedded-controller driver, which is new in
6.19-rc1 manages to reliably trigger the race (for me, with a Fedora
kernel distconfig).
I was surprised I could hit the race pretty reliably, but it did make
debugging this easier.
Hitting the race also leads to a crash due to a NULL ptr deref after
the WARN(). I did not elaborate on this in the commit msg, because
the WARN() is the first sign of trying to use uninitialized mem.
IMHO having a reproducable race which causes a crash is
a good reason to submit this as a fix for 6.19 .
Regards,
Hans
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
2026-01-08 12:17 ` Hans de Goede
@ 2026-01-09 9:35 ` Lee Jones
2026-01-22 11:49 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2026-01-09 9:35 UTC (permalink / raw)
To: Hans de Goede; +Cc: Sebastian Reichel, Pavel Machek, linux-leds, stable
On Thu, 08 Jan 2026, Hans de Goede wrote:
> Hi Lee,
>
> On 8-Jan-26 13:11, Lee Jones wrote:
> > On Fri, 12 Dec 2025, Hans de Goede wrote:
> >
> >> Hi,
> >>
> >> On 12-Dec-25 07:49, Sebastian Reichel wrote:
> >>> Hi,
> >>>
> >>> On Thu, Dec 11, 2025 at 05:37:27PM +0100, Hans de Goede wrote:
> >>>> Before this change the LED was added to leds_list before led_init_core()
> >>>> gets called adding it the list before led_classdev.set_brightness_work gets
> >>>> initialized.
> >>>>
> >>>> This leaves a window where led_trigger_register() of a LED's default
> >>>> trigger will call led_trigger_set() which calls led_set_brightness()
> >>>> which in turn will end up queueing the *uninitialized*
> >>>> led_classdev.set_brightness_work.
> >>>>
> >>>> This race gets hit by the lenovo-thinkpad-t14s EC driver which registers
> >>>> 2 LEDs with a default trigger provided by snd_ctl_led.ko in quick
> >>>> succession. The first led_classdev_register() causes an async modprobe of
> >>>> snd_ctl_led to run and that async modprobe manages to exactly hit
> >>>> the window where the second LED is on the leds_list without led_init_core()
> >>>> being called for it, resulting in:
> >>>>
> >>>> ------------[ cut here ]------------
> >>>> WARNING: CPU: 11 PID: 5608 at kernel/workqueue.c:4234 __flush_work+0x344/0x390
> >>>> Hardware name: LENOVO 21N2S01F0B/21N2S01F0B, BIOS N42ET93W (2.23 ) 09/01/2025
> >>>> ...
> >>>> Call trace:
> >>>> __flush_work+0x344/0x390 (P)
> >>>> flush_work+0x2c/0x50
> >>>> led_trigger_set+0x1c8/0x340
> >>>> led_trigger_register+0x17c/0x1c0
> >>>> led_trigger_register_simple+0x84/0xe8
> >>>> snd_ctl_led_init+0x40/0xf88 [snd_ctl_led]
> >>>> do_one_initcall+0x5c/0x318
> >>>> do_init_module+0x9c/0x2b8
> >>>> load_module+0x7e0/0x998
> >>>>
> >>>> Close the race window by moving the adding of the LED to leds_list to
> >>>> after the led_init_core() call.
> >>>>
> >>>> Cc: Sebastian Reichel <sre@kernel.org>
> >>>> Cc: stable@vger.kernel.org
> >>>> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> >>>> ---
> >>>
> >>> heh, I've never hit this. But I guess that is not too surprising
> >>> considering it is a race condition. The change looks good to me:
> >>>
> >>> Reviewed-by: Sebastian Reichel <sre@kernel.org>
> >>
> >> Thx.
> >>
> >>>> Note no Fixes tag as this problem has been around for a long long time,
> >>>> so I could not really find a good commit for the Fixes tag.
> >>>
> >>> My suggestion would be:
> >>>
> >>> Fixes: d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink")
> >>
> >> Ack, that works for me.
> >>
> >> Lee can you add this Fixes tag while merging ?
> >>
> >> Also (in case it is not obvious) this is a bugfix so it would be
> >> nice if this could go in a fixes pull-request for 6.19.
> >
> > Yes, I can add the Fixes: tag and no, I have no plans to send this for
> > -fixes. As you rightly mentioned, this issue has been around for a long
> > time already. I tend to only send -fixes pull-requests for things that
> > broke in -rc1 of the same release.
>
> Even though this has been around for a long time, it would be good
> to get this in as a fix for 6.19-rc# because as described in the commit
> msg the lenovo-thinkpad-t14s embedded-controller driver, which is new in
> 6.19-rc1 manages to reliably trigger the race (for me, with a Fedora
> kernel distconfig).
>
> I was surprised I could hit the race pretty reliably, but it did make
> debugging this easier.
>
> Hitting the race also leads to a crash due to a NULL ptr deref after
> the WARN(). I did not elaborate on this in the commit msg, because
> the WARN() is the first sign of trying to use uninitialized mem.
>
> IMHO having a reproducable race which causes a crash is
> a good reason to submit this as a fix for 6.19 .
Noted. Leave it with me.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
2026-01-09 9:35 ` Lee Jones
@ 2026-01-22 11:49 ` Lee Jones
0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2026-01-22 11:49 UTC (permalink / raw)
To: Hans de Goede; +Cc: Sebastian Reichel, Pavel Machek, linux-leds, stable
On Fri, 09 Jan 2026, Lee Jones wrote:
> On Thu, 08 Jan 2026, Hans de Goede wrote:
>
> > Hi Lee,
> >
> > On 8-Jan-26 13:11, Lee Jones wrote:
> > > On Fri, 12 Dec 2025, Hans de Goede wrote:
> > >
> > >> Hi,
> > >>
> > >> On 12-Dec-25 07:49, Sebastian Reichel wrote:
> > >>> Hi,
> > >>>
> > >>> On Thu, Dec 11, 2025 at 05:37:27PM +0100, Hans de Goede wrote:
> > >>>> Before this change the LED was added to leds_list before led_init_core()
> > >>>> gets called adding it the list before led_classdev.set_brightness_work gets
> > >>>> initialized.
> > >>>>
> > >>>> This leaves a window where led_trigger_register() of a LED's default
> > >>>> trigger will call led_trigger_set() which calls led_set_brightness()
> > >>>> which in turn will end up queueing the *uninitialized*
> > >>>> led_classdev.set_brightness_work.
> > >>>>
> > >>>> This race gets hit by the lenovo-thinkpad-t14s EC driver which registers
> > >>>> 2 LEDs with a default trigger provided by snd_ctl_led.ko in quick
> > >>>> succession. The first led_classdev_register() causes an async modprobe of
> > >>>> snd_ctl_led to run and that async modprobe manages to exactly hit
> > >>>> the window where the second LED is on the leds_list without led_init_core()
> > >>>> being called for it, resulting in:
> > >>>>
> > >>>> ------------[ cut here ]------------
> > >>>> WARNING: CPU: 11 PID: 5608 at kernel/workqueue.c:4234 __flush_work+0x344/0x390
> > >>>> Hardware name: LENOVO 21N2S01F0B/21N2S01F0B, BIOS N42ET93W (2.23 ) 09/01/2025
> > >>>> ...
> > >>>> Call trace:
> > >>>> __flush_work+0x344/0x390 (P)
> > >>>> flush_work+0x2c/0x50
> > >>>> led_trigger_set+0x1c8/0x340
> > >>>> led_trigger_register+0x17c/0x1c0
> > >>>> led_trigger_register_simple+0x84/0xe8
> > >>>> snd_ctl_led_init+0x40/0xf88 [snd_ctl_led]
> > >>>> do_one_initcall+0x5c/0x318
> > >>>> do_init_module+0x9c/0x2b8
> > >>>> load_module+0x7e0/0x998
> > >>>>
> > >>>> Close the race window by moving the adding of the LED to leds_list to
> > >>>> after the led_init_core() call.
> > >>>>
> > >>>> Cc: Sebastian Reichel <sre@kernel.org>
> > >>>> Cc: stable@vger.kernel.org
> > >>>> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> > >>>> ---
> > >>>
> > >>> heh, I've never hit this. But I guess that is not too surprising
> > >>> considering it is a race condition. The change looks good to me:
> > >>>
> > >>> Reviewed-by: Sebastian Reichel <sre@kernel.org>
> > >>
> > >> Thx.
> > >>
> > >>>> Note no Fixes tag as this problem has been around for a long long time,
> > >>>> so I could not really find a good commit for the Fixes tag.
> > >>>
> > >>> My suggestion would be:
> > >>>
> > >>> Fixes: d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink")
> > >>
> > >> Ack, that works for me.
> > >>
> > >> Lee can you add this Fixes tag while merging ?
> > >>
> > >> Also (in case it is not obvious) this is a bugfix so it would be
> > >> nice if this could go in a fixes pull-request for 6.19.
> > >
> > > Yes, I can add the Fixes: tag and no, I have no plans to send this for
> > > -fixes. As you rightly mentioned, this issue has been around for a long
> > > time already. I tend to only send -fixes pull-requests for things that
> > > broke in -rc1 of the same release.
> >
> > Even though this has been around for a long time, it would be good
> > to get this in as a fix for 6.19-rc# because as described in the commit
> > msg the lenovo-thinkpad-t14s embedded-controller driver, which is new in
> > 6.19-rc1 manages to reliably trigger the race (for me, with a Fedora
> > kernel distconfig).
> >
> > I was surprised I could hit the race pretty reliably, but it did make
> > debugging this easier.
> >
> > Hitting the race also leads to a crash due to a NULL ptr deref after
> > the WARN(). I did not elaborate on this in the commit msg, because
> > the WARN() is the first sign of trying to use uninitialized mem.
> >
> > IMHO having a reproducable race which causes a crash is
> > a good reason to submit this as a fix for 6.19 .
>
> Noted. Leave it with me.
https://lore.kernel.org/all/20260122114749.GE3831112@google.com/T/#u
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: (subset) [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready
2025-12-11 16:37 [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready Hans de Goede
2025-12-12 6:49 ` Sebastian Reichel
@ 2026-01-08 12:14 ` Lee Jones
1 sibling, 0 replies; 8+ messages in thread
From: Lee Jones @ 2026-01-08 12:14 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Hans de Goede
Cc: linux-leds, Sebastian Reichel, stable
On Thu, 11 Dec 2025 17:37:27 +0100, Hans de Goede wrote:
> Before this change the LED was added to leds_list before led_init_core()
> gets called adding it the list before led_classdev.set_brightness_work gets
> initialized.
>
> This leaves a window where led_trigger_register() of a LED's default
> trigger will call led_trigger_set() which calls led_set_brightness()
> which in turn will end up queueing the *uninitialized*
> led_classdev.set_brightness_work.
>
> [...]
Applied, thanks!
[1/1] leds: led-class: Only Add LED to leds_list when it is fully ready
commit: bbe55e436524c99549e05d61371fac0806718a86
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-01-22 11:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-11 16:37 [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready Hans de Goede
2025-12-12 6:49 ` Sebastian Reichel
2025-12-12 9:05 ` Hans de Goede
2026-01-08 12:11 ` Lee Jones
2026-01-08 12:17 ` Hans de Goede
2026-01-09 9:35 ` Lee Jones
2026-01-22 11:49 ` Lee Jones
2026-01-08 12:14 ` (subset) " Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox