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

Hi all,

On Fri, 2026-08-21 at 17:27 +0000, [email protected] 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 if multiple driver instances are bound, causing lock and list corruption.

The driver is very platform-specific and the relevant platforms never
have more than one such device.

Therefore, fixing it is far beyond the scope of the series.

Thanks,
Rong

> --
> 
> --- Patch [5]: [PATCH v5 05/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger ---
> commit f8e07428579f0c7761609a5c572f8b597a213a09
> Author: Rong Zhang <[email protected]>
> 
> leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger
> 
> "omnia-mcu" is a private hardware control trigger which always stays in
> hardware control mode. Implement offloaded() callback with its return
> value to be always true to reflect this.
> 
> > 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 isn't a bug introduced by this patch, but does using a statically
> allocated global trigger structure risk lock and list corruption?
> 
> If multiple instances of the driver are bound (for example, via I2C
> `new_device` sysfs bindings), `omnia_leds_probe()` will call
> `devm_led_trigger_register()` multiple times using this shared
> `omnia_hw_trigger` object:
> 
> omnia_leds_probe()
>     devm_led_trigger_register(dev, &omnia_hw_trigger)
> 
> Inside the core `led_trigger_register()` function, `spin_lock_init()` and
> `INIT_LIST_HEAD()` are called unconditionally before checking if the trigger
> name already exists. As a result, the second probe call will silently
> re-initialize the active spinlock and reset the list head of the first
> instance's trigger, which could lead to a kernel panic.