bridge: fix RCU races with bridge port
The macro br_port_exists() is not enough protection when only
RCU is being used. There is a tiny race where other CPU has cleared port
handler hook, but is bridge port flag might still be set.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 90512cc..2872393 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -238,15 +238,18 @@
int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
{
struct net_bridge_fdb_entry *fdb;
+ struct net_bridge_port *port;
int ret;
- if (!br_port_exists(dev))
- return 0;
-
rcu_read_lock();
- fdb = __br_fdb_get(br_port_get_rcu(dev)->br, addr);
- ret = fdb && fdb->dst->dev != dev &&
- fdb->dst->state == BR_STATE_FORWARDING;
+ port = br_port_get_rcu(dev);
+ if (!port)
+ ret = 0;
+ else {
+ fdb = __br_fdb_get(port->br, addr);
+ ret = fdb && fdb->dst->dev != dev &&
+ fdb->dst->state == BR_STATE_FORWARDING;
+ }
rcu_read_unlock();
return ret;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 89ad25a..427f90a 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -475,11 +475,8 @@
{
struct net_bridge_port *p;
- if (!br_port_exists(dev))
- return -EINVAL;
-
p = br_port_get(dev);
- if (p->br != br)
+ if (!p || p->br != br)
return -EINVAL;
del_nbp(p);
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 865fd76..ce8b2ee 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -131,17 +131,18 @@
static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
{
- if (!br_port_exists(dev))
- return NULL;
- return &br_port_get_rcu(dev)->br->fake_rtable;
+ struct net_bridge_port *port;
+
+ port = br_port_get_rcu(dev);
+ return port ? &port->br->fake_rtable : NULL;
}
static inline struct net_device *bridge_parent(const struct net_device *dev)
{
- if (!br_port_exists(dev))
- return NULL;
+ struct net_bridge_port *port;
- return br_port_get_rcu(dev)->br->dev;
+ port = br_port_get_rcu(dev);
+ return port ? port->br->dev : NULL;
}
static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 4a6a378..e3de0a4 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -119,11 +119,13 @@
idx = 0;
for_each_netdev(net, dev) {
+ struct net_bridge_port *port = br_port_get(dev);
+
/* not a bridge port */
- if (!br_port_exists(dev) || idx < cb->args[0])
+ if (!port || idx < cb->args[0])
goto skip;
- if (br_fill_ifinfo(skb, br_port_get(dev),
+ if (br_fill_ifinfo(skb, port,
NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq, RTM_NEWLINK,
NLM_F_MULTI) < 0)
@@ -169,9 +171,9 @@
if (!dev)
return -ENODEV;
- if (!br_port_exists(dev))
- return -EINVAL;
p = br_port_get(dev);
+ if (!p)
+ return -EINVAL;
/* if kernel STP is running, don't allow changes */
if (p->br->stp_enabled == BR_KERNEL_STP)
diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 404d4e1..ef2175c 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -32,7 +32,7 @@
static int br_device_event(struct notifier_block *unused, unsigned long event, void *ptr)
{
struct net_device *dev = ptr;
- struct net_bridge_port *p = br_port_get(dev);
+ struct net_bridge_port *p;
struct net_bridge *br;
int err;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b862071..46e0bec 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -151,11 +151,19 @@
#endif
};
-#define br_port_get_rcu(dev) \
- ((struct net_bridge_port *) rcu_dereference(dev->rx_handler_data))
-#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data)
#define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
+static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev)
+{
+ struct net_bridge_port *port = rcu_dereference(dev->rx_handler_data);
+ return br_port_exists(dev) ? port : NULL;
+}
+
+static inline struct net_bridge_port *br_port_get(struct net_device *dev)
+{
+ return br_port_exists(dev) ? dev->rx_handler_data : NULL;
+}
+
struct br_cpu_netstats {
u64 rx_packets;
u64 rx_bytes;
diff --git a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
index 35cf270..3d9a55d 100644
--- a/net/bridge/br_stp_bpdu.c
+++ b/net/bridge/br_stp_bpdu.c
@@ -141,10 +141,6 @@
struct net_bridge *br;
const unsigned char *buf;
- if (!br_port_exists(dev))
- goto err;
- p = br_port_get_rcu(dev);
-
if (!pskb_may_pull(skb, 4))
goto err;
@@ -153,6 +149,10 @@
if (buf[0] != 0 || buf[1] != 0 || buf[2] != 0)
goto err;
+ p = br_port_get_rcu(dev);
+ if (!p)
+ goto err;
+
br = p->br;
spin_lock(&br->lock);
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index a1dcf83..cbc9f39 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -128,6 +128,7 @@
const struct net_device *in, const struct net_device *out)
{
const struct ethhdr *h = eth_hdr(skb);
+ const struct net_bridge_port *p;
__be16 ethproto;
int verdict, i;
@@ -148,13 +149,11 @@
if (FWINV2(ebt_dev_check(e->out, out), EBT_IOUT))
return 1;
/* rcu_read_lock()ed by nf_hook_slow */
- if (in && br_port_exists(in) &&
- FWINV2(ebt_dev_check(e->logical_in, br_port_get_rcu(in)->br->dev),
- EBT_ILOGICALIN))
+ if (in && (p = br_port_get_rcu(in)) != NULL &&
+ FWINV2(ebt_dev_check(e->logical_in, p->br->dev), EBT_ILOGICALIN))
return 1;
- if (out && br_port_exists(out) &&
- FWINV2(ebt_dev_check(e->logical_out, br_port_get_rcu(out)->br->dev),
- EBT_ILOGICALOUT))
+ if (out && (p = br_port_get_rcu(out)) != NULL &&
+ FWINV2(ebt_dev_check(e->logical_out, p->br->dev), EBT_ILOGICALOUT))
return 1;
if (e->bitmask & EBT_SOURCEMAC) {