The function netxbig_gpio_ext_get() acquires GPIO descriptors but
fails to release them when errors occur mid-way through initialization.
The cleanup callback registered by devm_add_action_or_reset() only
runs on success, leaving acquired GPIOs leaked on error paths.

Add goto-based error handling to release all acquired GPIOs before
returning errors.

Fixes: 9af512e81964 ("leds: netxbig: Convert to use GPIO descriptors")
Suggested-by: Markus Elfring <Markus.Elfring@web.de>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

---
Changes in v3:
- Rename err_gpiod_put to err_set_code for clarity
- Position err_set_code between err_free_data and err_free_addr
  as suggested by Markus Elfring
Changes in v2:
- Consolidate PTR_ERR(gpiod) extraction into err_gpiod_put label
  (suggested by Markus Elfring)
---
 drivers/leds/leds-netxbig.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index e95287416ef8..99df46f2d9f5 100644
--- a/drivers/leds/leds-netxbig.c
+++ b/drivers/leds/leds-netxbig.c
@@ -364,6 +364,9 @@ static int netxbig_gpio_ext_get(struct device *dev,
 	if (!addr)
 		return -ENOMEM;
 
+	gpio_ext->addr = addr;
+	gpio_ext->num_addr = 0;
+
 	/*
 	 * We cannot use devm_ managed resources with these GPIO descriptors
 	 * since they are associated with the "GPIO extension device" which
@@ -375,45 +378,58 @@ static int netxbig_gpio_ext_get(struct device *dev,
 		gpiod = gpiod_get_index(gpio_ext_dev, "addr", i,
 					GPIOD_OUT_LOW);
 		if (IS_ERR(gpiod))
-			return PTR_ERR(gpiod);
+			goto err_set_code;
 		gpiod_set_consumer_name(gpiod, "GPIO extension addr");
 		addr[i] = gpiod;
+		gpio_ext->num_addr++;
 	}
-	gpio_ext->addr = addr;
-	gpio_ext->num_addr = num_addr;
 
 	ret = gpiod_count(gpio_ext_dev, "data");
 	if (ret < 0) {
 		dev_err(dev,
 			"Failed to count GPIOs in DT property data-gpios\n");
-		return ret;
+		goto err_free_addr;
 	}
 	num_data = ret;
 	data = devm_kcalloc(dev, num_data, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		ret = -ENOMEM;
+		goto err_free_addr;
+	}
+
+	gpio_ext->data = data;
+	gpio_ext->num_data = 0;
 
 	for (i = 0; i < num_data; i++) {
 		gpiod = gpiod_get_index(gpio_ext_dev, "data", i,
 					GPIOD_OUT_LOW);
 		if (IS_ERR(gpiod))
-			return PTR_ERR(gpiod);
+			goto err_free_data;
 		gpiod_set_consumer_name(gpiod, "GPIO extension data");
 		data[i] = gpiod;
+		gpio_ext->num_data++;
 	}
-	gpio_ext->data = data;
-	gpio_ext->num_data = num_data;
 
 	gpiod = gpiod_get(gpio_ext_dev, "enable", GPIOD_OUT_LOW);
 	if (IS_ERR(gpiod)) {
 		dev_err(dev,
 			"Failed to get GPIO from DT property enable-gpio\n");
-		return PTR_ERR(gpiod);
+		goto err_free_data;
 	}
 	gpiod_set_consumer_name(gpiod, "GPIO extension enable");
 	gpio_ext->enable = gpiod;
 
 	return devm_add_action_or_reset(dev, netxbig_gpio_ext_remove, gpio_ext);
+
+err_free_data:
+	for (i = 0; i < gpio_ext->num_data; i++)
+		gpiod_put(gpio_ext->data[i]);
+err_set_code:
+	ret = PTR_ERR(gpiod);
+err_free_addr:
+	for (i = 0; i < gpio_ext->num_addr; i++)
+		gpiod_put(gpio_ext->addr[i]);
+	return ret;
 }
 
 static int netxbig_leds_get_of_pdata(struct device *dev,
-- 
2.50.1.windows.1

On Fri, Oct 31, 2025 at 10:16:20AM +0800, Haotian Zhang wrote:
> The function netxbig_gpio_ext_get() acquires GPIO descriptors but
> fails to release them when errors occur mid-way through initialization.
> The cleanup callback registered by devm_add_action_or_reset() only
> runs on success, leaving acquired GPIOs leaked on error paths.
> 
> Add goto-based error handling to release all acquired GPIOs before
> returning errors.

...

>  	data = devm_kcalloc(dev, num_data, sizeof(*data), GFP_KERNEL);
> +	if (!data) {
> +		ret = -ENOMEM;
> +		goto err_free_addr;
> +	}
> +
> +	gpio_ext->data = data;
> +	gpio_ext->num_data = 0;
>  
>  	for (i = 0; i < num_data; i++) {
>  		gpiod = gpiod_get_index(gpio_ext_dev, "data", i,
>  					GPIOD_OUT_LOW);
>  		if (IS_ERR(gpiod))
> +			goto err_free_data;
>  		gpiod_set_consumer_name(gpiod, "GPIO extension data");
>  		data[i] = gpiod;
> +		gpio_ext->num_data++;
>  	}

While fixing one issue, this brings wrong order of the devm_ and non-devm
resource cleaning. This may lead in some cases to the crash at ->remove() or on
error path at ->probe().

I think this needs much deeper refactoring, and rethinking. Easiest approach is
to get rid of devm_ allocations altogether with a huge comment why.

That said, NAK to it in _this_ form.
(However I see it is already applied, so perhaps it will be fixed by some
 followups)

-- 
With Best Regards,
Andy Shevchenko

On Fri, 31 Oct 2025 10:16:20 +0800, Haotian Zhang wrote:
> The function netxbig_gpio_ext_get() acquires GPIO descriptors but
> fails to release them when errors occur mid-way through initialization.
> The cleanup callback registered by devm_add_action_or_reset() only
> runs on success, leaving acquired GPIOs leaked on error paths.
> 
> Add goto-based error handling to release all acquired GPIOs before
> returning errors.
> 
> [...]

Applied, thanks!

[1/1] leds: netxbig: fix GPIO descriptor leak in error paths
      commit: 03865dd8af52eb16c38062df2ed30a91b604780e

--
Lee Jones [李琼斯]