[PATCH net-next 1/4] bridge: vlan: use proper rcu for the vlgrp member

[Nikolay Aleksandrov]

From: Nikolay Aleksandrov 

The bridge and port's vlgrp member is already used in RCU way, currently
we rely on the fact that it cannot disappear while the port exists but
that is error-prone and we might miss places with improper locking
(either RCU or RTNL must be held to walk the vlan_list). So make it
official and use RCU for vlgrp to catch offenders. Introduce proper vlgrp
accessors and use them consistently throughout the code.

Signed-off-by: Nikolay Aleksandrov 
---
 net/bridge/br_device.c  |   2 +-
 net/bridge/br_forward.c |   6 +--
 net/bridge/br_input.c   |   4 +-
 net/bridge/br_netlink.c |   4 +-
 net/bridge/br_private.h |  34 +--
 net/bridge/br_vlan.c| 107 +---
 6 files changed, 104 insertions(+), 53 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index bdfb9544ca03..5e88d3e17546 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -56,7 +56,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct 
net_device *dev)
skb_reset_mac_header(skb);
skb_pull(skb, ETH_HLEN);
 
-   if (!br_allowed_ingress(br, br_vlan_group(br), skb, ))
+   if (!br_allowed_ingress(br, br_vlan_group_rcu(br), skb, ))
goto out;
 
if (is_broadcast_ether_addr(dest))
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 6d5ed795c3e2..a9d424e20229 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -32,7 +32,7 @@ static inline int should_deliver(const struct net_bridge_port 
*p,
 {
struct net_bridge_vlan_group *vg;
 
-   vg = nbp_vlan_group(p);
+   vg = nbp_vlan_group_rcu(p);
return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
br_allowed_egress(vg, skb) && p->state == BR_STATE_FORWARDING;
 }
@@ -80,7 +80,7 @@ static void __br_deliver(const struct net_bridge_port *to, 
struct sk_buff *skb)
 {
struct net_bridge_vlan_group *vg;
 
-   vg = nbp_vlan_group(to);
+   vg = nbp_vlan_group_rcu(to);
skb = br_handle_vlan(to->br, vg, skb);
if (!skb)
return;
@@ -112,7 +112,7 @@ static void __br_forward(const struct net_bridge_port *to, 
struct sk_buff *skb)
return;
}
 
-   vg = nbp_vlan_group(to);
+   vg = nbp_vlan_group_rcu(to);
skb = br_handle_vlan(to->br, vg, skb);
if (!skb)
return;
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index f5c5a4500e2f..f7fba74108a9 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -44,7 +44,7 @@ static int br_pass_frame_up(struct sk_buff *skb)
brstats->rx_bytes += skb->len;
u64_stats_update_end(>syncp);
 
-   vg = br_vlan_group(br);
+   vg = br_vlan_group_rcu(br);
/* Bridge is just like any other port.  Make sure the
 * packet is allowed except in promisc modue when someone
 * may be running packet capture.
@@ -140,7 +140,7 @@ int br_handle_frame_finish(struct net *net, struct sock 
*sk, struct sk_buff *skb
if (!p || p->state == BR_STATE_DISABLED)
goto drop;
 
-   if (!br_allowed_ingress(p->br, nbp_vlan_group(p), skb, ))
+   if (!br_allowed_ingress(p->br, nbp_vlan_group_rcu(p), skb, ))
goto out;
 
/* insert into forwarding database after filtering to avoid spoofing */
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index d78b4429505a..edee48e9aa8f 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -102,10 +102,10 @@ static size_t br_get_link_af_size_filtered(const struct 
net_device *dev,
rcu_read_lock();
if (br_port_exists(dev)) {
p = br_port_get_rcu(dev);
-   vg = nbp_vlan_group(p);
+   vg = nbp_vlan_group_rcu(p);
} else if (dev->priv_flags & IFF_EBRIDGE) {
br = netdev_priv(dev);
-   vg = br_vlan_group(br);
+   vg = br_vlan_group_rcu(br);
}
num_vlan_infos = br_get_num_vlan_infos(vg, filter_mask);
rcu_read_unlock();
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 09d3ecbcb4f0..7d14ba93bba4 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -132,6 +132,7 @@ struct net_bridge_vlan_group {
struct list_headvlan_list;
u16 num_vlans;
u16 pvid;
+   struct rcu_head rcu;
 };
 
 struct net_bridge_fdb_entry
@@ -229,7 +230,7 @@ struct net_bridge_port
struct netpoll  *np;
 #endif
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
-   struct net_bridge_vlan_group*vlgrp;
+   struct net_bridge_vlan_group__rcu *vlgrp;
 #endif
 };
 
@@ -337,7 +338,7 @@ struct net_bridge
struct kobject  *ifobj;
u32 

Re: [PATCH net-next 1/4] bridge: vlan: use proper rcu for the vlgrp member

[Ido Schimmel]

Mon, Oct 12, 2015 at 02:41:06PM IDT, ra...@blackwall.org wrote:
>From: Nikolay Aleksandrov 
>
Hi Nik,

Small nitpick:



>@@ -825,17 +848,19 @@ unlock:
> 
> int br_vlan_init(struct net_bridge *br)
> {
>+  struct net_bridge_vlan_group *vg;
>   int ret = -ENOMEM;
> 
>-  br->vlgrp = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL);
>-  if (!br->vlgrp)
>+  vg = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL);
Maybe just do sizeof(*vg)?
>+  if (!vg)
>   goto out;
>-  ret = rhashtable_init(>vlgrp->vlan_hash, _vlan_rht_params);
>+  ret = rhashtable_init(>vlan_hash, _vlan_rht_params);
>   if (ret)
>   goto err_rhtbl;
>-  INIT_LIST_HEAD(>vlgrp->vlan_list);
>+  INIT_LIST_HEAD(>vlan_list);
>   br->vlan_proto = htons(ETH_P_8021Q);
>   br->default_pvid = 1;
>+  rcu_assign_pointer(br->vlgrp, vg);
>   ret = br_vlan_add(br, 1,
> BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED |
> BRIDGE_VLAN_INFO_BRENTRY);
>@@ -846,9 +871,9 @@ out:
>   return ret;



>-- 
>2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html