public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/2] rtnetlink: Fix small memory leaks
@ 2025-02-12  8:23 Bastien Curutchet (eBPF Foundation)
  2025-02-12  8:23 ` [PATCH net 1/2] rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off Bastien Curutchet (eBPF Foundation)
  2025-02-12  8:23 ` [PATCH net 2/2] rtnetlink: Release nets when leaving rtnl_setlink() Bastien Curutchet (eBPF Foundation)
  0 siblings, 2 replies; 7+ messages in thread
From: Bastien Curutchet (eBPF Foundation) @ 2025-02-12  8:23 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Nikolay Aleksandrov, Kuniyuki Iwashima
  Cc: Alexis Lothore, Thomas Petazzoni, netdev, linux-kernel,
	Bastien Curutchet (eBPF Foundation), stable

Hi all,

I ran into a small memory leak while working on the BPF selftests suite
on the bpf-next tree. It leads to an oom-kill after thousands of iterations
in the qemu environment provided by tools/testing/selftests/bpf/vmtest.sh.

To reproduce the issue from the net-next tree:
$ git remote add bpf-next https://github.com/kernel-patches/bpf
$ git fetch bpf-next
$ git cherry-pick 723f1b9ce332^..edb996fae276
$ tools/testing/selftests/bpf/vmtest.sh -i -s "bash -c 'for i in {1..8192}; do ./test_progs -t xdp_veth_redirect; done'"
[... coffee break ...]
[ XXXX.YYYYYY] sh invoked oom-killer: gfp_mask=0x440dc0(GFP_KERNEL_ACCOUNT|__GFP_COMP|__GFP_ZERO), order=0, oom_score_adj=0
[...]
[ XXXX.YYYYYY] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),cpuset=/,mems_allowed=0,global_oom,task_memcg=/,task=bash,pid=116,uid=0
[ XXXX.YYYYYY] Out of memory: Killed process 116 (bash) total-vm:6816kB, anon-rss:2816kB, file-rss:240kB, shmem-rss:0kB, UID:0 pgtables:48kB oom_score_adj:0

Signed-off-by: Bastien Curutchet (eBPF Foundation) <bastien.curutchet@bootlin.com>
---
Bastien Curutchet (eBPF Foundation) (2):
      rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off
      rtnetlink: Release nets when leaving rtnl_setlink()

 net/core/rtnetlink.c | 4 ++++
 1 file changed, 4 insertions(+)
---
base-commit: 5d332c1ad3226c0a31653dbf2391bd332e157625
change-id: 20250211-rtnetlink_leak-8b12ca5c83f3

Best regards,
-- 
Bastien Curutchet (eBPF Foundation) <bastien.curutchet@bootlin.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net 1/2] rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off
  2025-02-12  8:23 [PATCH net 0/2] rtnetlink: Fix small memory leaks Bastien Curutchet (eBPF Foundation)
@ 2025-02-12  8:23 ` Bastien Curutchet (eBPF Foundation)
  2025-02-12  8:45   ` Kuniyuki Iwashima
  2025-02-12  8:23 ` [PATCH net 2/2] rtnetlink: Release nets when leaving rtnl_setlink() Bastien Curutchet (eBPF Foundation)
  1 sibling, 1 reply; 7+ messages in thread
From: Bastien Curutchet (eBPF Foundation) @ 2025-02-12  8:23 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Nikolay Aleksandrov, Kuniyuki Iwashima
  Cc: Alexis Lothore, Thomas Petazzoni, netdev, linux-kernel,
	Bastien Curutchet (eBPF Foundation), stable

rtnl_net_cmp_locks() always returns -1 if CONFIG_DEBUG_NET_SMALL_RTNL is
disabled. However, if CONFIG_DEBUG_NET_SMALL_RTNL is enabled, it returns 0
when both inputs are equal. It is then used by rtnl_nets_add() to call
put_net() if the net to be added is already present in the struct
rtnl_nets. As a result, when rtnl_nets_add() is called on an already
present net, put_net() is called only if DEBUG is on.

Add the input comparison in the DEBUG off case so that put_net() is always
called in this scenario.

Fixes: cbaaa6326bc5 ("rtnetlink: Introduce struct rtnl_nets and helpers.")
Cc: stable@vger.kernel.org
Signed-off-by: Bastien Curutchet (eBPF Foundation) <bastien.curutchet@bootlin.com>
---
 net/core/rtnetlink.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index cb7fad8d1f95ff287810229c341de6a6d20a9c07..94111d3383788566f2296039e68549e2b40d5a4a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -275,6 +275,9 @@ EXPORT_SYMBOL(lockdep_rtnl_net_is_held);
 #else
 static int rtnl_net_cmp_locks(const struct net *net_a, const struct net *net_b)
 {
+	if (net_eq(net_a, net_b))
+		return 0;
+
 	/* No need to swap */
 	return -1;
 }

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 2/2] rtnetlink: Release nets when leaving rtnl_setlink()
  2025-02-12  8:23 [PATCH net 0/2] rtnetlink: Fix small memory leaks Bastien Curutchet (eBPF Foundation)
  2025-02-12  8:23 ` [PATCH net 1/2] rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off Bastien Curutchet (eBPF Foundation)
