| Message ID | 1517509617.3715.120.camel@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: igmp: add a missing rcu locking section | expand |
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 01 Feb 2018 10:26:57 -0800 > From: Eric Dumazet <edumazet@google.com> > > Newly added igmpv3_get_srcaddr() needs to be called under rcu lock. > > Timer callbacks do not ensure this locking. ... > Fixes: a46182b00290 ("net: igmp: Use correct source address on IGMPv3 reports") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: syzbot <syzkaller@googlegroups.com> Applied and queued up for -stable.
Another path has same issue, currently testing this. From 92bf924c9eb7af1eade064583b9073baa03ec9f2 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger <sthemmin@microsoft.com> Date: Fri, 2 Feb 2018 13:10:11 -0800 Subject: [PATCH] igmp: fix unsafe RCU usage in igmpv3_src_addr Running with lockdep checking enabled, regularly see the following RCU usage warning caused by IGMP processing happening without holding RCU read lock in some cases. The warning happens where new IGMP source address lookup is, but real cause is having different context back in igmpv3_send_report. [ 378.847402] ============================= [ 378.847403] WARNING: suspicious RCU usage [ 378.847405] 4.15.0-net-next+ #1 Not tainted [ 378.847407] ----------------------------- [ 378.847410] ./include/linux/inetdevice.h:216 suspicious rcu_dereference_check() usage! [ 378.847413] other info that might help us debug this: [ 378.847415] rcu_scheduler_active = 2, debug_locks = 1 [ 378.847416] 4 locks held by kworker/4:0/35: [ 378.847417] #0: ((wq_completion)"events"){+.+.}, at: [<00000000bfcc881d>] process_one_work+0x202/0x6d0 [ 378.847428] #1: ((work_completion)(&(&net_device_ctx->dwork)->work)){+.+.}, at: [<00000000bfcc881d>] process_one_work+0x202/0x6d0 [ 378.847434] #2: (rtnl_mutex){+.+.}, at: [<00000000303e0aaf>] netdev_notify_peers+0x22/0x80 [ 378.847443] #3: (&(&im->lock)->rlock){+.-.}, at: [<0000000005e1cdc1>] igmpv3_send_report+0x29/0x270 [ 378.847450] stack backtrace: [ 378.847453] CPU: 4 PID: 35 Comm: kworker/4:0 Not tainted 4.15.0-net-next+ #1 [ 378.847454] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v1.0 11/26/2012 [ 378.847458] Workqueue: events netvsc_link_change [hv_netvsc] [ 378.847461] Call Trace: [ 378.847465] dump_stack+0x85/0xc5 [ 378.847468] igmpv3_newpack+0x2b2/0x2e0 [ 378.847472] add_grhead.isra.29+0x7a/0x90 [ 378.847474] add_grec+0x3d6/0x4e0 [ 378.847476] ? igmpv3_send_report+0x29/0x270 [ 378.847480] igmpv3_send_report+0x45/0x270 [ 378.847483] igmp_send_report+0x25a/0x280 [ 378.847486] ? __lock_is_held+0x55/0x90 [ 378.847488] ? __lock_is_held+0x55/0x90 [ 378.847492] igmp_netdev_event+0x103/0x210 [ 378.847495] notifier_call_chain+0x45/0x70 [ 378.847497] netdev_notify_peers+0x56/0x80 [ 378.847501] netvsc_link_change+0x254/0x2e0 [hv_netvsc] [ 378.847504] process_one_work+0x27e/0x6d0 [ 378.847508] worker_thread+0x37/0x3f0 [ 378.847511] ? process_one_work+0x6d0/0x6d0 [ 378.847513] kthread+0x11c/0x140 [ 378.847514] ? kthread_create_worker_on_cpu+0x70/0x70 [ 378.847518] ret_from_fork+0x3a/0x50 Fixes: a46182b00290 ("net: igmp: Use correct source address on IGMPv3 reports") Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- net/ipv4/igmp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index 10f7f74a0831..ff8dc5b9f120 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c @@ -579,8 +579,8 @@ static int igmpv3_send_report(struct in_device *in_dev, struct ip_mc_list *pmc) struct net *net = dev_net(in_dev->dev); int type; + rcu_read_lock(); if (!pmc) { - rcu_read_lock(); for_each_pmc_rcu(in_dev, pmc) { if (pmc->multiaddr == IGMP_ALL_HOSTS) continue; @@ -595,7 +595,6 @@ static int igmpv3_send_report(struct in_device *in_dev, struct ip_mc_list *pmc) skb = add_grec(skb, pmc, type, 0, 0); spin_unlock_bh(&pmc->lock); } - rcu_read_unlock(); } else { spin_lock_bh(&pmc->lock); if (pmc->sfcount[MCAST_EXCLUDE]) @@ -605,6 +604,8 @@ static int igmpv3_send_report(struct in_device *in_dev, struct ip_mc_list *pmc) skb = add_grec(skb, pmc, type, 0, 0); spin_unlock_bh(&pmc->lock); } + rcu_read_unlock(); + if (!skb) return 0; return igmpv3_sendpack(skb);
============================= WARNING: suspicious RCU usage 4.15.0+ #200 Not tainted ----------------------------- ./include/linux/inetdevice.h:216 suspicious rcu_dereference_check() usage! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 3 locks held by syzkaller616973/4074: #0: (&mm->mmap_sem){++++}, at: [<00000000bfce669e>] __do_page_fault+0x32d/0xc90 arch/x86/mm/fault.c:1355 #1: ((&im->timer)){+.-.}, at: [<00000000619d2f71>] lockdep_copy_map include/linux/lockdep.h:178 [inline] #1: ((&im->timer)){+.-.}, at: [<00000000619d2f71>] call_timer_fn+0x1c6/0x820 kernel/time/timer.c:1316 #2: (&(&im->lock)->rlock){+.-.}, at: [<000000005f833c5c>] spin_lock_bh include/linux/spinlock.h:315 [inline] #2: (&(&im->lock)->rlock){+.-.}, at: [<000000005f833c5c>] igmpv3_send_report+0x98/0x5b0 net/ipv4/igmp.c:600 stack backtrace: CPU: 0 PID: 4074 Comm: syzkaller616973 Not tainted 4.15.0+ #200 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: <IRQ> __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x194/0x257 lib/dump_stack.c:53 lockdep_rcu_suspicious+0x123/0x170 kernel/locking/lockdep.c:4592 __in_dev_get_rcu include/linux/inetdevice.h:216 [inline] igmpv3_get_srcaddr net/ipv4/igmp.c:329 [inline] igmpv3_newpack+0xeef/0x12e0 net/ipv4/igmp.c:389 add_grhead.isra.27+0x235/0x300 net/ipv4/igmp.c:432 add_grec+0xbd3/0x1170 net/ipv4/igmp.c:565 igmpv3_send_report+0xd5/0x5b0 net/ipv4/igmp.c:605 igmp_send_report+0xc43/0x1050 net/ipv4/igmp.c:722 igmp_timer_expire+0x322/0x5c0 net/ipv4/igmp.c:831 call_timer_fn+0x228/0x820 kernel/time/timer.c:1326 expire_timers kernel/time/timer.c:1363 [inline] __run_timers+0x7ee/0xb70 kernel/time/timer.c:1666 run_timer_softirq+0x4c/0x70 kernel/time/timer.c:1692 __do_softirq+0x2d7/0xb85 kernel/softirq.c:285 invoke_softirq kernel/softirq.c:365 [inline] irq_exit+0x1cc/0x200 kernel/softirq.c:405 exiting_irq arch/x86/include/asm/apic.h:541 [inline] smp_apic_timer_interrupt+0x16b/0x700 arch/x86/kernel/apic/apic.c:1052 apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:938 Fixes: a46182b00290 ("net: igmp: Use correct source address on IGMPv3 reports") Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: syzbot <syzkaller@googlegroups.com> --- net/ipv4/igmp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index 10f7f74a0831ee3d4d6720552dfa91fddeb4c31a..f2402581fef1b559c0952e142a682d596d4be5b5 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c @@ -386,7 +386,11 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, unsigned int mtu) pip->frag_off = htons(IP_DF); pip->ttl = 1; pip->daddr = fl4.daddr; + + rcu_read_lock(); pip->saddr = igmpv3_get_srcaddr(dev, &fl4); + rcu_read_unlock(); + pip->protocol = IPPROTO_IGMP; pip->tot_len = 0; /* filled in later */ ip_select_ident(net, skb, NULL);
From: Eric Dumazet <edumazet@google.com> Newly added igmpv3_get_srcaddr() needs to be called under rcu lock. Timer callbacks do not ensure this locking.