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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Registering a globally shared static trigger via `devm_led_trigger_register()` corrupts internal trigger lists when multiple instances probe.
--

--- Patch [5]: [PATCH v4 05/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger ---
commit 258c3687f19ed5315a5258f70ed410cbdb5b243c
Author: Rong Zhang <i@rong.moe>

leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger

The omnia-mcu trigger always stays in hardware control mode. This commit
implements the offloaded() callback to always return true and declares it
as a hardware control trigger.

> 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",

[Severity: High]
This is a pre-existing issue, but could this statically allocated global
trigger lead to list corruption when multiple instances of the device probe?

If a second instance of the device is bound (e.g., via sysfs new_device or a
DT overlay), omnia_leds_probe() will call devm_led_trigger_register() again
on this same global omnia_hw_trigger structure.

In led_trigger_register(), the core unconditionally executes
INIT_LIST_HEAD(&trig->led_cdevs) and spin_lock_init(&trig->leddev_list_lock)
before checking for duplicate trigger names and returning -EEXIST.

Because the same global structure is passed, this resets the list head of the
first, active instance, stranding the currently attached LEDs.

Subsequent attempts to unregister the first device or trigger events will then
operate on a corrupted linked list, potentially causing a kernel oops.

>  	.activate	= omnia_hwtrig_activate,
>  	.deactivate	= omnia_hwtrig_deactivate,
> +	.offloaded	= omnia_hwtrig_offloaded,
>  	.trigger_type	= &omnia_hw_trigger_type,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-leds-trigger-hw-changed-v4-0-f97e2ca976fe@rong.moe?part=5