[PATCH] nvmem: core: Fix memleak in nvmem_add_cells_from_dt error path

[PATCH] nvmem: core: Fix memleak in nvmem_add_cells_from_dt error path

[Ruihai Zhou]

In the nvmem_add_cells_from_dt(), if the devicetree 'bits' property
is outside of the valid range, the info.name alloc by kasprintf will
cause memleak. Just free before return from nvmem_add_cells_from_dt
in the error path.

Fixes: def3173d4f17 ("nvmem: core: Print error on wrong bits DT property")
Signed-off-by: Ruihai Zhou <zhou.ruihai@qq.com>
---
 drivers/nvmem/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 516dfd861b9f..ce71bee5e1a9 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -826,6 +826,7 @@ static int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_nod
 			info.nbits = be32_to_cpup(addr);
 			if (info.bit_offset >= BITS_PER_BYTE || info.nbits < 1) {
 				dev_err(dev, "nvmem: invalid bits on %pOF\n", child);
+				kfree(info.name);
 				of_node_put(child);
 				return -EINVAL;
 			}
-- 
2.31.1

Re: [PATCH] nvmem: core: Fix memleak in nvmem_add_cells_from_dt error path

[Greg KH]

On Wed, Sep 11, 2024 at 08:06:55PM +0800, Ruihai Zhou wrote:
> In the nvmem_add_cells_from_dt(), if the devicetree 'bits' property
> is outside of the valid range, the info.name alloc by kasprintf will
> cause memleak. Just free before return from nvmem_add_cells_from_dt
> in the error path.
> 
> Fixes: def3173d4f17 ("nvmem: core: Print error on wrong bits DT property")
> Signed-off-by: Ruihai Zhou <zhou.ruihai@qq.com>
> ---
>  drivers/nvmem/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 516dfd861b9f..ce71bee5e1a9 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -826,6 +826,7 @@ static int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_nod
>  			info.nbits = be32_to_cpup(addr);
>  			if (info.bit_offset >= BITS_PER_BYTE || info.nbits < 1) {
>  				dev_err(dev, "nvmem: invalid bits on %pOF\n", child);
> +				kfree(info.name);
>  				of_node_put(child);
>  				return -EINVAL;
>  			}
> -- 
> 2.31.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] nvmem: core: Fix memleak in nvmem_add_cells_from_dt error path

[Markus Elfring]

…
> is outside of the valid range, the info.name alloc by kasprintf will

                                               allocated by kasprintf()?


> cause memleak. Just free before return from nvmem_add_cells_from_dt
…

        a memory leak?                                               ()?


How do you think about to increase the application of scope-based resource management?

Regards,
Markus

Re: [PATCH] nvmem: core: Fix memleak in nvmem_add_cells_from_dt error path

[Greg Kroah-Hartman]

On Wed, Sep 11, 2024 at 03:05:20PM +0200, Markus Elfring wrote:
> …
> > is outside of the valid range, the info.name alloc by kasprintf will
> 
>                                                allocated by kasprintf()?
> 
> 
> > cause memleak. Just free before return from nvmem_add_cells_from_dt
> …
> 
>         a memory leak?                                               ()?
> 
> 
> How do you think about to increase the application of scope-based resource management?
> 
> Regards,
> Markus


Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

Re: [PATCH] nvmem: core: Fix memleak in nvmem_add_cells_from_dt error path

[Markus Schneider-Pargmann]

On Wed, Sep 11, 2024 at 08:06:55PM GMT, Ruihai Zhou wrote:
> In the nvmem_add_cells_from_dt(), if the devicetree 'bits' property
> is outside of the valid range, the info.name alloc by kasprintf will
> cause memleak. Just free before return from nvmem_add_cells_from_dt
> in the error path.

Thanks for spotting and fixing this.

Alternative to addding a kfree() I think the info.name allocation can be
moved down behind the if-block so it can't leak when returning inside of
the nested if block.

Both is fine for me.

Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>

Best
Markus

> 
> Fixes: def3173d4f17 ("nvmem: core: Print error on wrong bits DT property")
> Signed-off-by: Ruihai Zhou <zhou.ruihai@qq.com>
> ---
>  drivers/nvmem/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 516dfd861b9f..ce71bee5e1a9 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -826,6 +826,7 @@ static int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_nod
>  			info.nbits = be32_to_cpup(addr);
>  			if (info.bit_offset >= BITS_PER_BYTE || info.nbits < 1) {
>  				dev_err(dev, "nvmem: invalid bits on %pOF\n", child);
> +				kfree(info.name);
>  				of_node_put(child);
>  				return -EINVAL;
>  			}
> -- 
> 2.31.1
>