[PATCH] mtd: rawnand: atmel: Fix HSMC clock leak in legacy controller init

[PATCH] mtd: rawnand: atmel: Fix HSMC clock leak in legacy controller init

[Wentao Liang]

atmel_hsmc_nand_controller_legacy_init() takes a reference to the
HSMC clock with of_clk_get() and enables it, but the error paths
that follow only release the device node, so both the reference and
the enable are leaked whenever the controller cannot be fully
initialized.  The probe fails in that case and never reaches
atmel_hsmc_nand_controller_remove(), which is where the clock is
normally disabled and put.

Add an err_disable_clk path that disables and releases the clock
before jumping to the existing out path.

Fixes: f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 25 +++++++++++++-------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index e7fdf532c5fe..87d9227f6ecc 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -2184,6 +2184,7 @@ atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
 	if (ret) {
 		dev_err(dev, "Failed to enable the HSMC clock (err = %d)\n",
 			ret);
+		clk_put(nc->clk);
 		goto out;
 	}
 
@@ -2193,20 +2194,20 @@ atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
 		if (ret != -EPROBE_DEFER)
 			dev_err(dev, "Failed to get IRQ number (err = %d)\n",
 				ret);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	ret = of_address_to_resource(nfc_np, 0, &res);
 	if (ret) {
 		dev_err(dev, "Invalid or missing NFC IO resource (err = %d)\n",
 			ret);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	iomem = devm_ioremap_resource(dev, &res);
 	if (IS_ERR(iomem)) {
 		ret = PTR_ERR(iomem);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	regmap_conf.name = "nfc-io";
@@ -2216,20 +2217,20 @@ atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
 		ret = PTR_ERR(nc->io);
 		dev_err(dev, "Could not create NFC IO regmap (err = %d)\n",
 			ret);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	ret = of_address_to_resource(nfc_np, 1, &res);
 	if (ret) {
 		dev_err(dev, "Invalid or missing HSMC resource (err = %d)\n",
 			ret);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	iomem = devm_ioremap_resource(dev, &res);
 	if (IS_ERR(iomem)) {
 		ret = PTR_ERR(iomem);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	regmap_conf.name = "smc";
@@ -2239,20 +2240,20 @@ atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
 		ret = PTR_ERR(nc->base.smc);
 		dev_err(dev, "Could not create NFC IO regmap (err = %d)\n",
 			ret);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	ret = of_address_to_resource(nfc_np, 2, &res);
 	if (ret) {
 		dev_err(dev, "Invalid or missing SRAM resource (err = %d)\n",
 			ret);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	nc->sram.virt = devm_ioremap_resource(dev, &res);
 	if (IS_ERR(nc->sram.virt)) {
 		ret = PTR_ERR(nc->sram.virt);
-		goto out;
+		goto err_disable_clk;
 	}
 
 	nc->sram.dma = res.start;
@@ -2261,6 +2262,12 @@ atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
 	of_node_put(nfc_np);
 
 	return ret;
+
+err_disable_clk:
+	clk_disable_unprepare(nc->clk);
+	clk_put(nc->clk);
+
+	goto out;
 }
 
 static int
-- 
2.34.1

Re: [PATCH] mtd: rawnand: atmel: Fix HSMC clock leak in legacy controller init

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mtd: backward goto in error handling path violates kernel style
--

commit aad96ec48f9b270598811f4065826c70f378687c
Author: Wentao Liang <vulab@iscas.ac.cn>

mtd: rawnand: atmel: Fix HSMC clock leak in legacy controller init

atmel_hsmc_nand_controller_legacy_init() takes a reference to the
HSMC clock with of_clk_get() and enables it, but the error paths
that follow only release the device node, so both the reference and
the enable are leaked whenever the controller cannot be fully
initialized. Add an err_disable_clk path that disables and releases
the clock before jumping to the existing out path.

> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 8a19408abb635..bafc8c995e376 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
[ ... ]
> @@ -2260,6 +2261,12 @@ atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
>  	of_node_put(nfc_np);
>  
>  	return ret;
> +
> +err_disable_clk:
> +	clk_disable_unprepare(nc->clk);
> +	clk_put(nc->clk);
> +
> +	goto out;

[Severity: Low]
Does this backward jump to the out label introduce a regression in standard
kernel conventions for error handling?

While functionally correct, jumping backwards instead of structuring cascading
error labels so that the success path exits or jumps forward over the error
handling blocks introduces non-standard control flow in this function.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917102804.2146887-1-vulab@iscas.ac.cn?part=1