* [PATCH net] net: igmp: add a missing rcu locking section
@ 2018-02-01 18:26 Eric Dumazet
2018-02-01 20:00 ` David Miller
2018-02-02 21:19 ` Stephen Hemminger
0 siblings, 2 replies; 3+ messages in thread
From: Eric Dumazet @ 2018-02-01 18:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Kevin Cernekee
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.
=============================
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);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: igmp: add a missing rcu locking section
2018-02-01 18:26 [PATCH net] net: igmp: add a missing rcu locking section Eric Dumazet
@ 2018-02-01 20:00 ` David Miller
2018-02-02 21:19 ` Stephen Hemminger
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2018-02-01 20:00 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, cernekee
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: igmp: add a missing rcu locking section
2018-02-01 18:26 [PATCH net] net: igmp: add a missing rcu locking section Eric Dumazet
2018-02-01 20:00 ` David Miller
@ 2018-02-02 21:19 ` Stephen Hemminger
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2018-02-02 21:19 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Kevin Cernekee
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);
--
2.15.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-02 21:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-01 18:26 [PATCH net] net: igmp: add a missing rcu locking section Eric Dumazet
2018-02-01 20:00 ` David Miller
2018-02-02 21:19 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).