[PATCH] igmp: fix unsafe RCU usage in igmpv3_src_addr

[PATCH] igmp: fix unsafe RCU usage in igmpv3_src_addr

[Stephen Hemminger]

From: Stephen Hemminger <stephen@networkplumber.org>

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: [<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

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 <stephen@networkplumber.org>
> 
> 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

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

[Stephen Hemminger]

On Fri, 02 Feb 2018 14:18:45 -0800
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Fri, 2018-02-02 at 13:30 -0800, Stephen Hemminger wrote:
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > 
> > 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.