[U-Boot] [PATCH V2] pinctrl: imx: fix memory leak

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; }

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");
return -EINVAL; }devm_kfree(dev, pin_data);
@@ -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

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");
return -EINVAL; }devm_kfree(dev, pin_data);
@@ -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
participants (3)
-
Lothar Waßmann
-
Peng Fan
-
Stefano Babic