[PATCH v2] leds: pwm: Annotate leds[] with __counted_by()

[PATCH v2] leds: pwm: Annotate leds[] with __counted_by()

[Mert Seftali]

From: Mert Seftali <mertsftl@gmail.com>

Add the __counted_by() attribute to the flexible array member leds[] in
struct led_pwm_priv so the compiler and runtime (e.g. FORTIFY_SOURCE,
UBSAN_BOUNDS) can bounds-check accesses against num_leds.

While here, pass the led_pwm_data element into led_pwm_add() instead of
the whole led_pwm_priv, so the helper no longer needs to index the array
itself. num_leds is incremented in the caller as each LED is added and
doubles as the index.

No functional change intended.

Suggested-by: Lee Jones <lee@kernel.org>
Signed-off-by: Mert Seftali <mertsftl@gmail.com>
---
Changes in v2 (per Lee Jones review):
- Pass the led_pwm_data element into led_pwm_add() so it drops the priv
  and index arguments; increment num_leds in the caller, where it also
  serves as the array index.

Build-tested only (also with FORTIFY_SOURCE and UBSAN_BOUNDS enabled).

 drivers/leds/leds-pwm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 6c1f2f50ff85..dabdf0423582 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -36,7 +36,7 @@ struct led_pwm_data {
 
 struct led_pwm_priv {
 	int num_leds;
-	struct led_pwm_data leds[];
+	struct led_pwm_data leds[] __counted_by(num_leds);
 };
 
 static int led_pwm_set(struct led_classdev *led_cdev,
@@ -81,10 +81,9 @@ static int led_pwm_default_brightness_get(struct fwnode_handle *fwnode,
 }
 
 __attribute__((nonnull))
-static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
+static int led_pwm_add(struct device *dev, struct led_pwm_data *led_data,
 		       struct led_pwm *led, struct fwnode_handle *fwnode)
 {
-	struct led_pwm_data *led_data = &priv->leds[priv->num_leds];
 	struct led_init_data init_data = { .fwnode = fwnode };
 	int ret;
 
@@ -167,7 +166,6 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
 		}
 	}
 
-	priv->num_leds++;
 	return 0;
 }
 
@@ -193,9 +191,10 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
 
 		led.default_state = led_init_default_state_get(fwnode);
 
-		ret = led_pwm_add(dev, priv, &led, fwnode);
+		ret = led_pwm_add(dev, &priv->leds[priv->num_leds], &led, fwnode);
 		if (ret)
 			return ret;
+		priv->num_leds++;
 	}
 
 	return 0;
-- 
2.55.0

Re: [PATCH v2] leds: pwm: Annotate leds[] with __counted_by()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The addition of the `__counted_by(num_leds)` annotation causes an unconditional out-of-bounds trap during probe.
--

commit f2069ade09481403f091ed2758ef698b0c75c2cc
Author: Mert Seftali <mertsftl@gmail.com>

leds: pwm: Annotate leds[] with __counted_by()

This commit adds the __counted_by(num_leds) attribute to the flexible array
member leds[] in struct led_pwm_priv for bounds checking. It also updates
led_pwm_add() to accept the led_pwm_data element directly, leaving num_leds
to be incremented in the caller.

> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index 6c1f2f50ff854..dabdf04235826 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -36,7 +36,7 @@ struct led_pwm_data {
>  
>  struct led_pwm_priv {
>  	int num_leds;
> -	struct led_pwm_data leds[];
> +	struct led_pwm_data leds[] __counted_by(num_leds);
>  };

[ ... ]

> @@ -193,9 +191,10 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
>  
>  		led.default_state = led_init_default_state_get(fwnode);
>  
> -		ret = led_pwm_add(dev, priv, &led, fwnode);
> +		ret = led_pwm_add(dev, &priv->leds[priv->num_leds], &led, fwnode);

[Severity: High]
Does evaluating &priv->leds[priv->num_leds] before incrementing num_leds
trigger an out-of-bounds trap under UBSAN_BOUNDS or FORTIFY_SOURCE?

Because leds is now annotated with __counted_by(num_leds), the compiler
expects the valid indices to be less than num_leds. When this loop starts
in led_pwm_create_fwnode(), priv->num_leds is initially 0. Taking the address
of index 0 when the count is 0 will fail the bounds check and trigger a
splat during driver probe.

>  		if (ret)
>  			return ret;
> +		priv->num_leds++;
>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702155452.67594-1-mertseftali@web.de?part=1