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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 2 Feb 2018 13:19:57 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Kevin Cernekee <cernekee@...omium.org>
Subject: Re: [PATCH net] net: igmp: add a missing rcu locking section

Another path has same issue, currently testing this.


From 92bf924c9eb7af1eade064583b9073baa03ec9f2 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <sthemmin@...rosoft.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@...workplumber.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