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

[Lee Jones <lee@kernel.org>]

On Fri, 19 Jun 2026, Mert Seftali wrote:

> 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.
> 
> For the annotation to be correct, num_leds must reflect the number of
> allocated elements before leds[] is accessed. The driver already
> allocates space for device_get_child_node_count() elements up front, so
> set num_leds to that count right after allocation and fill the array by
> explicit index, instead of incrementing num_leds as each LED is added.
> 
> No functional change intended.
> 
> Signed-off-by: Mert Seftali <mertsftl@gmail.com>
> ---
> Build-tested only (also with FORTIFY_SOURCE and UBSAN_BOUNDS enabled).
> 
>  drivers/leds/leds-pwm.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index 6c1f2f50ff85..4c99a07da576 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,
> @@ -82,9 +82,10 @@ 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,
> -		       struct led_pwm *led, struct fwnode_handle *fwnode)
> +		       int idx, struct led_pwm *led,
> +		       struct fwnode_handle *fwnode)
>  {
> -	struct led_pwm_data *led_data = &priv->leds[priv->num_leds];
> +	struct led_pwm_data *led_data = &priv->leds[idx];
>  	struct led_init_data init_data = { .fwnode = fwnode };
>  	int ret;
>  
> @@ -167,14 +168,13 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
>  		}
>  	}
>  
> -	priv->num_leds++;
>  	return 0;
>  }
>  
>  static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
>  {
>  	struct led_pwm led;
> -	int ret;
> +	int ret, i = 0;
>  	device_for_each_child_node_scoped(dev, fwnode) {
>  		memset(&led, 0, sizeof(led));
> @@ -193,7 +193,7 @@ 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, i++, &led, fwnode);

Nit, why not keep the variable names the same rather than use the for
iterator?  i's and j's are usually for throw-away variables, but this is
counting something real.

Or if you wanted to be even smarter, why not just pass the priv->leds[n]
that you want to operate on and increment num_leds here instead?

Less args == more better.

>  		if (ret)
>  			return ret;
>  	}
> @@ -217,6 +217,8 @@ static int led_pwm_probe(struct platform_device *pdev)
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	priv->num_leds = count;
> +
>  	ret = led_pwm_create_fwnode(&pdev->dev, priv);
>  
>  	if (ret)
> -- 
> 2.54.0
> 

-- 
Lee Jones