Re: [PATCH v4 08/12] leds: trigger: Do not attach trigger to a removing LED

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Non-atomic update of `led_cdev->flags` causes a data race that can drop the `LED_UNREGISTERING` flag, undermining the use-after-free fix.
--

--- Patch [8]: [PATCH v4 08/12] leds: trigger: Do not attach trigger to a removing LED ---
commit dd467951eb46d9ac1296845c072ca12344efe3da
Author: Rong Zhang <i@rong.moe>

leds: trigger: Do not attach trigger to a removing LED

This commit attempts to fix a race condition where a trigger could attach
to an LED during removal. It sets the LED_UNREGISTERING flag earlier in
led_classdev_unregister() and checks it before attaching a trigger.

> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 2460fcf0c4690..4158337896bc7 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -631,6 +631,8 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
>  	if (IS_ERR_OR_NULL(led_cdev->dev))
>  		return;
>  
> +	led_cdev->flags |= LED_UNREGISTERING;

[Severity: High]
This is a pre-existing issue, but does relying on a non-atomic update to 
flags here fail to prevent the race condition?

Because led_cdev->flags is updated non-atomically via a read-modify-write
operation, concurrent modifications to flags can overwrite and drop the
LED_UNREGISTERING bit.

For example, if a user-space thread writes to the LED's trigger sysfs
attribute to remove a trigger concurrently, __led_trigger_set() modifies
the flags locklessly:

drivers/leds/led-triggers.c:__led_trigger_set() {
    ...
    led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
    ...
}

If this overwrites the LED_UNREGISTERING flag, the new early-exit check
introduced in this patch:

drivers/leds/led-triggers.c:__led_trigger_set() {
    if ((led_cdev->flags & LED_UNREGISTERING) && trig)
        return -ENODEV;
...

will falsely pass. This still allows a trigger to attach to the dying LED,
potentially resulting in a use-after-free.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-leds-trigger-hw-changed-v4-0-f97e2ca976fe@rong.moe?part=8