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

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.
--

--- 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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5