@ 2025-02-12  8:23 ` Bastien Curutchet (eBPF Foundation)
  2025-02-12  8:31   ` Kuniyuki Iwashima
  1 sibling, 1 reply; 7+ messages in thread
From: Bastien Curutchet (eBPF Foundation) @ 2025-02-12  8:23 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Nikolay Aleksandrov, Kuniyuki Iwashima
  Cc: Alexis Lothore, Thomas Petazzoni, netdev, linux-kernel,
	Bastien Curutchet (eBPF Foundation), stable

rtnl_setlink() uses the rtnl_nets_* helpers but never calls the
rtnl_nets_destroy(). It leads to small memory leaks.

Call rtnl_nets_destroy() before exiting to properly decrement the nets'
reference counters.

Fixes: 636af13f213b ("rtnetlink: Register rtnl_dellink() and rtnl_setlink() with RTNL_FLAG_DOIT_PERNET_WIP.")
Cc: stable@vger.kernel.org
Signed-off-by: Bastien Curutchet (eBPF Foundation) <bastien.curutchet@bootlin.com>
---
 net/core/rtnetlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 94111d3383788566f2296039e68549e2b40d5a4a..e4ac14c081a48e36f5381e025a3991c90827c2bf 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3441,6 +3441,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	rtnl_nets_unlock(&rtnl_nets);
 errout:
+	rtnl_nets_destroy(&rtnl_nets);
 	return err;
 }
 

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/2] rtnetlink: Release nets when leaving rtnl_setlink()
  2025-02-12  8:23 ` [PATCH net 2/2] rtnetlink: Release nets when leaving rtnl_setlink() Bastien Curutchet (eBPF Foundation)
@ 2025-02-12  8:31   ` Kuniyuki Iwashima
  2025-02-12  9:05     ` Bastien Curutchet
  0 siblings, 1 reply; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-02-12  8:31 UTC (permalink / raw)
  To: bastien.curutchet
  Cc: alexis.lothore, davem, edumazet, horms, kuba, kuniyu,
	linux-kernel, netdev, pabeni, razor, stable, thomas.petazzoni

From: "Bastien Curutchet (eBPF Foundation)" <bastien.curutchet@bootlin.com>
Date: Wed, 12 Feb 2025 09:23:48 +0100
> rtnl_setlink() uses the rtnl_nets_* helpers but never calls the
> rtnl_nets_destroy(). It leads to small memory leaks.
> 
> Call rtnl_nets_destroy() before exiting to properly decrement the nets'
> reference counters.
> 
> Fixes: 636af13f213b ("rtnetlink: Register rtnl_dellink() and rtnl_setlink() with RTNL_FLAG_DOIT_PERNET_WIP.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bastien Curutchet (eBPF Foundation) <bastien.curutchet@bootlin.com>

It's fixed in 1438f5d07b9a ("rtnetlink: fix netns leak with
rtnl_setlink()").

Thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 1/2] rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off
  2025-02-12  8:23 ` [PATCH net 1/2] rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off Bastien Curutchet (eBPF Foundation)
@ 2025-02-12  8:45   ` Kuniyuki Iwashima
  2025-02-12  9:07     ` Bastien Curutchet
  0 siblings, 1 reply; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-02-12  8:45 UTC (permalink / raw)
  To: bastien.curutchet
  Cc: alexis.lothore, davem, edumazet, horms, kuba, kuniyu,
	linux-kernel, netdev, pabeni, razor, stable, thomas.petazzoni

