Re: [PATCH] igmp: fix unsafe RCU usage in igmpv3_src_addr

[Stephen Hemminger]

On Fri, 02 Feb 2018 14:18:45 -0800
Eric Dumazet  wrote:

> On Fri, 2018-02-02 at 13:30 -0800, Stephen Hemminger wrote:
> > From: Stephen Hemminger 
> > 
> > New igmpv3_get_src_addr would sometimes be called in receive path
> > without holding RCU lock.  
> 
> Strange, this should have been fixed already ?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=e7aadb27a5415e8125834b84a74477bfbee4eff5

Yes, that fixes it as well.
Did not have that commit on my branch yet.

Re: [PATCH] igmp: fix unsafe RCU usage in igmpv3_src_addr

[Eric Dumazet]

On Fri, 2018-02-02 at 13:30 -0800, Stephen Hemminger wrote:
> From: Stephen Hemminger 
> 
> New igmpv3_get_src_addr would sometimes be called in receive path
> without holding RCU lock.

Strange, this should have been fixed already ?

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=e7aadb27a5415e8125834b84a74477bfbee4eff5

[PATCH] igmp: fix unsafe RCU usage in igmpv3_src_addr

[Stephen Hemminger]

From: Stephen Hemminger 

New igmpv3_get_src_addr would sometimes be called in receive path
without holding RCU lock.

[  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)(&(_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:  (&(>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(>lock);
}
-   rcu_read_unlock();
} else {
spin_lock_bh(>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(>lock);
}
+   rcu_read_unlock();
+
if (!skb)
return 0;
return igmpv3_sendpack(skb);
-- 
2.15.1