Re: [PATCH v6 05/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.
--

commit bc24951be035009b57d7197b147b7679371f913d
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
> @@ -85,12 +85,18 @@ static int cros_ec_led_trigger_activate(struct led_classdev *led_cdev)
>  	return cros_ec_led_send_cmd(priv->cros_ec, &arg);
>  }
>  
> +static bool cros_ec_led_trigger_offloaded(struct led_classdev *led_cdev)
> +{
> +	return true;
> +}
> +
>  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 does registering this global static
led_trigger structure in a driver's probe function cause list corruption and
lock re-initialization if the driver is probed multiple times?

If cros_ec_led_probe() runs multiple times (for example, if a user manually
binds the driver to a secondary device via sysfs, or if multiple matching
devices are present), devm_led_trigger_register() would be called multiple
times on this same global structure.

In led_trigger_register():

	spin_lock_init(&trig->leddev_list_lock);
	INIT_LIST_HEAD(&trig->led_cdevs);

This unconditional initialization corrupts the in-use lock and list head
before checking for duplicates and returning -EEXIST. Could this memory
corruption lead to a kernel panic or use-after-free when iterating the list
or tearing down the trigger?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-leds-trigger-hw-changed-v6-0-55693cd78877@rong.moe?part=5