[PATCH] leds: max77705: fix fwnode reference leak in max77705_add_led()

[PATCH] leds: max77705: fix fwnode reference leak in max77705_add_led()

[Miles Krause via B4 Relay]

From: Miles Krause <mileskrause5200@gmail.com>

max77705_add_led() iterates over the multicolor LED's child nodes with
fwnode_for_each_child_node() and returns directly from inside the loop
when parsing a child fails:

	fwnode_for_each_child_node(np, child) {
		ret = max77705_parse_subled(dev, child, &info[i]);
		if (ret < 0)
			return ret;
		...
	}

The iterator holds a reference on the current child for the duration of
each iteration: fwnode_get_next_child_node() takes a reference on the
node it returns and only drops the previous one when it is called again
(for the OF backend that is the of_node_put(prev) in
of_get_next_status_child()).  Returning from inside the loop skips that
final call, so the reference taken for the child that failed to parse is
never released.

max77705_parse_subled() rejects a missing, zero or out-of-range "reg"
property and propagates errors from reading "color", so a malformed
device tree is enough to leak a device_node reference.

Use fwnode_for_each_child_node_scoped() instead, which releases the
reference on every exit path, and drop the now unused 'child'
declaration.  max77705_led_probe() already uses the equivalent
device_for_each_child_node_scoped() for the outer loop.

Fixes: aebb5fc9a0d8 ("leds: max77705: Add LEDs support")
Signed-off-by: Miles Krause <mileskrause5200@gmail.com>
---
 drivers/leds/leds-max77705.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/leds/leds-max77705.c b/drivers/leds/leds-max77705.c
index 1e2054c1bf80..4fd803c95989 100644
--- a/drivers/leds/leds-max77705.c
+++ b/drivers/leds/leds-max77705.c
@@ -160,7 +160,6 @@ static int max77705_add_led(struct device *dev, struct regmap *regmap, struct fw
 	struct max77705_led *led;
 	struct led_classdev *cdev;
 	struct mc_subled *info;
-	struct fwnode_handle *child;
 	struct led_init_data init_data = {};
 
 	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
@@ -191,7 +190,7 @@ static int max77705_add_led(struct device *dev, struct regmap *regmap, struct fw
 		cdev->brightness_set_blocking = max77705_led_brightness_set_multi;
 		cdev->blink_set = max77705_rgb_blink;
 
-		fwnode_for_each_child_node(np, child) {
+		fwnode_for_each_child_node_scoped(np, child) {
 			ret = max77705_parse_subled(dev, child, &info[i]);
 			if (ret < 0)
 				return ret;

---
base-commit: 88405f0ad1d5c680afe3ea0ce9345fa9e1deaac8
change-id: 20260906-leds-max77705-fwnode-leak-1cde49cec58c

Best regards,
-- 
Miles Krause <mileskrause5200@gmail.com>

Re: [PATCH] leds: max77705: fix fwnode reference leak in max77705_add_led()

[Lee Jones]

On Sun, 06 Sep 2026, Miles Krause via B4 Relay wrote:

> From: Miles Krause <mileskrause5200@gmail.com>
> 
> max77705_add_led() iterates over the multicolor LED's child nodes with
> fwnode_for_each_child_node() and returns directly from inside the loop
> when parsing a child fails:
> 
> 	fwnode_for_each_child_node(np, child) {
> 		ret = max77705_parse_subled(dev, child, &info[i]);
> 		if (ret < 0)
> 			return ret;
> 		...
> 	}
> 
> The iterator holds a reference on the current child for the duration of
> each iteration: fwnode_get_next_child_node() takes a reference on the
> node it returns and only drops the previous one when it is called again
> (for the OF backend that is the of_node_put(prev) in
> of_get_next_status_child()).  Returning from inside the loop skips that
> final call, so the reference taken for the child that failed to parse is
> never released.
> 
> max77705_parse_subled() rejects a missing, zero or out-of-range "reg"
> property and propagates errors from reading "color", so a malformed
> device tree is enough to leak a device_node reference.
> 
> Use fwnode_for_each_child_node_scoped() instead, which releases the
> reference on every exit path, and drop the now unused 'child'
> declaration.  max77705_led_probe() already uses the equivalent
> device_for_each_child_node_scoped() for the outer loop.
> 
> Fixes: aebb5fc9a0d8 ("leds: max77705: Add LEDs support")
> Signed-off-by: Miles Krause <mileskrause5200@gmail.com>

Beaten to it by this I'm afraid:

  4805e5ec50d0 leds: max77705: Use fwnode_for_each_child_node_scoped() in max77705_add_led()

-- 
Lee Jones

Re: [PATCH] leds: max77705: fix fwnode reference leak in max77705_add_led()

[Krzysztof Kozlowski]

On 28/08/2026 10:46, Manush Prajwal wrote:
> In the RGB LED path of max77705_add_led(), the
> fwnode_for_each_child_node() loop over the subled child nodes
> returns directly on a max77705_parse_subled() failure without
> releasing the reference held on the current child, since
> fwnode_for_each_child_node() is not a scoped/cleanup-based
> iterator and expects the caller to drop the reference itself on
> any early exit from the loop body.
> 
> Call fwnode_handle_put() on the child fwnode before returning to
> fix the leak.
> 
> Fixes: 3b6eaa3db567 ("leds: Use fwnode_for_each_child_node() instead")
> Signed-off-by: Manush Prajwal <manushprajwal555@gmail.com>
> ---
>  drivers/leds/leds-max77705.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/leds/leds-max77705.c b/drivers/leds/leds-max77705.c
> index 1e2054c..a5030c3 100644
> --- a/drivers/leds/leds-max77705.c
> +++ b/drivers/leds/leds-max77705.c
> @@ -193,8 +193,10 @@ static int max77705_add_led(struct device *dev, struct regmap *regmap, struct fw
> 
>  		fwnode_for_each_child_node(np, child) {

Would fwnode_for_each_child_node_scoped() work here?

>  			ret = max77705_parse_subled(dev, child, &info[i]);
> -			if (ret < 0)
> +			if (ret < 0) {
> +				fwnode_handle_put(child);
>  				return ret;
> +			}
> 
Best regards,
Krzysztof

[PATCH] leds: max77705: fix fwnode reference leak in max77705_add_led()

[Manush Prajwal]

In the RGB LED path of max77705_add_led(), the
fwnode_for_each_child_node() loop over the subled child nodes
returns directly on a max77705_parse_subled() failure without
releasing the reference held on the current child, since
fwnode_for_each_child_node() is not a scoped/cleanup-based
iterator and expects the caller to drop the reference itself on
any early exit from the loop body.

Call fwnode_handle_put() on the child fwnode before returning to
fix the leak.

Fixes: 3b6eaa3db567 ("leds: Use fwnode_for_each_child_node() instead")
Signed-off-by: Manush Prajwal <manushprajwal555@gmail.com>
---
 drivers/leds/leds-max77705.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-max77705.c b/drivers/leds/leds-max77705.c
index 1e2054c..a5030c3 100644
--- a/drivers/leds/leds-max77705.c
+++ b/drivers/leds/leds-max77705.c
@@ -193,8 +193,10 @@ static int max77705_add_led(struct device *dev, struct regmap *regmap, struct fw

 		fwnode_for_each_child_node(np, child) {
 			ret = max77705_parse_subled(dev, child, &info[i]);
-			if (ret < 0)
+			if (ret < 0) {
+				fwnode_handle_put(child);
 				return ret;
+			}

 			info[i].intensity = 0;
 			i++;
--
2.46.2.windows.1