Re: Suspicious RCU usage in bridge with Linux v4.0-9362-g1fc149933fd4

[Cong Wang <cwang@twopensource.com>]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

On Mon, May 4, 2015 at 6:39 AM, Dominick Grift <dac.override@gmail.com> wrote:
> On Thu, Apr 23, 2015 at 01:07:45PM -0400, Josh Boyer wrote:
>> Hi All,
>>
>> We've had a user report the following backtrace from the bridge module
>> with a recent Linus' tree.  Has anything like this been reported yet?
>> If you have any questions on setup, the user is CC'd.
>>
>> josh
>>
>> [   29.382235] br0: port 1(tap0) entered forwarding state
>>
>> [   29.382286] ===============================
>> [   29.382315] [ INFO: suspicious RCU usage. ]
>> [   29.382344] 4.1.0-0.rc0.git11.1.fc23.x86_64 #1 Not tainted
>> [   29.382380] -------------------------------
>> [   29.382409] net/bridge/br_private.h:626 suspicious
>> rcu_dereference_check() usage!
>
> <snip>
>
> With 4.1.0-0.rc1.git1.1.fc23.x86_64 the situation seems to have slightly changed:
>

Should be the same issue. Please give the attached patch a try,
it is compile-tested only.

Thanks!

[-- Attachment #2: br.diff --]
[-- Type: text/plain, Size: 1708 bytes --]

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 4b5c236..8ba989a 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -325,22 +325,28 @@ static int br_fill_ifinfo(struct sk_buff *skb,
 		struct nlattr *af;
 		int err;
 
+		rcu_read_lock();
 		if (port)
 			pv = nbp_get_vlan_info(port);
 		else
 			pv = br_get_vlan_info(br);
 
-		if (!pv || bitmap_empty(pv->vlan_bitmap, VLAN_N_VID))
+		if (!pv || bitmap_empty(pv->vlan_bitmap, VLAN_N_VID)) {
+			rcu_read_unlock();
 			goto done;
+		}
 
 		af = nla_nest_start(skb, IFLA_AF_SPEC);
-		if (!af)
+		if (!af) {
+			rcu_read_unlock();
 			goto nla_put_failure;
+		}
 
 		if (filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED)
 			err = br_fill_ifvlaninfo_compressed(skb, pv);
 		else
 			err = br_fill_ifvlaninfo(skb, pv);
+		rcu_read_unlock();
 		if (err)
 			goto nla_put_failure;
 		nla_nest_end(skb, af);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 3362c29..860832e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -617,13 +617,17 @@ int nbp_vlan_init(struct net_bridge_port *port);
 static inline struct net_port_vlans *br_get_vlan_info(
 						const struct net_bridge *br)
 {
-	return rcu_dereference_rtnl(br->vlan_info);
+	return rcu_dereference_protected(br->vlan_info,
+					 rcu_read_lock_held() ||
+					 lockdep_rtnl_is_held());
 }
 
 static inline struct net_port_vlans *nbp_get_vlan_info(
 						const struct net_bridge_port *p)
 {
-	return rcu_dereference_rtnl(p->vlan_info);
+	return rcu_dereference_protected(p->vlan_info,
+					 rcu_read_lock_held() ||
+					 lockdep_rtnl_is_held());
 }
 
 /* Since bridge now depends on 8021Q module, but the time bridge sees the