[PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Hans de Goede]

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

Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Sebastian Reichel]

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

Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Hans de Goede]

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

Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Lee Jones]

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 [李琼斯]

Re: (subset) [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Lee Jones]

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 [李琼斯]

Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Hans de Goede]

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

Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Lee Jones]

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 [李琼斯]

Re: [PATCH] leds: led-class: Only Add LED to leds_list when it is fully ready

[Lee Jones]

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 [李琼斯]