-----Original Message----- From: Lothar Waßmann [mailto:LW@KARO-electronics.de] Sent: Thursday, May 11, 2017 5:13 PM To: Peng Fan peng.fan@nxp.com Cc: sjg@chromium.org; u-boot@lists.denx.de; Stefan Agner stefan.agner@toradex.com Subject: Re: [U-Boot] [PATCH] pinctrl: imx: fix memory leak

Hi,

On Thu, 11 May 2017 10:43:02 +0800 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..31a28e1 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(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(pin_data);
  return -EINVAL;
  

  }

@@ -166,6 +168,8 @@ static int imx_pinctrl_set_state(struct udevice *dev,

struct udevice *config)

...

}

}

• devm_kfree(pin_data);
• return 0;

}

IMO 'devm'_kzalloc() doesn't make any sense here. It would make sense for allocations that have the same lifetime as the 'struct udevice', but not for a temporary buffer that is used within on function.

Size is different for different fsl,pins group. So need to use devm_kzalloc.

Thanks, Peng.

Lothar Waßmann