Messages in this thread | | | Date | Thu, 9 Dec 2021 09:23:47 +0100 | From | Simon Horman <> | Subject | Re: [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 >
| |