[PATCH net] gianfar: fix memory leak in gfar_parse_group error paths

[PATCH net] gianfar: fix memory leak in gfar_parse_group error paths

[Rosen Penev]

In gfar_parse_group, if any allocation in the irqinfo[] loop fails,
or if of_iomap fails, or if the IRQ parsing fails, the already-
allocated irqinfo[] entries and mapped regs are not freed.  The
caller's free_gfar_dev cannot clean them up because priv->num_grps
is only incremented after a successful return.

Fix by adding an err_out label that frees all successfully allocated
irqinfo entries and unmaps regs if mapped.  Track the number of
allocated entries with 'allocated' and propagate the correct error
code.

Fixes: 46ceb60ca80fa ("gianfar: Add Multiple group Support")
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 3271de5844f84..12ebb207be0be 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -502,17 +502,19 @@ static int gfar_parse_group(struct device_node *np,
 			    struct gfar_private *priv, const char *model)
 {
 	struct gfar_priv_grp *grp = &priv->gfargrp[priv->num_grps];
-	int i;
+	int err = -ENOMEM;
+	int i, allocated = 0;
 
 	for (i = 0; i < GFAR_NUM_IRQS; i++) {
 		grp->irqinfo[i] = kzalloc_obj(struct gfar_irqinfo);
 		if (!grp->irqinfo[i])
-			return -ENOMEM;
+			goto err_out;
+		allocated++;
 	}
 
 	grp->regs = of_iomap(np, 0);
 	if (!grp->regs)
-		return -ENOMEM;
+		goto err_out;
 
 	gfar_irq(grp, TX)->irq = irq_of_parse_and_map(np, 0);
 
@@ -522,8 +524,10 @@ static int gfar_parse_group(struct device_node *np,
 		gfar_irq(grp, ER)->irq = irq_of_parse_and_map(np, 2);
 		if (!gfar_irq(grp, TX)->irq ||
 		    !gfar_irq(grp, RX)->irq ||
-		    !gfar_irq(grp, ER)->irq)
-			return -EINVAL;
+		    !gfar_irq(grp, ER)->irq) {
+			err = -EINVAL;
+			goto err_out;
+		}
 	}
 
 	grp->priv = priv;
@@ -567,6 +571,13 @@ static int gfar_parse_group(struct device_node *np,
 	priv->num_grps++;
 
 	return 0;
+
+err_out:
+	for (i = 0; i < allocated; i++)
+		kfree(grp->irqinfo[i]);
+	if (grp->regs)
+		iounmap(grp->regs);
+	return err;
 }
 
 /* Reads the controller's registers to determine what interface
-- 
2.54.0

Re: [PATCH net] gianfar: fix memory leak in gfar_parse_group error paths

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 3204 bytes --]

On Mon, Jun 22, 2026 at 7:49 AM Rosen Penev <rosenp@gmail.com> wrote:
>
> In gfar_parse_group, if any allocation in the irqinfo[] loop fails,
> or if of_iomap fails, or if the IRQ parsing fails, the already-
> allocated irqinfo[] entries and mapped regs are not freed.  The
> caller's free_gfar_dev cannot clean them up because priv->num_grps
> is only incremented after a successful return.
>
> Fix by adding an err_out label that frees all successfully allocated
> irqinfo entries and unmaps regs if mapped.  Track the number of
> allocated entries with 'allocated' and propagate the correct error
> code.
>
> Fixes: 46ceb60ca80fa ("gianfar: Add Multiple group Support")
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/ethernet/freescale/gianfar.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index 3271de5844f84..12ebb207be0be 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -502,17 +502,19 @@ static int gfar_parse_group(struct device_node *np,
>                             struct gfar_private *priv, const char *model)
>  {
>         struct gfar_priv_grp *grp = &priv->gfargrp[priv->num_grps];
> -       int i;
> +       int err = -ENOMEM;
> +       int i, allocated = 0;

Declarations should follow reverse xmas-tree fashion.
BTW, why num_grps cannot be incremented quicker? Not sure it can cause
any issues, since all this is happening probe(), it should be safe to
do that?
That way you could avoid cleaning up at two different places...

>
>         for (i = 0; i < GFAR_NUM_IRQS; i++) {
>                 grp->irqinfo[i] = kzalloc_obj(struct gfar_irqinfo);
>                 if (!grp->irqinfo[i])
> -                       return -ENOMEM;
> +                       goto err_out;
> +               allocated++;
>         }
>
>         grp->regs = of_iomap(np, 0);
>         if (!grp->regs)
> -               return -ENOMEM;
> +               goto err_out;
>
>         gfar_irq(grp, TX)->irq = irq_of_parse_and_map(np, 0);
>
> @@ -522,8 +524,10 @@ static int gfar_parse_group(struct device_node *np,
>                 gfar_irq(grp, ER)->irq = irq_of_parse_and_map(np, 2);
>                 if (!gfar_irq(grp, TX)->irq ||
>                     !gfar_irq(grp, RX)->irq ||
> -                   !gfar_irq(grp, ER)->irq)
> -                       return -EINVAL;
> +                   !gfar_irq(grp, ER)->irq) {
> +                       err = -EINVAL;
> +                       goto err_out;
> +               }
>         }
>
>         grp->priv = priv;
> @@ -567,6 +571,13 @@ static int gfar_parse_group(struct device_node *np,
>         priv->num_grps++;
>
>         return 0;
> +
> +err_out:
> +       for (i = 0; i < allocated; i++)
> +               kfree(grp->irqinfo[i]);
> +       if (grp->regs)
> +               iounmap(grp->regs);
> +       return err;
>  }
>
>  /* Reads the controller's registers to determine what interface
> --
> 2.54.0
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]