From: "Bastien Curutchet (eBPF Foundation)" <bastien.curutchet@bootlin.com>
Date: Wed, 12 Feb 2025 09:23:47 +0100
> rtnl_net_cmp_locks() always returns -1 if CONFIG_DEBUG_NET_SMALL_RTNL is
> disabled. However, if CONFIG_DEBUG_NET_SMALL_RTNL is enabled, it returns 0
> when both inputs are equal. It is then used by rtnl_nets_add() to call
> put_net() if the net to be added is already present in the struct
> rtnl_nets. As a result, when rtnl_nets_add() is called on an already
> present net, put_net() is called only if DEBUG is on.

If CONFIG_DEBUG_NET_SMALL_RTNL is disabled, every duplicate net is
added to rtnl_nets, so put_net() is expected to be called for each
in rtnl_nets_destroy().

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/2] rtnetlink: Release nets when leaving rtnl_setlink()
  2025-02-12  8:31   ` Kuniyuki Iwashima
@ 2025-02-12  9:05     ` Bastien Curutchet
  0 siblings, 0 replies; 7+ messages in thread
From: Bastien Curutchet @ 2025-02-12  9:05 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: alexis.lothore, davem, edumazet, horms, kuba, linux-kernel,
	netdev, pabeni, razor, stable, thomas.petazzoni

On 2/12/25 9:31 AM, Kuniyuki Iwashima wrote:
> From: "Bastien Curutchet (eBPF Foundation)" <bastien.curutchet@bootlin.com>
> Date: Wed, 12 Feb 2025 09:23:48 +0100
>> rtnl_setlink() uses the rtnl_nets_* helpers but never calls the
>> rtnl_nets_destroy(). It leads to small memory leaks.
>>
>> Call rtnl_nets_destroy() before exiting to properly decrement the nets'
>> reference counters.
>>
>> Fixes: 636af13f213b ("rtnetlink: Register rtnl_dellink() and rtnl_setlink() with RTNL_FLAG_DOIT_PERNET_WIP.")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bastien Curutchet (eBPF Foundation) <bastien.curutchet@bootlin.com>
> 
> It's fixed in 1438f5d07b9a ("rtnetlink: fix netns leak with
> rtnl_setlink()").
> 

Oops, I missed it, sorry about that.

Best regards,
Bastien

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 1/2] rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off
  2025-02-12  8:45   ` Kuniyuki Iwashima
@ 2025-02-12  9:07     ` Bastien Curutchet
  0 siblings, 0 replies; 7+ messages in thread
From: Bastien Curutchet @ 2025-02-12  9:07 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: alexis.lothore, davem, edumazet, horms, kuba, linux-kernel,
	netdev, pabeni, razor, stable, thomas.petazzoni

On 2/12/25 9:45 AM, Kuniyuki Iwashima wrote:
> From: "Bastien Curutchet (eBPF Foundation)" <bastien.curutchet@bootlin.com>
> Date: Wed, 12 Feb 2025 09:23:47 +0100
>> rtnl_net_cmp_locks() always returns -1 if CONFIG_DEBUG_NET_SMALL_RTNL is
>> disabled. However, if CONFIG_DEBUG_NET_SMALL_RTNL is enabled, it returns 0
>> when both inputs are equal. It is then used by rtnl_nets_add() to call
>> put_net() if the net to be added is already present in the struct
>> rtnl_nets. As a result, when rtnl_nets_add() is called on an already
>> present net, put_net() is called only if DEBUG is on.
> 
> If CONFIG_DEBUG_NET_SMALL_RTNL is disabled, every duplicate net is
> added to rtnl_nets, so put_net() is expected to be called for each
> in rtnl_nets_destroy().

I see, sorry for the irrelevant series then ...

Best regards,
Bastien

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-02-12  9:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-12  8:23 [PATCH net 0/2] rtnetlink: Fix small memory leaks Bastien Curutchet (eBPF Foundation)
2025-02-12  8:23 ` [PATCH net 1/2] rtnetlink: Fix rtnl_net_cmp_locks() when DEBUG is off Bastien Curutchet (eBPF Foundation)
2025-02-12  8:45   ` Kuniyuki Iwashima
2025-02-12  9:07     ` Bastien Curutchet
2025-02-12  8:23 ` [PATCH net 2/2] rtnetlink: Release nets when leaving rtnl_setlink() Bastien Curutchet (eBPF Foundation)
2025-02-12  8:31   ` Kuniyuki Iwashima
2025-02-12  9:05     ` Bastien Curutchet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox