Re: [PATCH net] net: igmp: add a missing rcu locking section

2018-02-02 Thread Stephen Hemminger
Another path has same issue, currently testing this.


From 92bf924c9eb7af1eade064583b9073baa03ec9f2 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger 
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: [] 
process_one_work+0x202/0x6d0
[  378.847428]  #1:  
((work_completion)(&(&net_device_ctx->dwork)->work)){+.+.}, at: 
[] process_one_work+0x202/0x6d0
[  378.847434]  #2:  (rtnl_mutex){+.+.}, at: [<303e0aaf>] 
netdev_notify_peers+0x22/0x80
[  378.847443]  #3:  (&(&im->lock)->rlock){+.-.}, at: [<05e1cdc1>] 
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 
---
 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



Re: [PATCH net] net: igmp: add a missing rcu locking section

2018-02-01 Thread David Miller
From: Eric Dumazet 
Date: Thu, 01 Feb 2018 10:26:57 -0800

> From: Eric Dumazet 
> 
> 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 
> Reported-by: syzbot 

Applied and queued up for -stable.