tmakin/ack-tegra
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

leds: Fix LED_OFF brightness race

Browse Source 
[ Upstream commit 2c70953b6f535f7698ccbf22c1f5ba26cb6c2816 ]

While commit fa15d8c692 ("leds: Fix set_brightness_delayed() race")
successfully forces led_set_brightness() to be called with LED_OFF at
least once when switching from blinking to LED on state so that
hw-blinking can be disabled, another race remains. Indeed in
led_set_brightness(LED_OFF) followed by led_set_brightness(any)
scenario the following CPU scheduling can happen:

    CPU0                                     CPU1
    ----                                     ----
 set_brightness_delayed() {
   test_and_clear_bit(BRIGHTNESS_OFF)
                                         led_set_brightness(LED_OFF) {
                                           set_bit(BRIGHTNESS_OFF)
					   queue_work()
                                         }
                                         led_set_brightness(any) {
                                           set_bit(BRIGHTNESS)
					   queue_work() //already queued
                                         }
   test_and_clear_bit(BRIGHTNESS)
     /* LED set with brightness any */
 }

 /* From previous CPU1 queue_work() */
 set_brightness_delayed() {
   test_and_clear_bit(BRIGHTNESS_OFF)
     /* LED turned off */
   test_and_clear_bit(BRIGHTNESS)
     /* Clear from previous run, LED remains off */

In that case the led_set_brightness(LED_OFF)/led_set_brightness(any)
sequence will be effectively executed in reverse order and LED will
remain off.

With the introduction of commit 32360bf6a5d4 ("leds: Introduce ordered
workqueue for LEDs events instead of system_wq") the race is easier to
trigger as sysfs brightness configuration does not wait for
set_brightness_delayed() work to finish (flush_work() removal).

Use delayed_set_value to optionnally re-configure brightness after a
LED_OFF. That way a LED state could be configured more that once but
final state will always be as expected. Ensure that delayed_set_value
modification is seen before set_bit() using smp_mb__before_atomic().

Fixes: fa15d8c692 ("leds: Fix set_brightness_delayed() race")
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/19c81177059dab7b656c42063958011a8e4d1a66.1740050412.git.repk@triplefau.lt
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Remi Pommarel
2025-02-20 12:23:17 +01:00
committed by Greg Kroah-Hartman
parent a1fab9e649
commit c2ddf2f576
1 changed files with 18 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
drivers/leds/led-core.c
View File

@@ -159,8 +159,19 @@ static void set_brightness_delayed(struct work_struct *ws)
* before this work item runs once. To make sure this works properly
* handle LED_SET_BRIGHTNESS_OFF first.
*/
if (test_and_clear_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags))
if (test_and_clear_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags)) {
set_brightness_delayed_set_brightness(led_cdev, LED_OFF);
/*
* The consecutives led_set_brightness(LED_OFF),
* led_set_brightness(LED_FULL) could have been executed out of
* order (LED_FULL first), if the work_flags has been set
* between LED_SET_BRIGHTNESS_OFF and LED_SET_BRIGHTNESS of this
* work. To avoid ending with the LED turned off, turn the LED
* on again.
*/
if (led_cdev->delayed_set_value != LED_OFF)
set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
}
if (test_and_clear_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags))
set_brightness_delayed_set_brightness(led_cdev, led_cdev->delayed_set_value);
@@ -331,10 +342,13 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev, unsigned int value)
* change is done immediately afterwards (before the work runs),
* it uses a separate work_flag.
*/
if (value) {
led_cdev->delayed_set_value = value;
led_cdev->delayed_set_value = value;
/* Ensure delayed_set_value is seen before work_flags modification */
smp_mb__before_atomic();
if (value)
set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
} else {
else {
clear_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
clear_bit(LED_SET_BLINK, &led_cdev->work_flags);
set_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags);