| Message ID | 20200921220709.96107-1-vladimir.oltean@nxp.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: bridge: br_vlan_get_pvid_rcu() should dereference the VLAN group under RCU | expand |
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: Tue, 22 Sep 2020 01:07:09 +0300 > When calling the RCU brother of br_vlan_get_pvid(), lockdep warns: > > ============================= > WARNING: suspicious RCU usage > 5.9.0-rc3-01631-g13c17acb8e38-dirty #814 Not tainted > ----------------------------- > net/bridge/br_private.h:1054 suspicious rcu_dereference_protected() usage! > > Call trace: > lockdep_rcu_suspicious+0xd4/0xf8 > __br_vlan_get_pvid+0xc0/0x100 > br_vlan_get_pvid_rcu+0x78/0x108 > > The warning is because br_vlan_get_pvid_rcu() calls nbp_vlan_group() > which calls rtnl_dereference() instead of rcu_dereference(). In turn, > rtnl_dereference() calls rcu_dereference_protected() which assumes > operation under an RCU write-side critical section, which obviously is > not the case here. So, when the incorrect primitive is used to access > the RCU-protected VLAN group pointer, READ_ONCE() is not used, which may > cause various unexpected problems. > > I'm sad to say that br_vlan_get_pvid() and br_vlan_get_pvid_rcu() cannot > share the same implementation. So fix the bug by splitting the 2 > functions, and making br_vlan_get_pvid_rcu() retrieve the VLAN groups > under proper locking annotations. > > Fixes: 7582f5b70f9a ("bridge: add br_vlan_get_pvid_rcu()") > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Applied and queued up for -stable, thank you.
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 199deb2adf60..002bbc93209d 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -1288,11 +1288,13 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v, } } -static int __br_vlan_get_pvid(const struct net_device *dev, - struct net_bridge_port *p, u16 *p_pvid) +int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid) { struct net_bridge_vlan_group *vg; + struct net_bridge_port *p; + ASSERT_RTNL(); + p = br_port_get_check_rtnl(dev); if (p) vg = nbp_vlan_group(p); else if (netif_is_bridge_master(dev)) @@ -1303,18 +1305,23 @@ static int __br_vlan_get_pvid(const struct net_device *dev, *p_pvid = br_get_pvid(vg); return 0; } - -int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid) -{ - ASSERT_RTNL(); - - return __br_vlan_get_pvid(dev, br_port_get_check_rtnl(dev), p_pvid); -} EXPORT_SYMBOL_GPL(br_vlan_get_pvid); int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid) { - return __br_vlan_get_pvid(dev, br_port_get_check_rcu(dev), p_pvid); + struct net_bridge_vlan_group *vg; + struct net_bridge_port *p; + + p = br_port_get_check_rcu(dev); + if (p) + vg = nbp_vlan_group_rcu(p); + else if (netif_is_bridge_master(dev)) + vg = br_vlan_group_rcu(netdev_priv(dev)); + else + return -EINVAL; + + *p_pvid = br_get_pvid(vg); + return 0; } EXPORT_SYMBOL_GPL(br_vlan_get_pvid_rcu);
When calling the RCU brother of br_vlan_get_pvid(), lockdep warns: ============================= WARNING: suspicious RCU usage 5.9.0-rc3-01631-g13c17acb8e38-dirty #814 Not tainted ----------------------------- net/bridge/br_private.h:1054 suspicious rcu_dereference_protected() usage! Call trace: lockdep_rcu_suspicious+0xd4/0xf8 __br_vlan_get_pvid+0xc0/0x100 br_vlan_get_pvid_rcu+0x78/0x108 The warning is because br_vlan_get_pvid_rcu() calls nbp_vlan_group() which calls rtnl_dereference() instead of rcu_dereference(). In turn, rtnl_dereference() calls rcu_dereference_protected() which assumes operation under an RCU write-side critical section, which obviously is not the case here. So, when the incorrect primitive is used to access the RCU-protected VLAN group pointer, READ_ONCE() is not used, which may cause various unexpected problems. I'm sad to say that br_vlan_get_pvid() and br_vlan_get_pvid_rcu() cannot share the same implementation. So fix the bug by splitting the 2 functions, and making br_vlan_get_pvid_rcu() retrieve the VLAN groups under proper locking annotations. Fixes: 7582f5b70f9a ("bridge: add br_vlan_get_pvid_rcu()") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- net/bridge/br_vlan.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)