[PATCH] leds: lp8860: fix device_node leak in lp8860_probe()

[PATCH] leds: lp8860: fix device_node leak in lp8860_probe()

[Miles Krause]

lp8860_probe() looks up the driver's single LED child node with
of_get_next_available_child() and hands it to the LED core as
init_data.fwnode, but never drops the reference that lookup returned:

	child_node = of_get_next_available_child(np, NULL);
	if (!child_node)
		return -EINVAL;
	...
	init_data.fwnode = of_fwnode_handle(child_node);

of_get_next_available_child() returns the node with its refcount
incremented, and the LED core does not take a reference of its own:
led_classdev_register_ext() only reads properties out of
init_data.fwnode and then stores the bare pointer with
device_set_node().  The reference therefore stays owned by the driver
for as long as it holds the node.

Nothing in lp8860_probe() ever releases it, so it is leaked on every
error return taken after the lookup - the enable GPIO, the vled
regulator, devm_mutex_init(), the regmap allocation, the optional
EEPROM programming and the LED class registration - and on a fully
successful probe alike.  A device that binds and unbinds repeatedly
leaks one device_node reference per bind.

Sibling drivers already get this right: leds-lm3692x.c, from the same
TI LED family, calls fwnode_handle_put(init_data.fwnode) once after
devm_led_classdev_register_ext() to cover both outcomes, while
leds-ktd2692.c and leds-aat1290.c take the node with __free(device_node).

Use __free(device_node) here too.  Unlike a single of_node_put() at the
end of probe it also covers the intermediate error returns, and it keeps
the node alive for the whole function, which the LED core still needs
when it reads the child's properties during registration.

Fixes: 99ca0ea57309 ("leds: lp8860: Use generic support for composing LED names")
Signed-off-by: Miles Krause <mileskrause5200@gmail.com>
---
Found by auditing drivers that take a device_node/fwnode reference during
probe and hand it to a subsystem registration helper without releasing it.

Compile-tested only (x86_64, CONFIG_LEDS_LP8860=m, W=1 clean); I have no
LP8860 hardware.
---
 drivers/leds/leds-lp8860.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 69f064781f69..f6e4227de903 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -7,6 +7,7 @@
  * Author: Dan Murphy <dmurphy@ti.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/leds.h>
@@ -274,7 +275,6 @@ static int lp8860_probe(struct i2c_client *client)
 	int ret;
 	struct lp8860_led *led;
 	struct device_node *np = dev_of_node(&client->dev);
-	struct device_node *child_node;
 	struct led_init_data init_data = {};
 	struct gpio_desc *enable_gpio;
 
@@ -282,7 +282,8 @@ static int lp8860_probe(struct i2c_client *client)
 	if (!led)
 		return -ENOMEM;
 
-	child_node = of_get_next_available_child(np, NULL);
+	struct device_node *child_node __free(device_node) =
+		of_get_next_available_child(np, NULL);
 	if (!child_node)
 		return -EINVAL;
 

---
base-commit: 50d05c7c76c96b90462f24debacca971d2e86713
change-id: 20260910-leds-lp8860-of-node-leak-63b2670ff614

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

Re: [PATCH] leds: lp8860: fix device_node leak in lp8860_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The enable_gpio is initialized to logical low and never asserted, leaving the device permanently disabled.
--

commit fc2187284635f3278f3bb6af71e6925402976134
Author: Miles Krause <mileskrause5200@gmail.com>

leds: lp8860: fix device_node leak in lp8860_probe()

This commit fixes a device_node leak in lp8860_probe() by using the
__free(device_node) cleanup helper. It drops the manual management of
the reference returned by of_get_next_available_child().

> diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
> index 69f064781f699..f6e4227de903a 100644
> --- a/drivers/leds/leds-lp8860.c
> +++ b/drivers/leds/leds-lp8860.c
[ ... ]
> @@ -282,7 +282,8 @@ static int lp8860_probe(struct i2c_client *client)
>  	if (!led)
>  		return -ENOMEM;
>  
> -	child_node = of_get_next_available_child(np, NULL);
> +	struct device_node *child_node __free(device_node) =
> +		of_get_next_available_child(np, NULL);
>  	if (!child_node)
>  		return -EINVAL;
>  

[Severity: High]
This isn't a bug introduced by this patch, but this is a pre-existing 
issue in lp8860_probe(). Immediately following this lookup:

drivers/leds/leds-lp8860.c:lp8860_probe() {
    ...
    enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_LOW);
    ...
}

Does initializing the enable GPIO to logical low (GPIOD_OUT_LOW) leave the 
device permanently disabled? The driver never seems to assert the GPIO to 
high to enable the chip. 

Other sibling drivers appear to correctly initialize their enable GPIO with
GPIOD_OUT_HIGH.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-leds-lp8860-of-node-leak-v1-1-d333cff4345b@gmail.com?part=1