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>

8 files changed, 44 insertions, 34 deletions

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 90512ccfd3e9..2872393b2939 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -238,15 +238,18 @@ struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
 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 89ad25a76202..427f90a8ab7b 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -475,11 +475,8 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 {
         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 865fd7634b67..ce8b2eed4e73 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -131,17 +131,18 @@ void br_netfilter_rtable_init(struct net_bridge *br)
 
 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 4a6a378c84e3..e3de0a428f5d 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -119,11 +119,13 @@ static int br_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 
         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 @@ static int br_rtm_setlink(struct sk_buff *skb,  struct nlmsghdr *nlh, void *arg)
         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 404d4e14c6a7..ef2175c8b91d 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -32,7 +32,7 @@ struct notifier_block br_device_notifier = {
 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 b862071bf601..46e0bec1d7c5 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -151,11 +151,19 @@ struct net_bridge_port
 #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 35cf27087b56..3d9a55d3822f 100644
--- a/net/bridge/br_stp_bpdu.c
+++ b/net/bridge/br_stp_bpdu.c
@@ -141,10 +141,6 @@ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
         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 @@ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
         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 a1dcf83f0d58..cbc9f395ab1e 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -128,6 +128,7 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb,
                 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 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb,
         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) {