* [U-Boot] [PATCH V2] pinctrl: imx: fix memory leak
@ 2017-05-11 9:34 Peng Fan
2017-05-18 9:15 ` Stefano Babic
0 siblings, 1 reply; 3+ messages in thread
From: Peng Fan @ 2017-05-11 9:34 UTC (permalink / raw)
To: u-boot
Each time set_state is called, a new piece memory will
be allocated for pin_data, but not freed, this will
incur memory leak.
When error, the devm API could not free memory automatically.
So need call devm_kfree when error.
Issue reported by Coverity
Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Stefan Agner <stefan.agner@toradex.com>
Cc: Stefano Babic <sbabic@denx.de>
---
drivers/pinctrl/nxp/pinctrl-imx.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pinctrl/nxp/pinctrl-imx.c b/drivers/pinctrl/nxp/pinctrl-imx.c
index f0321c4..ebc14a3 100644
--- a/drivers/pinctrl/nxp/pinctrl-imx.c
+++ b/drivers/pinctrl/nxp/pinctrl-imx.c
@@ -53,6 +53,7 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
if (fdtdec_get_int_array(gd->fdt_blob, node, "fsl,pins",
pin_data, size >> 2)) {
dev_err(dev, "Error reading pin data.\n");
+ devm_kfree(dev, pin_data);
return -EINVAL;
}
@@ -78,6 +79,7 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
if ((mux_reg == -1) || (conf_reg == -1)) {
dev_err(dev, "Error mux_reg or conf_reg\n");
+ devm_kfree(dev, pin_data);
return -EINVAL;
}
@@ -166,6 +168,8 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
}
}
+ devm_kfree(dev, pin_data);
+
return 0;
}
--
2.6.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH V2] pinctrl: imx: fix memory leak
2017-05-11 9:34 [U-Boot] [PATCH V2] pinctrl: imx: fix memory leak Peng Fan
@ 2017-05-18 9:15 ` Stefano Babic
2017-05-18 10:20 ` Lothar Waßmann
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Babic @ 2017-05-18 9:15 UTC (permalink / raw)
To: u-boot
On 11/05/2017 11:34, Peng Fan wrote:
> Each time set_state is called, a new piece memory will
> be allocated for pin_data, but not freed, this will
> incur memory leak.
>
> When error, the devm API could not free memory automatically.
> So need call devm_kfree when error.
>
> Issue reported by Coverity
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefan Agner <stefan.agner@toradex.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
> drivers/pinctrl/nxp/pinctrl-imx.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/pinctrl/nxp/pinctrl-imx.c b/drivers/pinctrl/nxp/pinctrl-imx.c
> index f0321c4..ebc14a3 100644
> --- a/drivers/pinctrl/nxp/pinctrl-imx.c
> +++ b/drivers/pinctrl/nxp/pinctrl-imx.c
> @@ -53,6 +53,7 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> if (fdtdec_get_int_array(gd->fdt_blob, node, "fsl,pins",
> pin_data, size >> 2)) {
> dev_err(dev, "Error reading pin data.\n");
> + devm_kfree(dev, pin_data);
> return -EINVAL;
> }
>
> @@ -78,6 +79,7 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
>
> if ((mux_reg == -1) || (conf_reg == -1)) {
> dev_err(dev, "Error mux_reg or conf_reg\n");
> + devm_kfree(dev, pin_data);
> return -EINVAL;
> }
>
> @@ -166,6 +168,8 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> }
> }
>
> + devm_kfree(dev, pin_data);
> +
> return 0;
> }
>
>
Applied to u-boot-imx, thanks !
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH V2] pinctrl: imx: fix memory leak
2017-05-18 9:15 ` Stefano Babic
@ 2017-05-18 10:20 ` Lothar Waßmann
0 siblings, 0 replies; 3+ messages in thread
From: Lothar Waßmann @ 2017-05-18 10:20 UTC (permalink / raw)
To: u-boot
Hi,
On Thu, 18 May 2017 11:15:08 +0200 Stefano Babic wrote:
> On 11/05/2017 11:34, Peng Fan wrote:
> > Each time set_state is called, a new piece memory will
> > be allocated for pin_data, but not freed, this will
> > incur memory leak.
> >
> > When error, the devm API could not free memory automatically.
> > So need call devm_kfree when error.
> >
> > Issue reported by Coverity
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Stefan Agner <stefan.agner@toradex.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > ---
> > drivers/pinctrl/nxp/pinctrl-imx.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/pinctrl/nxp/pinctrl-imx.c b/drivers/pinctrl/nxp/pinctrl-imx.c
> > index f0321c4..ebc14a3 100644
> > --- a/drivers/pinctrl/nxp/pinctrl-imx.c
> > +++ b/drivers/pinctrl/nxp/pinctrl-imx.c
> > @@ -53,6 +53,7 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> > if (fdtdec_get_int_array(gd->fdt_blob, node, "fsl,pins",
> > pin_data, size >> 2)) {
> > dev_err(dev, "Error reading pin data.\n");
> > + devm_kfree(dev, pin_data);
> > return -EINVAL;
> > }
> >
> > @@ -78,6 +79,7 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> >
> > if ((mux_reg == -1) || (conf_reg == -1)) {
> > dev_err(dev, "Error mux_reg or conf_reg\n");
> > + devm_kfree(dev, pin_data);
> > return -EINVAL;
> > }
> >
> > @@ -166,6 +168,8 @@ static int imx_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> > }
> > }
> >
> > + devm_kfree(dev, pin_data);
> > +
> > return 0;
> > }
> >
> >
>
> Applied to u-boot-imx, thanks !
>
I had commented on this patch, but unfortunately my mails were rejected
by the mailing list server, so the only reached the individuals on CC.
IMO the use of the 'devm_' functions is inappropriate here, since the
allocated buffer is only used temporarily inside the function and
unconditionally freed at function exit.
Lothar Waßmann
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-18 10:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11 9:34 [U-Boot] [PATCH V2] pinctrl: imx: fix memory leak Peng Fan
2017-05-18 9:15 ` Stefano Babic
2017-05-18 10:20 ` Lothar Waßmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.