All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: netdev <netdev@vger.kernel.org>,
	Francesco Ruggeri <fruggeri@aristanetworks.com>,
	Willem de Bruijn <willemb@google.com>
Subject: [PATCH net] net/packet: fix a race in packet_bind() and packet_notifier()
Date: Mon, 27 Nov 2017 20:00:52 -0800	[thread overview]
Message-ID: <1511841652.16595.11.camel@gmail.com> (raw)

From: Eric Dumazet <edumazet@google.com>

syzbot reported crashes [1] and provided a C repro easing bug hunting.

When/if packet_do_bind() calls __unregister_prot_hook() and releases
po->bind_lock, another thread can run packet_notifier() and process an
NETDEV_UP event.

This calls register_prot_hook() and hook again the socket right before
first thread was able to grab again po->bind_lock.

Fixes this issue by adding po->frozen bit :

It is set and cleared by __unregister_prot_hook() if po->bind_lock
needs to be released temporarily.

It is tested in register_prot_hook() to prevent the race condition.

[1]
dev_remove_pack: ffff8801bf16fa80 not found
------------[ cut here ]------------
kernel BUG at net/core/dev.c:7945!  ( BUG_ON(!list_empty(&dev->ptype_all)); )
invalid opcode: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in:
device syz0 entered promiscuous mode
CPU: 0 PID: 3161 Comm: syzkaller404108 Not tainted 4.14.0+ #190
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
task: ffff8801cc57a500 task.stack: ffff8801cc588000
RIP: 0010:netdev_run_todo+0x772/0xae0 net/core/dev.c:7945
RSP: 0018:ffff8801cc58f598 EFLAGS: 00010293
RAX: ffff8801cc57a500 RBX: dffffc0000000000 RCX: ffffffff841f75b2
RDX: 0000000000000000 RSI: 1ffff100398b1ede RDI: ffff8801bf1f8810
device syz0 entered promiscuous mode
RBP: ffff8801cc58f898 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801bf1f8cd8
R13: ffff8801cc58f870 R14: ffff8801bf1f8780 R15: ffff8801cc58f7f0
FS:  0000000001716880(0000) GS:ffff8801db400000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020b13000 CR3: 0000000005e25000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 rtnl_unlock+0xe/0x10 net/core/rtnetlink.c:106
 tun_detach drivers/net/tun.c:670 [inline]
 tun_chr_close+0x49/0x60 drivers/net/tun.c:2845
 __fput+0x333/0x7f0 fs/file_table.c:210
 ____fput+0x15/0x20 fs/file_table.c:244
 task_work_run+0x199/0x270 kernel/task_work.c:113
 exit_task_work include/linux/task_work.h:22 [inline]
 do_exit+0x9bb/0x1ae0 kernel/exit.c:865
 do_group_exit+0x149/0x400 kernel/exit.c:968
 SYSC_exit_group kernel/exit.c:979 [inline]
 SyS_exit_group+0x1d/0x20 kernel/exit.c:977
 entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x44ad19


Fixes: 30f7ea1c2b5f ("packet: race condition in packet_bind")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Francesco Ruggeri <fruggeri@aristanetworks.com>
---
 net/packet/af_packet.c |    5 ++++-
 net/packet/internal.h  |    1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 737092ca9b4eed464b6c0907d85b679ae4da6046..64382200ab9b0701a510f16a098257ccd7ac5ff5 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -336,7 +336,7 @@ static void register_prot_hook(struct sock *sk)
 {
 	struct packet_sock *po = pkt_sk(sk);
 
-	if (!po->running) {
+	if (!po->running && !po->frozen) {
 		if (po->fanout)
 			__fanout_link(sk, po);
 		else
@@ -368,9 +368,11 @@ static void __unregister_prot_hook(struct sock *sk, bool sync)
 	__sock_put(sk);
 
 	if (sync) {
+		po->frozen = 1;
 		spin_unlock(&po->bind_lock);
 		synchronize_net();
 		spin_lock(&po->bind_lock);
+		po->frozen = 0;
 	}
 }
 
@@ -3105,6 +3107,7 @@ static int packet_do_bind(struct sock *sk, const char *name, int ifindex,
 								 dev->ifindex);
 		}
 
+		BUG_ON(po->running);
 		po->num = proto;
 		po->prot_hook.type = proto;
 
diff --git a/net/packet/internal.h b/net/packet/internal.h
index 562fbc155006374862e5bfdd78b65a7f46210bea..e039af6e71d650e3beb2936af6cbdd167313499e 100644
--- a/net/packet/internal.h
+++ b/net/packet/internal.h
@@ -114,6 +114,7 @@ struct packet_sock {
 	spinlock_t		bind_lock;
 	struct mutex		pg_vec_lock;
 	unsigned int		running:1,	/* prot_hook is attached*/
+				frozen:1,
 				auxdata:1,
 				origdev:1,
 				has_vnet_hdr:1;

             reply	other threads:[~2017-11-28  4:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-28  4:00 Eric Dumazet [this message]
2017-11-28 10:23 ` [PATCH net] net/packet: fix a race in packet_bind() and packet_notifier() Francesco Ruggeri
2017-11-28 15:13   ` Eric Dumazet
2017-11-28 14:48 ` David Miller
2017-11-28 15:14   ` Eric Dumazet
2017-11-28 16:03     ` [PATCH v2 " Eric Dumazet
2017-11-28 16:14       ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1511841652.16595.11.camel@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=fruggeri@aristanetworks.com \
    --cc=netdev@vger.kernel.org \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.