netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).