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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
> ---
> 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
>
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
On Thu, 9 Dec 2021 14:15:11 +0800 you 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).
>
> [...]
Here is the summary with links:
- nfp: Fix memory leak in nfp_cpp_area_cache_add()
https://git.kernel.org/netdev/net/c/c56c96303e92
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html