[PATCH] leds: netxbig: fix GPIO descriptor leak in error paths

[PATCH] leds: netxbig: fix GPIO descriptor leak in error paths

[Haotian Zhang]

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")
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/leds/leds-netxbig.c | 43 ++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index e95287416ef8..cc8c1c5006bc 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
@@ -374,46 +377,62 @@ static int netxbig_gpio_ext_get(struct device *dev,
 	for (i = 0; i < num_addr; i++) {
 		gpiod = gpiod_get_index(gpio_ext_dev, "addr", i,
 					GPIOD_OUT_LOW);
-		if (IS_ERR(gpiod))
-			return PTR_ERR(gpiod);
+		if (IS_ERR(gpiod)) {
+			ret = PTR_ERR(gpiod);
+			goto err_free_addr;
+		}
 		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);
+		if (IS_ERR(gpiod)) {
+			ret = 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);
+		ret = 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_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

Re: [PATCH] leds: netxbig: fix GPIO descriptor leak in error paths

[Markus Elfring]

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

I propose to move the statement “ret = PTR_ERR(gpiod);” behind
an additional label.

Regards,
Markus

[PATCH v2] leds: netxbig: fix GPIO descriptor leak in error paths

[Haotian Zhang]

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 v2:
- Consolidate PTR_ERR(gpiod) extraction into err_gpiod_put label
  (suggested by Markus Elfring)
---
 drivers/leds/leds-netxbig.c | 40 +++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index e95287416ef8..55afb03ee933 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
@@ -374,46 +377,61 @@ static int netxbig_gpio_ext_get(struct device *dev,
 	for (i = 0; i < num_addr; i++) {
 		gpiod = gpiod_get_index(gpio_ext_dev, "addr", i,
 					GPIOD_OUT_LOW);
-		if (IS_ERR(gpiod))
-			return PTR_ERR(gpiod);
+		if (IS_ERR(gpiod)) {
+			ret = PTR_ERR(gpiod);
+			goto err_free_addr;
+		}
 		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_gpiod_put;
 		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_gpiod_put;
 	}
 	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_gpiod_put:
+	ret = PTR_ERR(gpiod);
+err_free_data:
+	for (i = 0; i < gpio_ext->num_data; i++)
+		gpiod_put(gpio_ext->data[i]);
+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

Re: [PATCH v2] leds: netxbig: fix GPIO descriptor leak in error paths

[Markus Elfring]

…> ---
> Changes in v2:
> - Consolidate PTR_ERR(gpiod) extraction into err_gpiod_put label
…

Thanks.

I see another refinement possibility.


…> +++ b/drivers/leds/leds-netxbig.c
…> @@ -374,46 +377,61 @@ static int netxbig_gpio_ext_get(struct device *dev,
>  	for (i = 0; i < num_addr; i++) {
>  		gpiod = gpiod_get_index(gpio_ext_dev, "addr", i,
>  					GPIOD_OUT_LOW);
> -		if (IS_ERR(gpiod))
> -			return PTR_ERR(gpiod);
> +		if (IS_ERR(gpiod)) {
> +			ret = PTR_ERR(gpiod);

Why do propose to add this statement here?


> +			goto err_free_addr;
> +		}
…> +err_gpiod_put:
> +	ret = PTR_ERR(gpiod);
> +err_free_data:
> +	for (i = 0; i < gpio_ext->num_data; i++)
> +		gpiod_put(gpio_ext->data[i]);


Would you find the following source code usable between the other two labels?

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;
>  }
…

Regards,
Markus

Re: [PATCH v2] leds: netxbig: fix GPIO descriptor leak in error paths

[kernel test robot]

Hi Haotian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-leds/for-leds-next]
[also build test WARNING on linus/master v6.18-rc3 next-20251030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Haotian-Zhang/leds-netxbig-fix-GPIO-descriptor-leak-in-error-paths/20251030-105705
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next
patch link:    https://lore.kernel.org/r/20251030025312.1623-1-vulab%40iscas.ac.cn
patch subject: [PATCH v2] leds: netxbig: fix GPIO descriptor leak in error paths
config: nios2-randconfig-r072-20251030 (https://download.01.org/0day-ci/archive/20251031/202510310145.tlVMfaP8-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251031/202510310145.tlVMfaP8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510310145.tlVMfaP8-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/leds/leds-netxbig.c: In function 'netxbig_gpio_ext_get':
>> drivers/leds/leds-netxbig.c:428:1: warning: label 'err_free_data' defined but not used [-Wunused-label]
     428 | err_free_data:
         | ^~~~~~~~~~~~~


vim +/err_free_data +428 drivers/leds/leds-netxbig.c

   335	
   336	/**
   337	 * netxbig_gpio_ext_get() - Obtain GPIO extension device data
   338	 * @dev: main LED device
   339	 * @gpio_ext_dev: the GPIO extension device
   340	 * @gpio_ext: the data structure holding the GPIO extension data
   341	 *
   342	 * This function walks the subdevice that only contain GPIO line
   343	 * handles in the device tree and obtains the GPIO descriptors from that
   344	 * device.
   345	 */
   346	static int netxbig_gpio_ext_get(struct device *dev,
   347					struct device *gpio_ext_dev,
   348					struct netxbig_gpio_ext *gpio_ext)
   349	{
   350		struct gpio_desc **addr, **data;
   351		int num_addr, num_data;
   352		struct gpio_desc *gpiod;
   353		int ret;
   354		int i;
   355	
   356		ret = gpiod_count(gpio_ext_dev, "addr");
   357		if (ret < 0) {
   358			dev_err(dev,
   359				"Failed to count GPIOs in DT property addr-gpios\n");
   360			return ret;
   361		}
   362		num_addr = ret;
   363		addr = devm_kcalloc(dev, num_addr, sizeof(*addr), GFP_KERNEL);
   364		if (!addr)
   365			return -ENOMEM;
   366	
   367		gpio_ext->addr = addr;
   368		gpio_ext->num_addr = 0;
   369	
   370		/*
   371		 * We cannot use devm_ managed resources with these GPIO descriptors
   372		 * since they are associated with the "GPIO extension device" which
   373		 * does not probe any driver. The device tree parser will however
   374		 * populate a platform device for it so we can anyway obtain the
   375		 * GPIO descriptors from the device.
   376		 */
   377		for (i = 0; i < num_addr; i++) {
   378			gpiod = gpiod_get_index(gpio_ext_dev, "addr", i,
   379						GPIOD_OUT_LOW);
   380			if (IS_ERR(gpiod)) {
   381				ret = PTR_ERR(gpiod);
   382				goto err_free_addr;
   383			}
   384			gpiod_set_consumer_name(gpiod, "GPIO extension addr");
   385			addr[i] = gpiod;
   386			gpio_ext->num_addr++;
   387		}
   388	
   389		ret = gpiod_count(gpio_ext_dev, "data");
   390		if (ret < 0) {
   391			dev_err(dev,
   392				"Failed to count GPIOs in DT property data-gpios\n");
   393			goto err_free_addr;
   394		}
   395		num_data = ret;
   396		data = devm_kcalloc(dev, num_data, sizeof(*data), GFP_KERNEL);
   397		if (!data) {
   398			ret = -ENOMEM;
   399			goto err_free_addr;
   400		}
   401	
   402		gpio_ext->data = data;
   403		gpio_ext->num_data = 0;
   404	
   405		for (i = 0; i < num_data; i++) {
   406			gpiod = gpiod_get_index(gpio_ext_dev, "data", i,
   407						GPIOD_OUT_LOW);
   408			if (IS_ERR(gpiod))
   409				goto err_gpiod_put;
   410			gpiod_set_consumer_name(gpiod, "GPIO extension data");
   411			data[i] = gpiod;
   412			gpio_ext->num_data++;
   413		}
   414	
   415		gpiod = gpiod_get(gpio_ext_dev, "enable", GPIOD_OUT_LOW);
   416		if (IS_ERR(gpiod)) {
   417			dev_err(dev,
   418				"Failed to get GPIO from DT property enable-gpio\n");
   419			goto err_gpiod_put;
   420		}
   421		gpiod_set_consumer_name(gpiod, "GPIO extension enable");
   422		gpio_ext->enable = gpiod;
   423	
   424		return devm_add_action_or_reset(dev, netxbig_gpio_ext_remove, gpio_ext);
   425	
   426	err_gpiod_put:
   427		ret = PTR_ERR(gpiod);
 > 428	err_free_data:
   429		for (i = 0; i < gpio_ext->num_data; i++)
   430			gpiod_put(gpio_ext->data[i]);
   431	err_free_addr:
   432		for (i = 0; i < gpio_ext->num_addr; i++)
   433			gpiod_put(gpio_ext->addr[i]);
   434		return ret;
   435	}
   436	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] leds: netxbig: fix GPIO descriptor leak in error paths

[kernel test robot]

Hi Haotian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-leds/for-leds-next]
[also build test WARNING on linus/master v6.18-rc3 next-20251030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Haotian-Zhang/leds-netxbig-fix-GPIO-descriptor-leak-in-error-paths/20251030-105705
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next
patch link:    https://lore.kernel.org/r/20251030025312.1623-1-vulab%40iscas.ac.cn
patch subject: [PATCH v2] leds: netxbig: fix GPIO descriptor leak in error paths
config: sparc64-randconfig-002-20251031 (https://download.01.org/0day-ci/archive/20251031/202510310640.VxJ2qrj2-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project d1c086e82af239b245fe8d7832f2753436634990)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251031/202510310640.VxJ2qrj2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510310640.VxJ2qrj2-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/leds/leds-netxbig.c:428:1: warning: unused label 'err_free_data' [-Wunused-label]
     428 | err_free_data:
         | ^~~~~~~~~~~~~~
   1 warning generated.


vim +/err_free_data +428 drivers/leds/leds-netxbig.c

   335	
   336	/**
   337	 * netxbig_gpio_ext_get() - Obtain GPIO extension device data
   338	 * @dev: main LED device
   339	 * @gpio_ext_dev: the GPIO extension device
   340	 * @gpio_ext: the data structure holding the GPIO extension data
   341	 *
   342	 * This function walks the subdevice that only contain GPIO line
   343	 * handles in the device tree and obtains the GPIO descriptors from that
   344	 * device.
   345	 */
   346	static int netxbig_gpio_ext_get(struct device *dev,
   347					struct device *gpio_ext_dev,
   348					struct netxbig_gpio_ext *gpio_ext)
   349	{
   350		struct gpio_desc **addr, **data;
   351		int num_addr, num_data;
   352		struct gpio_desc *gpiod;
   353		int ret;
   354		int i;
   355	
   356		ret = gpiod_count(gpio_ext_dev, "addr");
   357		if (ret < 0) {
   358			dev_err(dev,
   359				"Failed to count GPIOs in DT property addr-gpios\n");
   360			return ret;
   361		}
   362		num_addr = ret;
   363		addr = devm_kcalloc(dev, num_addr, sizeof(*addr), GFP_KERNEL);
   364		if (!addr)
   365			return -ENOMEM;
   366	
   367		gpio_ext->addr = addr;
   368		gpio_ext->num_addr = 0;
   369	
   370		/*
   371		 * We cannot use devm_ managed resources with these GPIO descriptors
   372		 * since they are associated with the "GPIO extension device" which
   373		 * does not probe any driver. The device tree parser will however
   374		 * populate a platform device for it so we can anyway obtain the
   375		 * GPIO descriptors from the device.
   376		 */
   377		for (i = 0; i < num_addr; i++) {
   378			gpiod = gpiod_get_index(gpio_ext_dev, "addr", i,
   379						GPIOD_OUT_LOW);
   380			if (IS_ERR(gpiod)) {
   381				ret = PTR_ERR(gpiod);
   382				goto err_free_addr;
   383			}
   384			gpiod_set_consumer_name(gpiod, "GPIO extension addr");
   385			addr[i] = gpiod;
   386			gpio_ext->num_addr++;
   387		}
   388	
   389		ret = gpiod_count(gpio_ext_dev, "data");
   390		if (ret < 0) {
   391			dev_err(dev,
   392				"Failed to count GPIOs in DT property data-gpios\n");
   393			goto err_free_addr;
   394		}
   395		num_data = ret;
   396		data = devm_kcalloc(dev, num_data, sizeof(*data), GFP_KERNEL);
   397		if (!data) {
   398			ret = -ENOMEM;
   399			goto err_free_addr;
   400		}
   401	
   402		gpio_ext->data = data;
   403		gpio_ext->num_data = 0;
   404	
   405		for (i = 0; i < num_data; i++) {
   406			gpiod = gpiod_get_index(gpio_ext_dev, "data", i,
   407						GPIOD_OUT_LOW);
   408			if (IS_ERR(gpiod))
   409				goto err_gpiod_put;
   410			gpiod_set_consumer_name(gpiod, "GPIO extension data");
   411			data[i] = gpiod;
   412			gpio_ext->num_data++;
   413		}
   414	
   415		gpiod = gpiod_get(gpio_ext_dev, "enable", GPIOD_OUT_LOW);
   416		if (IS_ERR(gpiod)) {
   417			dev_err(dev,
   418				"Failed to get GPIO from DT property enable-gpio\n");
   419			goto err_gpiod_put;
   420		}
   421		gpiod_set_consumer_name(gpiod, "GPIO extension enable");
   422		gpio_ext->enable = gpiod;
   423	
   424		return devm_add_action_or_reset(dev, netxbig_gpio_ext_remove, gpio_ext);
   425	
   426	err_gpiod_put:
   427		ret = PTR_ERR(gpiod);
 > 428	err_free_data:
   429		for (i = 0; i < gpio_ext->num_data; i++)
   430			gpiod_put(gpio_ext->data[i]);
   431	err_free_addr:
   432		for (i = 0; i < gpio_ext->num_addr; i++)
   433			gpiod_put(gpio_ext->addr[i]);
   434		return ret;
   435	}
   436	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v3] leds: netxbig: fix GPIO descriptor leak in error paths

[Haotian Zhang]

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

Re: (subset) [PATCH v3] leds: netxbig: fix GPIO descriptor leak in error paths

[Lee Jones]

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 [李琼斯]

Re: [PATCH v3] leds: netxbig: fix GPIO descriptor leak in error paths

[Andy Shevchenko]

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