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

Hi all,

On Sun, 2026-08-02 at 17:31 +0000, [email protected] wrote:
> 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 <[email protected]>
> 
> 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.
> 
> [ ... ]

I will wrap the RMW with trigger_lock in v5.

Thanks,
Rong