Re: [PATCH 1/2] mtd: rawnand: fix GPIO descriptor leak in rawnand_dt_parse_gpio_cs()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] mtd: rawnand: arasan: GPIO descriptor leak in anfc_probe()
  and anfc_parse_cs() [1]
--

commit 15f3589a1d2a790f1885eb5bb428694b51898fd3
Author: Zhao Feng <zhaofeng2@cmss.chinamobile.com>

mtd: rawnand: fix GPIO descriptor leak in rawnand_dt_parse_gpio_cs()

This commit releases previously acquired GPIO descriptors with gpiod_put()
if gpiod_get_index_optional() fails during the loop in
rawnand_dt_parse_gpio_cs().

> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index a5b278ab9384f..fa923ef96262a 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5466,7 +5466,7 @@ int rawnand_dt_parse_gpio_cs(struct device *dev, struct gpio_desc ***cs_array,
>  			     unsigned int *ncs_array)
>  {
>  	struct gpio_desc **descs;
> -	int ndescs, i;
> +	int ndescs, i, ret;
>  
>  	ndescs = gpiod_count(dev, "cs");
>  	if (ndescs < 0) {
> @@ -5481,8 +5481,14 @@ int rawnand_dt_parse_gpio_cs(struct device *dev, struct gpio_desc ***cs_array,
>  	for (i = 0; i < ndescs; i++) {
>  		descs[i] = gpiod_get_index_optional(dev, "cs", i,
>  						    GPIOD_OUT_HIGH);
> -		if (IS_ERR(descs[i]))
> -			return PTR_ERR(descs[i]);
> +		if (IS_ERR(descs[i])) {
> +			ret = PTR_ERR(descs[i]);
> +
> +			while (--i >= 0)
> +				gpiod_put(descs[i]);
> +
> +			return ret;
> +		}
>  	}

[Severity: Medium]
This problem wasn't introduced by this patch, but can its caller leak the
same nfc->cs_array GPIO descriptors [1] when probe fails after
rawnand_dt_parse_gpio_cs() succeeds?

Because rawnand_dt_parse_gpio_cs() uses unmanaged gpiod_get_index_optional(),
those descriptors are only released in anfc_remove():

drivers/mtd/nand/raw/arasan-nand-controller.c:anfc_parse_cs() {
	ret = rawnand_dt_parse_gpio_cs(nfc->dev, &nfc->cs_array, &nfc->ncs);
	if (ret)
		return ret;
	...
	if (nfc->cs_array) {
		if (nfc->ncs > 2 && !nfc->cs_array[0] && !nfc->cs_array[1]) {
			dev_err(nfc->dev,
				"Assign a single native CS when using GPIOs\n");
			return -EINVAL;
		}
}

If the native chip-select check in anfc_parse_cs() or a subsequent call to
anfc_chips_init() in anfc_probe() fails, the GPIO lines remain requested and
cause later probe attempts to fail with -EBUSY.  Switching to
devm_gpiod_get_index_optional() (and dropping the manual gpiod_put() loop in
anfc_remove()) would avoid leaking descriptors in both places.

[1] https://sashiko.dev/#/bug/linux-59d8a6d4-32bb-4a7d-8008-c2c272081e11

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109052724.1032-1-zhaofeng2@cmss.chinamobile.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/