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

Mert Seftali <[email protected]>
Newsgroups org.kernel.vger.linux-leds,org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 equal the number of
allocated elements before leds[] is accessed. Even taking the address
&priv->leds[i] is bounds-checked against num_leds under UBSAN_BOUNDS. The
driver allocates device_get_child_node_count() elements up front, so set
num_leds to that count right after allocation.

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;
the caller walks a led_pwm_data pointer over priv->leds.

No functional change intended.

Suggested-by: Lee Jones <[email protected]>
Signed-off-by: Mert Seftali <[email protected]>
---
Changes in v3:
- v2 followed the review suggestion to pass the element and increment
  num_leds in the caller. That is exactly what Sashiko caught: with
  __counted_by(num_leds), &priv->leds[num_leds] is bounds-checked, so
  incrementing num_leds from 0 as the loop runs trips UBSAN_BOUNDS on
  probe. num_leds is now set to the full count up front and the caller
  walks a led_pwm_data pointer, which keeps the element-passing and the
  dropped index variable, just without the running counter.

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.

Build-tested only (no PWM-LED hardware). The __counted_by bounds behaviour
that motivated v3 was verified with a standalone UBSAN reproducer on gcc and
clang.

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

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 6c1f2f50ff85..d1116a5cb2b2 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,12 +166,12 @@ 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_data *led_data = priv->leds;
 	struct led_pwm led;
 	int ret;
 
@@ -193,7 +192,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, led_data++, &led, fwnode);
 		if (ret)
 			return ret;
 	}
@@ -217,6 +216,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.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.