Re: [PATCH v6 06/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger

[Rong Zhang <i@rong.moe>]

Hi Lee,

On Wed, 2026-09-02 at 18:15 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The statically allocated `omnia_hw_trigger` object is registered multiple times via `devm_led_trigger_register()` on probe, causing lock and list corruption if multiple driver instances are bound.

Real hardware doesn't trigger the pre-existing issue, and the series has
nothing to do with the issue. Therefore, fixing it is out of the scope of
the series.

Thanks,
Rong

> --
> 
> commit 836486b7bf6dc3bf19d25d5b547db0edd0d638af
> Author: Rong Zhang <i@rong.moe>
> 
> leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger
>     
> This commit implements the offloaded() callback for the omnia-mcu hardware
> control trigger, returning true to reflect its state. It also declares it as
> a hardware control trigger in the class device during registration.
> 
> > diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
> > index ed6a47bbb44f9..32d40d176d3f1 100644
> > --- a/drivers/leds/leds-turris-omnia.c
> > +++ b/drivers/leds/leds-turris-omnia.c
> [ ... ]
> > @@ -195,10 +195,16 @@ static void omnia_hwtrig_deactivate(struct led_classdev *cdev)
> >  			err);
> >  }
> >  
> > +static bool omnia_hwtrig_offloaded(struct led_classdev *cdev)
> > +{
> > +	return true;
> > +}
> > +
> >  static struct led_trigger omnia_hw_trigger = {
> >  	.name		= "omnia-mcu",
> >  	.activate	= omnia_hwtrig_activate,
> >  	.deactivate	= omnia_hwtrig_deactivate,
> > +	.offloaded	= omnia_hwtrig_offloaded,
> >  	.trigger_type	= &omnia_hw_trigger_type,
> >  };
> 
> [Severity: High]
> This is a pre-existing issue, but does this statically allocated singleton
> trigger cause list corruption if multiple driver instances are bound?
> 
> If a user manually binds a second device instance to the leds-turris-omnia
> driver via the sysfs bind interface (or multiple I2C devices probe),
> devm_led_trigger_register() in omnia_leds_probe() will be called again with
> this exact same statically allocated omnia_hw_trigger structure pointer.
> 
> In drivers/leds/led-triggers.c:led_trigger_register(), the core initializes
> the lock and list head before checking for duplicate registrations:
> 
>     spin_lock_init(&trig->leddev_list_lock);
>     INIT_LIST_HEAD(&trig->led_cdevs);
> 
> Wouldn't this blindly overwrite the active lock and list state of the
> trigger, destroying the lock state while another CPU is iterating and
> unlinking any already-registered LEDs, leading to a use-after-free?