Skip to content

Commit 907b1e6

Browse files
Nikolay Aleksandrovdavem330
Nikolay Aleksandrov
authored and
davem330
committed
bridge: vlan: use proper rcu for the vlgrp member
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 <nikolay@cumulusnetworks.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 4b91816 commit 907b1e6

File tree

6 files changed

+104
-53
lines changed

6 files changed

+104
-53
lines changed

‎net/bridge/br_device.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
5656
skb_reset_mac_header(skb);
5757
skb_pull(skb, ETH_HLEN);
5858

59-
if (!br_allowed_ingress(br, br_vlan_group(br), skb, &vid))
59+
if (!br_allowed_ingress(br, br_vlan_group_rcu(br), skb, &vid))
6060
goto out;
6161

6262
if (is_broadcast_ether_addr(dest))

‎net/bridge/br_forward.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ static inline int should_deliver(const struct net_bridge_port *p,
3232
{
3333
struct net_bridge_vlan_group *vg;
3434

35-
vg = nbp_vlan_group(p);
35+
vg = nbp_vlan_group_rcu(p);
3636
return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
3737
br_allowed_egress(vg, skb) && p->state == BR_STATE_FORWARDING;
3838
}
@@ -80,7 +80,7 @@ static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
8080
{
8181
struct net_bridge_vlan_group *vg;
8282

83-
vg = nbp_vlan_group(to);
83+
vg = nbp_vlan_group_rcu(to);
8484
skb = br_handle_vlan(to->br, vg, skb);
8585
if (!skb)
8686
return;
@@ -112,7 +112,7 @@ static void __br_forward(const struct net_bridge_port *to, struct sk_buff *skb)
112112
return;
113113
}
114114

115-
vg = nbp_vlan_group(to);
115+
vg = nbp_vlan_group_rcu(to);
116116
skb = br_handle_vlan(to->br, vg, skb);
117117
if (!skb)
118118
return;

‎net/bridge/br_input.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ static int br_pass_frame_up(struct sk_buff *skb)
4444
brstats->rx_bytes += skb->len;
4545
u64_stats_update_end(&brstats->syncp);
4646

47-
vg = br_vlan_group(br);
47+
vg = br_vlan_group_rcu(br);
4848
/* Bridge is just like any other port. Make sure the
4949
* packet is allowed except in promisc modue when someone
5050
* may be running packet capture.
@@ -140,7 +140,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
140140
if (!p || p->state == BR_STATE_DISABLED)
141141
goto drop;
142142

143-
if (!br_allowed_ingress(p->br, nbp_vlan_group(p), skb, &vid))
143+
if (!br_allowed_ingress(p->br, nbp_vlan_group_rcu(p), skb, &vid))
144144
goto out;
145145

146146
/* insert into forwarding database after filtering to avoid spoofing */

‎net/bridge/br_netlink.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ static size_t br_get_link_af_size_filtered(const struct net_device *dev,
102102
rcu_read_lock();
103103
if (br_port_exists(dev)) {
104104
p = br_port_get_rcu(dev);
105-
vg = nbp_vlan_group(p);
105+
vg = nbp_vlan_group_rcu(p);
106106
} else if (dev->priv_flags & IFF_EBRIDGE) {
107107
br = netdev_priv(dev);
108-
vg = br_vlan_group(br);
108+
vg = br_vlan_group_rcu(br);
109109
}
110110
num_vlan_infos = br_get_num_vlan_infos(vg, filter_mask);
111111
rcu_read_unlock();

‎net/bridge/br_private.h

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ struct net_bridge_vlan_group {
132132
struct list_head vlan_list;
133133
u16 num_vlans;
134134
u16 pvid;
135+
struct rcu_head rcu;
135136
};
136137

137138
struct net_bridge_fdb_entry
@@ -229,7 +230,7 @@ struct net_bridge_port
229230
struct netpoll *np;
230231
#endif
231232
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
232-
struct net_bridge_vlan_group *vlgrp;
233+
struct net_bridge_vlan_group __rcu *vlgrp;
233234
#endif
234235
};
235236

@@ -337,7 +338,7 @@ struct net_bridge
337338
struct kobject *ifobj;
338339
u32 auto_cnt;
339340
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
340-
struct net_bridge_vlan_group *vlgrp;
341+
struct net_bridge_vlan_group __rcu *vlgrp;
341342
u8 vlan_enabled;
342343
__be16 vlan_proto;
343344
u16 default_pvid;
@@ -700,13 +701,25 @@ int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask);
700701
static inline struct net_bridge_vlan_group *br_vlan_group(
701702
const struct net_bridge *br)
702703
{
703-
return br->vlgrp;
704+
return rtnl_dereference(br->vlgrp);
704705
}
705706

706707
static inline struct net_bridge_vlan_group *nbp_vlan_group(
707708
const struct net_bridge_port *p)
708709
{
709-
return p->vlgrp;
710+
return rtnl_dereference(p->vlgrp);
711+
}
712+
713+
static inline struct net_bridge_vlan_group *br_vlan_group_rcu(
714+
const struct net_bridge *br)
715+
{
716+
return rcu_dereference(br->vlgrp);
717+
}
718+
719+
static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu(
720+
const struct net_bridge_port *p)
721+
{
722+
return rcu_dereference(p->vlgrp);
710723
}
711724

712725
/* Since bridge now depends on 8021Q module, but the time bridge sees the
@@ -853,6 +866,19 @@ static inline struct net_bridge_vlan_group *nbp_vlan_group(
853866
{
854867
return NULL;
855868
}
869+
870+
static inline struct net_bridge_vlan_group *br_vlan_group_rcu(
871+
const struct net_bridge *br)
872+
{
873+
return NULL;
874+
}
875+
876+
static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu(
877+
const struct net_bridge_port *p)
878+
{
879+
return NULL;
880+
}
881+
856882
#endif
857883

858884
struct nf_br_ops {

0 commit comments

Comments
 (0)