Re: [PATCH RFC v3 05/11] leds: turris-omnia: trigger: Implement offloaded() and declare hw_control_trigger

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

Pre-existing issues:
- [High] Calling `devm_led_trigger_register()` on a statically allocated global `struct led_trigger omnia_hw_trigger` causes list corruption and potential kernel panics if multiple driver instances are probed.
--

--- Patch [5]: [PATCH RFC v3 05/11] leds: turris-omnia: trigger: Implement offloaded() and declare hw_control_trigger ---
commit 2d28a7cfbe3bd11a356a1c04d42e4a480011bb29
Author: Rong Zhang <[email protected]>

leds: turris-omnia: trigger: Implement offloaded() 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

[ ... ]

>  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 calling devm_led_trigger_register()
on this statically allocated global structure cause list corruption if
multiple driver instances are probed?

If a privileged user instantiates a second instance of the driver on the
same I2C bus via sysfs:

drivers/leds/leds-turris-omnia.c:omnia_leds_probe() {
    ...
    ret = devm_led_trigger_register(dev, &omnia_hw_trigger);
    ...
}

The led_trigger_register() core function unconditionally executes
INIT_LIST_HEAD() and spin_lock_init() before checking if the trigger is
already registered:

drivers/leds/led-triggers.c:led_trigger_register() {
    ...
    spin_lock_init(&trig->leddev_list_lock);
    INIT_LIST_HEAD(&trig->led_cdevs);
    ...
}

Could this destroy the lock and the list of attached LEDs for the active
trigger, creating dangling pointers that lead to use-after-free or panics
during teardown before the duplicate name check returns -EEXIST?

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