packet: Do not call fanout_release from atomic contexts
commit 2bd624b4611ffee36422782d16e1c944d1351e98 upstream.
Commit 6664498280cf ("packet: call fanout_release, while UNREGISTERING a
netdev"), unfortunately, introduced the following issues.
1. calling mutex_lock(&fanout_mutex) (fanout_release()) from inside
rcu_read-side critical section. rcu_read_lock disables preemption, most often,
which prohibits calling sleeping functions.
[ ] include/linux/rcupdate.h:560 Illegal context switch in RCU read-side critical section!
[ ]
[ ] rcu_scheduler_active = 1, debug_locks = 0
[ ] 4 locks held by ovs-vswitchd/1969:
[ ] #0: (cb_lock){++++++}, at: [<ffffffff8158a6c9>] genl_rcv+0x19/0x40
[ ] #1: (ovs_mutex){+.+.+.}, at: [<ffffffffa04878ca>] ovs_vport_cmd_del+0x4a/0x100 [openvswitch]
[ ] #2: (rtnl_mutex){+.+.+.}, at: [<ffffffff81564157>] rtnl_lock+0x17/0x20
[ ] #3: (rcu_read_lock){......}, at: [<ffffffff81614165>] packet_notifier+0x5/0x3f0
[ ]
[ ] Call Trace:
[ ] [<ffffffff813770c1>] dump_stack+0x85/0xc4
[ ] [<ffffffff810c9077>] lockdep_rcu_suspicious+0x107/0x110
[ ] [<ffffffff810a2da7>] ___might_sleep+0x57/0x210
[ ] [<ffffffff810a2fd0>] __might_sleep+0x70/0x90
[ ] [<ffffffff8162e80c>] mutex_lock_nested+0x3c/0x3a0
[ ] [<ffffffff810de93f>] ? vprintk_default+0x1f/0x30
[ ] [<ffffffff81186e88>] ? printk+0x4d/0x4f
[ ] [<ffffffff816106dd>] fanout_release+0x1d/0xe0
[ ] [<ffffffff81614459>] packet_notifier+0x2f9/0x3f0
2. calling mutex_lock(&fanout_mutex) inside spin_lock(&po->bind_lock).
"sleeping function called from invalid context"
[ ] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:620
[ ] in_atomic(): 1, irqs_disabled(): 0, pid: 1969, name: ovs-vswitchd
[ ] INFO: lockdep is turned off.
[ ] Call Trace:
[ ] [<ffffffff813770c1>] dump_stack+0x85/0xc4
[ ] [<ffffffff810a2f52>] ___might_sleep+0x202/0x210
[ ] [<ffffffff810a2fd0>] __might_sleep+0x70/0x90
[ ] [<ffffffff8162e80c>] mutex_lock_nested+0x3c/0x3a0
[ ] [<ffffffff816106dd>] fanout_release+0x1d/0xe0
[ ] [<ffffffff81614459>] packet_notifier+0x2f9/0x3f0
3. calling dev_remove_pack(&fanout->prot_hook), from inside
spin_lock(&po->bind_lock) or rcu_read-side critical-section. dev_remove_pack()
-> synchronize_net(), which might sleep.
[ ] BUG: scheduling while atomic: ovs-vswitchd/1969/0x00000002
[ ] INFO: lockdep is turned off.
[ ] Call Trace:
[ ] [<ffffffff813770c1>] dump_stack+0x85/0xc4
[ ] [<ffffffff81186274>] __schedule_bug+0x64/0x73
[ ] [<ffffffff8162b8cb>] __schedule+0x6b/0xd10
[ ] [<ffffffff8162c5db>] schedule+0x6b/0x80
[ ] [<ffffffff81630b1d>] schedule_timeout+0x38d/0x410
[ ] [<ffffffff810ea3fd>] synchronize_sched_expedited+0x53d/0x810
[ ] [<ffffffff810ea6de>] synchronize_rcu_expedited+0xe/0x10
[ ] [<ffffffff8154eab5>] synchronize_net+0x35/0x50
[ ] [<ffffffff8154eae3>] dev_remove_pack+0x13/0x20
[ ] [<ffffffff8161077e>] fanout_release+0xbe/0xe0
[ ] [<ffffffff81614459>] packet_notifier+0x2f9/0x3f0
4. fanout_release() races with calls from different CPU.
To fix the above problems, remove the call to fanout_release() under
rcu_read_lock(). Instead, call __dev_remove_pack(&fanout->prot_hook) and
netdev_run_todo will be happy that &dev->ptype_specific list is empty. In order
to achieve this, I moved dev_{add,remove}_pack() out of fanout_{add,release} to
__fanout_{link,unlink}. So, call to {,__}unregister_prot_hook() will make sure
fanout->prot_hook is removed as well.
[js] no rollover in 3.12
Fixes: 6664498280cf ("packet: call fanout_release, while UNREGISTERING a netdev")
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Willy Tarreau <w@1wt.eu>
This commit is contained in:
Anoob Soman
syphyr
parent
a8ca5c30e1
commit
bba23ccd31
|
|
@ -1257,6 +1257,8 @@ static void __fanout_link(struct sock *sk, struct packet_sock *po)
|
|||
f->arr[f->num_members] = sk;
|
||||
smp_wmb();
|
||||
f->num_members++;
|
||||
if (f->num_members == 1)
|
||||
dev_add_pack(&f->prot_hook);
|
||||
spin_unlock(&f->lock);
|
||||
}
|
||||
|
||||
|
|
@ -1273,6 +1275,8 @@ static void __fanout_unlink(struct sock *sk, struct packet_sock *po)
|
|||
BUG_ON(i >= f->num_members);
|
||||
f->arr[i] = f->arr[f->num_members - 1];
|
||||
f->num_members--;
|
||||
if (f->num_members == 0)
|
||||
__dev_remove_pack(&f->prot_hook);
|
||||
spin_unlock(&f->lock);
|
||||
}
|
||||
|
||||
|
|
@ -1339,7 +1343,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
|
|||
match->prot_hook.func = packet_rcv_fanout;
|
||||
match->prot_hook.af_packet_priv = match;
|
||||
match->prot_hook.id_match = match_fanout_group;
|
||||
dev_add_pack(&match->prot_hook);
|
||||
list_add(&match->list, &fanout_list);
|
||||
}
|
||||
err = -EINVAL;
|
||||
|
|
@ -1370,7 +1373,12 @@ out:
|
|||
return err;
|
||||
}
|
||||
|
||||
static void fanout_release(struct sock *sk)
|
||||
/* If pkt_sk(sk)->fanout->sk_ref is zero, this function removes
|
||||
* pkt_sk(sk)->fanout from fanout_list and returns pkt_sk(sk)->fanout.
|
||||
* It is the responsibility of the caller to call fanout_release_data() and
|
||||
* free the returned packet_fanout (after synchronize_net())
|
||||
*/
|
||||
static struct packet_fanout *fanout_release(struct sock *sk)
|
||||
{
|
||||
struct packet_sock *po = pkt_sk(sk);
|
||||
struct packet_fanout *f;
|
||||
|
|
@ -1380,13 +1388,14 @@ static void fanout_release(struct sock *sk)
|
|||
if (f) {
|
||||
po->fanout = NULL;
|
||||
|
||||
if (atomic_dec_and_test(&f->sk_ref)) {
|
||||
if (atomic_dec_and_test(&f->sk_ref))
|
||||
list_del(&f->list);
|
||||
dev_remove_pack(&f->prot_hook);
|
||||
kfree(f);
|
||||
}
|
||||
else
|
||||
f = NULL;
|
||||
}
|
||||
mutex_unlock(&fanout_mutex);
|
||||
|
||||
return f;
|
||||
}
|
||||
|
||||
static const struct proto_ops packet_ops;
|
||||
|
|
@ -2439,6 +2448,7 @@ static int packet_release(struct socket *sock)
|
|||
{
|
||||
struct sock *sk = sock->sk;
|
||||
struct packet_sock *po;
|
||||
struct packet_fanout *f;
|
||||
struct net *net;
|
||||
union tpacket_req_u req_u;
|
||||
|
||||
|
|
@ -2478,9 +2488,13 @@ static int packet_release(struct socket *sock)
|
|||
packet_set_ring(sk, &req_u, 1, 1);
|
||||
}
|
||||
|
||||
fanout_release(sk);
|
||||
f = fanout_release(sk);
|
||||
|
||||
synchronize_net();
|
||||
|
||||
if (f) {
|
||||
kfree(f);
|
||||
}
|
||||
/*
|
||||
* Now the socket is dead. No more input will appear.
|
||||
*/
|
||||
|
|
@ -3406,7 +3420,6 @@ static int packet_notifier(struct notifier_block *this, unsigned long msg, void
|
|||
}
|
||||
if (msg == NETDEV_UNREGISTER) {
|
||||
packet_cached_dev_reset(po);
|
||||
fanout_release(sk);
|
||||
po->ifindex = -1;
|
||||
if (po->prot_hook.dev)
|
||||
dev_put(po->prot_hook.dev);
|
||||
|
|
|
|||
Loading…
Reference in New Issue