diff mbox series

[net] soreuseport: fix mem leak in reuseport_add_sock()

Message ID 1517596047.3715.135.camel@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series [net] soreuseport: fix mem leak in reuseport_add_sock() | expand

Commit Message

Eric Dumazet Feb. 2, 2018, 6:27 p.m. UTC
From: Eric Dumazet <edumazet@google.com>

reuseport_add_sock() needs to deal with attaching a socket having
its own sk_reuseport_cb, after a prior
setsockopt(SO_ATTACH_REUSEPORT_?BPF)

Without this fix, not only a WARN_ONCE() was issued, but we were also
leaking memory.

Thanks to sysbot and Eric Biggers for providing us nice C repros. 

------------[ cut here ]------------
socket already in reuseport group
WARNING: CPU: 0 PID: 3496 at net/core/sock_reuseport.c:119  
reuseport_add_sock+0x742/0x9b0 net/core/sock_reuseport.c:117
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 3496 Comm: syzkaller869503 Not tainted 4.15.0-rc6+ #245
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:17 [inline]
  dump_stack+0x194/0x257 lib/dump_stack.c:53
  panic+0x1e4/0x41c kernel/panic.c:183
  __warn+0x1dc/0x200 kernel/panic.c:547
  report_bug+0x211/0x2d0 lib/bug.c:184
  fixup_bug.part.11+0x37/0x80 arch/x86/kernel/traps.c:178
  fixup_bug arch/x86/kernel/traps.c:247 [inline]
  do_error_trap+0x2d7/0x3e0 arch/x86/kernel/traps.c:296
  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
  invalid_op+0x22/0x40 arch/x86/entry/entry_64.S:1079

Fixes: ef456144da8e ("soreuseport: define reuseport groups")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot+c0ea2226f77a42936bf7@syzkaller.appspotmail.com
---
 net/core/sock_reuseport.c |   35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

Comments

Craig Gallek Feb. 2, 2018, 6:57 p.m. UTC | #1
On Fri, Feb 2, 2018 at 1:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> reuseport_add_sock() needs to deal with attaching a socket having
> its own sk_reuseport_cb, after a prior
> setsockopt(SO_ATTACH_REUSEPORT_?BPF)
>
> Without this fix, not only a WARN_ONCE() was issued, but we were also
> leaking memory.
>
> Thanks to sysbot and Eric Biggers for providing us nice C repros.
>
> ------------[ cut here ]------------
> socket already in reuseport group
> WARNING: CPU: 0 PID: 3496 at net/core/sock_reuseport.c:119
> reuseport_add_sock+0x742/0x9b0 net/core/sock_reuseport.c:117
> Kernel panic - not syncing: panic_on_warn set ...
>
> CPU: 0 PID: 3496 Comm: syzkaller869503 Not tainted 4.15.0-rc6+ #245
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS
> Google 01/01/2011
> Call Trace:
>   __dump_stack lib/dump_stack.c:17 [inline]
>   dump_stack+0x194/0x257 lib/dump_stack.c:53
>   panic+0x1e4/0x41c kernel/panic.c:183
>   __warn+0x1dc/0x200 kernel/panic.c:547
>   report_bug+0x211/0x2d0 lib/bug.c:184
>   fixup_bug.part.11+0x37/0x80 arch/x86/kernel/traps.c:178
>   fixup_bug arch/x86/kernel/traps.c:247 [inline]
>   do_error_trap+0x2d7/0x3e0 arch/x86/kernel/traps.c:296
>   do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
>   invalid_op+0x22/0x40 arch/x86/entry/entry_64.S:1079
>
> Fixes: ef456144da8e ("soreuseport: define reuseport groups")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot+c0ea2226f77a42936bf7@syzkaller.appspotmail.com

Clever fix, thanks Eric(s)!
Acked-by: Craig Gallek <kraig@google.com>
David Miller Feb. 3, 2018, 12:47 a.m. UTC | #2
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 02 Feb 2018 10:27:27 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> reuseport_add_sock() needs to deal with attaching a socket having
> its own sk_reuseport_cb, after a prior
> setsockopt(SO_ATTACH_REUSEPORT_?BPF)
> 
> Without this fix, not only a WARN_ONCE() was issued, but we were also
> leaking memory.
> 
> Thanks to sysbot and Eric Biggers for providing us nice C repros. 
 ...
> Fixes: ef456144da8e ("soreuseport: define reuseport groups")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot+c0ea2226f77a42936bf7@syzkaller.appspotmail.com

Applied, thanks.
diff mbox series

Patch

diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index c5bb52bc73a18689c358389b5f1c89cf6a2402a2..064acb04be0f73ff48cf1b4f340476793eceb5cd 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -94,6 +94,16 @@  static struct sock_reuseport *reuseport_grow(struct sock_reuseport *reuse)
 	return more_reuse;
 }
 
+static void reuseport_free_rcu(struct rcu_head *head)
+{
+	struct sock_reuseport *reuse;
+
+	reuse = container_of(head, struct sock_reuseport, rcu);
+	if (reuse->prog)
+		bpf_prog_destroy(reuse->prog);
+	kfree(reuse);
+}
+
 /**
  *  reuseport_add_sock - Add a socket to the reuseport group of another.
  *  @sk:  New socket to add to the group.
@@ -102,7 +112,7 @@  static struct sock_reuseport *reuseport_grow(struct sock_reuseport *reuse)
  */
 int reuseport_add_sock(struct sock *sk, struct sock *sk2)
 {
-	struct sock_reuseport *reuse;
+	struct sock_reuseport *old_reuse, *reuse;
 
 	if (!rcu_access_pointer(sk2->sk_reuseport_cb)) {
 		int err = reuseport_alloc(sk2);
@@ -113,10 +123,13 @@  int reuseport_add_sock(struct sock *sk, struct sock *sk2)
 
 	spin_lock_bh(&reuseport_lock);
 	reuse = rcu_dereference_protected(sk2->sk_reuseport_cb,
-					  lockdep_is_held(&reuseport_lock)),
-	WARN_ONCE(rcu_dereference_protected(sk->sk_reuseport_cb,
-					    lockdep_is_held(&reuseport_lock)),
-		  "socket already in reuseport group");
+					  lockdep_is_held(&reuseport_lock));
+	old_reuse = rcu_dereference_protected(sk->sk_reuseport_cb,
+					     lockdep_is_held(&reuseport_lock));
+	if (old_reuse && old_reuse->num_socks != 1) {
+		spin_unlock_bh(&reuseport_lock);
+		return -EBUSY;
+	}
 
 	if (reuse->num_socks == reuse->max_socks) {
 		reuse = reuseport_grow(reuse);
@@ -134,19 +147,11 @@  int reuseport_add_sock(struct sock *sk, struct sock *sk2)
 
 	spin_unlock_bh(&reuseport_lock);
 
+	if (old_reuse)
+		call_rcu(&old_reuse->rcu, reuseport_free_rcu);
 	return 0;
 }
 
-static void reuseport_free_rcu(struct rcu_head *head)
-{
-	struct sock_reuseport *reuse;
-
-	reuse = container_of(head, struct sock_reuseport, rcu);
-	if (reuse->prog)
-		bpf_prog_destroy(reuse->prog);
-	kfree(reuse);
-}
-
 void reuseport_detach_sock(struct sock *sk)
 {
 	struct sock_reuseport *reuse;