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
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
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
linux-stable-mirror@lists.linaro.org
-
Hans de Goede -
Sebastian Reichel