Re: [PATCH v4 04/12] leds: cros_ec: Implement offloaded() trigger callback

[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 global static `led_trigger` structure in a driver's probe function causes list corruption and lock re-initialization if the driver is probed multiple times.
--

--- Patch [4]: [PATCH v4 04/12] leds: cros_ec: Implement offloaded() trigger callback ---
commit 74008c8c0e283ff69ceee35b25286706470006e0
Author: Rong Zhang <i@rong.moe>

leds: cros_ec: Implement offloaded() trigger callback
    
"chromeos-auto" is a private hardware control trigger which always stays
in hardware control. Implement offloaded() callback with its return
value to be always true to reflect this.

> diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c
> index 1844d0cd5f528..6db83d0152775 100644
> --- a/drivers/leds/leds-cros_ec.c
> +++ b/drivers/leds/leds-cros_ec.c
[ ... ]
>  static struct led_hw_trigger_type cros_ec_led_trigger_type;
>  
>  static struct led_trigger cros_ec_led_trigger = {
>  	.name = "chromeos-auto",
>  	.trigger_type = &cros_ec_led_trigger_type,
>  	.activate = cros_ec_led_trigger_activate,
> +	.offloaded = cros_ec_led_trigger_offloaded,
>  };

[Severity: High]
This is a pre-existing issue, but registering a global static led_trigger
structure in a driver's probe function can cause list corruption and lock
re-initialization if the driver is probed multiple times.

When cros_ec_led_probe() calls devm_led_trigger_register() and passes
this global static cros_ec_led_trigger struct, led_trigger_register()
unconditionally re-initializes the led_cdevs list and leddev_list_lock
on the passed pointer before checking if it already exists:

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

Does this corrupt the state of the active trigger if a subsequent probe
(such as for a second EC or manually via sysfs) calls it again with the
exact same static pointer?

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