* [PATCH] leds: pwm: Annotate leds[] with __counted_by()
@ 2026-06-19 16:21 Mert Seftali
2026-07-02 14:49 ` Lee Jones
0 siblings, 1 reply; 3+ messages in thread
From: Mert Seftali @ 2026-06-19 16:21 UTC (permalink / raw)
To: Lee Jones, Pavel Machek
Cc: Kees Cook, Gustavo A . R . Silva, linux-leds, linux-hardening,
linux-kernel, Mert Seftali
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);
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] leds: pwm: Annotate leds[] with __counted_by()
2026-06-19 16:21 [PATCH] leds: pwm: Annotate leds[] with __counted_by() Mert Seftali
@ 2026-07-02 14:49 ` Lee Jones
2026-07-02 15:39 ` Mert Seftali
0 siblings, 1 reply; 3+ messages in thread
From: Lee Jones @ 2026-07-02 14:49 UTC (permalink / raw)
To: Mert Seftali
Cc: Pavel Machek, Kees Cook, Gustavo A . R . Silva, linux-leds,
linux-hardening, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] leds: pwm: Annotate leds[] with __counted_by()
2026-07-02 14:49 ` Lee Jones
@ 2026-07-02 15:39 ` Mert Seftali
0 siblings, 0 replies; 3+ messages in thread
From: Mert Seftali @ 2026-07-02 15:39 UTC (permalink / raw)
To: Lee Jones
Cc: Pavel Machek, Kees Cook, Gustavo A . R . Silva, linux-leds,
linux-hardening, linux-kernel
On Fri, 19 Jun 2026, Lee Jones wrote:
> 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.
Sure, that's tidier, i'll do both. Passing the led_pwm_data element in
means led_pwm_add doesn't need priv or the index anymore, so it drops to
(dev, led_data, led, fwnode). And bumping num_leds in the loop means it
doubles as the index, so the throw-away i goes away too.
will prepare v2, thanks for the review!
Mert
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-02 15:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-19 16:21 [PATCH] leds: pwm: Annotate leds[] with __counted_by() Mert Seftali
2026-07-02 14:49 ` Lee Jones
2026-07-02 15:39 ` Mert Seftali
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox