diff mbox series

[next,2/2] mtd: rawnand: mtk: Fix init error path

Message ID 20240826153019.67106-2-miquel.raynal@bootlin.com (mailing list archive)
State New, archived
Headers show
Series [next,1/2] mtd: rawnand: mtk: Factorize out the logic cleaning mtk chips | expand

Commit Message

Miquel Raynal Aug. 26, 2024, 3:30 p.m. UTC
Reviewing a series converting the for_each_chil_of_node() loops into
their _scoped variants made me realize there was no cleanup of the
already registered NAND devices upon error which may leak memory on
systems with more than a chip when this error occurs. We should call the
_nand_chips_cleanup() function when this happens.

Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Backporting this change will probably fail. In practice, the memory leak
is very unlikely to happen so I guess we can live without it.
---
 drivers/mtd/nand/raw/mtk_nand.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Pratyush Yadav Sept. 4, 2024, 2:08 p.m. UTC | #1
On Mon, Aug 26 2024, Miquel Raynal wrote:

> Reviewing a series converting the for_each_chil_of_node() loops into
> their _scoped variants made me realize there was no cleanup of the
> already registered NAND devices upon error which may leak memory on
> systems with more than a chip when this error occurs. We should call the
> _nand_chips_cleanup() function when this happens.
>
> Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Did a quick scan. Can't see anything (obvious) else being leaked on
error paths.

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
Miquel Raynal Sept. 4, 2024, 2:25 p.m. UTC | #2
Hi Pratyush,

pratyush@kernel.org wrote on Wed, 04 Sep 2024 16:08:15 +0200:

> On Mon, Aug 26 2024, Miquel Raynal wrote:
> 
> > Reviewing a series converting the for_each_chil_of_node() loops into
> > their _scoped variants made me realize there was no cleanup of the
> > already registered NAND devices upon error which may leak memory on
> > systems with more than a chip when this error occurs. We should call the
> > _nand_chips_cleanup() function when this happens.
> >
> > Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> Did a quick scan. Can't see anything (obvious) else being leaked on
> error paths.

Great! Thanks a lot for the reviews!

> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Cheers,
Miquèl
Matthias Brugger Sept. 6, 2024, 3:01 p.m. UTC | #3
On 26/08/2024 17:30, Miquel Raynal wrote:
> Reviewing a series converting the for_each_chil_of_node() loops into
> their _scoped variants made me realize there was no cleanup of the
> already registered NAND devices upon error which may leak memory on
> systems with more than a chip when this error occurs. We should call the
> _nand_chips_cleanup() function when this happens.
> 
> Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Matthias Brugger <matthias.bgg@kernel.org>

> ---
> Backporting this change will probably fail. In practice, the memory leak
> is very unlikely to happen so I guess we can live without it.
> ---
>   drivers/mtd/nand/raw/mtk_nand.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index bf845dd16737..586868b4139f 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -1453,8 +1453,10 @@ static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
>   
>   	for_each_child_of_node_scoped(np, nand_np) {
>   		ret = mtk_nfc_nand_chip_init(dev, nfc, nand_np);
> -		if (ret)
> +		if (ret) {
> +			mtk_nfc_nand_chips_cleanup(nfc);
>   			return ret;
> +		}
>   	}
>   
>   	return 0;
Miquel Raynal Sept. 6, 2024, 3:02 p.m. UTC | #4
On Mon, 2024-08-26 at 15:30:19 UTC, Miquel Raynal wrote:
> Reviewing a series converting the for_each_chil_of_node() loops into
> their _scoped variants made me realize there was no cleanup of the
> already registered NAND devices upon error which may leak memory on
> systems with more than a chip when this error occurs. We should call the
> _nand_chips_cleanup() function when this happens.
> 
> Fixes: Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next.

Miquel
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index bf845dd16737..586868b4139f 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -1453,8 +1453,10 @@  static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
 
 	for_each_child_of_node_scoped(np, nand_np) {
 		ret = mtk_nfc_nand_chip_init(dev, nfc, nand_np);
-		if (ret)
+		if (ret) {
+			mtk_nfc_nand_chips_cleanup(nfc);
 			return ret;
+		}
 	}
 
 	return 0;