Skip to content

Commit 2dd6fc3

Browse files
anoobsGerrit - the friendly Code Review server
authored andcommitted
packet: Do not call fanout_release from atomic contexts
[ Upstream commit 2bd624b4611ffee36422782d16e1c944d1351e98 ] 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] [ ] AOSP-JF-MM#2: (rtnl_mutex){+.+.+.}, at: [<ffffffff81564157>] rtnl_lock+0x17/0x20 [ ] CyanogenMod#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 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: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Git-commit: fe41cfb Git-repo: https://source.codeaurora.org/quic/la/kernel/msm-4.4/commit/ Change-Id: If5c94fcd9ee7fb15fadd50fa05f3a73688be61e8 Signed-off-by: Tejaswi Tanikella <tejaswit@qti.qualcomm.com>
1 parent 4c99ac1 commit 2dd6fc3

1 file changed

Lines changed: 24 additions & 8 deletions

File tree

net/packet/af_packet.c

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,6 +1380,8 @@ static void __fanout_link(struct sock *sk, struct packet_sock *po)
13801380
f->arr[f->num_members] = sk;
13811381
smp_wmb();
13821382
f->num_members++;
1383+
if (f->num_members == 1)
1384+
dev_add_pack(&f->prot_hook);
13831385
spin_unlock(&f->lock);
13841386
}
13851387

@@ -1396,6 +1398,8 @@ static void __fanout_unlink(struct sock *sk, struct packet_sock *po)
13961398
BUG_ON(i >= f->num_members);
13971399
f->arr[i] = f->arr[f->num_members - 1];
13981400
f->num_members--;
1401+
if (f->num_members == 0)
1402+
__dev_remove_pack(&f->prot_hook);
13991403
spin_unlock(&f->lock);
14001404
}
14011405

@@ -1468,7 +1472,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
14681472
match->prot_hook.func = packet_rcv_fanout;
14691473
match->prot_hook.af_packet_priv = match;
14701474
match->prot_hook.id_match = match_fanout_group;
1471-
dev_add_pack(&match->prot_hook);
14721475
list_add(&match->list, &fanout_list);
14731476
}
14741477
err = -EINVAL;
@@ -1489,7 +1492,14 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
14891492
return err;
14901493
}
14911494

1492-
static void fanout_release(struct sock *sk)
1495+
/* If pkt_sk(sk)->fanout->sk_ref is zero, this function removes
1496+
* pkt_sk(sk)->fanout from fanout_list and returns pkt_sk(sk)->fanout.
1497+
* It is the responsibility of the caller to free the returned packet_fanout
1498+
* (after synchronize_net())
1499+
* This Branch still does not have support classic BPF fanout mode.
1500+
* upstream commit 47dceb8ecdc: packet: add classic BPF fanout mode
1501+
*/
1502+
static struct packet_fanout *fanout_release(struct sock *sk)
14931503
{
14941504
struct packet_sock *po = pkt_sk(sk);
14951505
struct packet_fanout *f;
@@ -1499,13 +1509,15 @@ static void fanout_release(struct sock *sk)
14991509
if (f) {
15001510
po->fanout = NULL;
15011511

1502-
if (atomic_dec_and_test(&f->sk_ref)) {
1512+
if (atomic_dec_and_test(&f->sk_ref))
15031513
list_del(&f->list);
1504-
dev_remove_pack(&f->prot_hook);
1505-
kfree(f);
1506-
}
1514+
else
1515+
f = NULL;
1516+
15071517
}
15081518
mutex_unlock(&fanout_mutex);
1519+
1520+
return f;
15091521
}
15101522

15111523
static bool packet_extra_vlan_len_allowed(const struct net_device *dev,
@@ -2587,6 +2599,7 @@ static int packet_release(struct socket *sock)
25872599
{
25882600
struct sock *sk = sock->sk;
25892601
struct packet_sock *po;
2602+
struct packet_fanout *f;
25902603
struct net *net;
25912604
union tpacket_req_u req_u;
25922605

@@ -2626,9 +2639,13 @@ static int packet_release(struct socket *sock)
26262639
packet_set_ring(sk, &req_u, 1, 1);
26272640
}
26282641

2629-
fanout_release(sk);
2642+
f = fanout_release(sk);
26302643

26312644
synchronize_net();
2645+
2646+
if (f)
2647+
kfree(f);
2648+
26322649
/*
26332650
* Now the socket is dead. No more input will appear.
26342651
*/
@@ -3567,7 +3584,6 @@ static int packet_notifier(struct notifier_block *this,
35673584
}
35683585
if (msg == NETDEV_UNREGISTER) {
35693586
packet_cached_dev_reset(po);
3570-
fanout_release(sk);
35713587
po->ifindex = -1;
35723588
if (po->prot_hook.dev)
35733589
dev_put(po->prot_hook.dev);

0 commit comments

Comments
 (0)