lkml.org 
[lkml]   [2021]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] nfp: Fix memory leak in nfp_cpp_area_cache_add()
Hi Jianglei,

On Thu, Dec 09, 2021 at 02:15:11PM +0800, Jianglei Nie wrote:
> In line 800 (#1), nfp_cpp_area_alloc() allocates and initializes a
> CPP area structure. But in line 807 (#2), when the cache is allocated
> failed, this CPP area structure is not freed, which will result in
> memory leak.
>
> We can fix it by freeing the CPP area when the cache is allocated
> failed (#2).
>
> 792 int nfp_cpp_area_cache_add(struct nfp_cpp *cpp, size_t size)
> 793 {
> 794 struct nfp_cpp_area_cache *cache;
> 795 struct nfp_cpp_area *area;
>
> 800 area = nfp_cpp_area_alloc(cpp, NFP_CPP_ID(7, NFP_CPP_ACTION_RW, 0),
> 801 0, size);
> // #1: allocates and initializes
>
> 802 if (!area)
> 803 return -ENOMEM;
>
> 805 cache = kzalloc(sizeof(*cache), GFP_KERNEL);
> 806 if (!cache)
> 807 return -ENOMEM; // #2: missing free
>
> 817 return 0;
> 818 }
>
> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>

Thanks for noticing this. I agree that this seems to be incorrect
and that your patch addresses the problem.

I do wonder if there is a value in adding:

Fixes: 4cb584e0ee7d ("nfp: add CPP access core")

Also, as I don't think this is hurting anything in practice, perhaps
this is for net-next (as oppoed to net), which is not specified
in the patch subject.

Regardless,

Acked-by: Simon Horman <simon.horman@corigine.com>

> ---
> drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c
> index d7ac0307797f..34c0d2ddf9ef 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c
> @@ -803,8 +803,10 @@ int nfp_cpp_area_cache_add(struct nfp_cpp *cpp, size_t size)
> return -ENOMEM;
>
> cache = kzalloc(sizeof(*cache), GFP_KERNEL);
> - if (!cache)
> + if (!cache) {
> + nfp_cpp_area_free(area);
> return -ENOMEM;
> + }
>
> cache->id = 0;
> cache->addr = 0;
> --
> 2.25.1
>

\
 
 \ /
  Last update: 2021-12-09 09:25    [W:0.086 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site