Commit b5ed54e9 authored by stephen hemminger's avatar stephen hemminger Committed by David S. Miller

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: default avatarStephen Hemminger <shemminger@vyatta.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 61391cde
...@@ -238,15 +238,18 @@ struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br, ...@@ -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) int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
{ {
struct net_bridge_fdb_entry *fdb; struct net_bridge_fdb_entry *fdb;
struct net_bridge_port *port;
int ret; int ret;
if (!br_port_exists(dev))
return 0;
rcu_read_lock(); rcu_read_lock();
fdb = __br_fdb_get(br_port_get_rcu(dev)->br, addr); port = br_port_get_rcu(dev);
if (!port)
ret = 0;
else {
fdb = __br_fdb_get(port->br, addr);
ret = fdb && fdb->dst->dev != dev && ret = fdb && fdb->dst->dev != dev &&
fdb->dst->state == BR_STATE_FORWARDING; fdb->dst->state == BR_STATE_FORWARDING;
}
rcu_read_unlock(); rcu_read_unlock();
return ret; return ret;
......
...@@ -475,11 +475,8 @@ int br_del_if(struct net_bridge *br, struct net_device *dev) ...@@ -475,11 +475,8 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
{ {
struct net_bridge_port *p; struct net_bridge_port *p;
if (!br_port_exists(dev))
return -EINVAL;
p = br_port_get(dev); p = br_port_get(dev);
if (p->br != br) if (!p || p->br != br)
return -EINVAL; return -EINVAL;
del_nbp(p); del_nbp(p);
......
...@@ -131,17 +131,18 @@ void br_netfilter_rtable_init(struct net_bridge *br) ...@@ -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) static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
{ {
if (!br_port_exists(dev)) struct net_bridge_port *port;
return NULL;
return &br_port_get_rcu(dev)->br->fake_rtable; 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) static inline struct net_device *bridge_parent(const struct net_device *dev)
{ {
if (!br_port_exists(dev)) struct net_bridge_port *port;
return NULL;
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) static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
......
...@@ -119,11 +119,13 @@ static int br_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb) ...@@ -119,11 +119,13 @@ static int br_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
idx = 0; idx = 0;
for_each_netdev(net, dev) { for_each_netdev(net, dev) {
struct net_bridge_port *port = br_port_get(dev);
/* not a bridge port */ /* not a bridge port */
if (!br_port_exists(dev) || idx < cb->args[0]) if (!port || idx < cb->args[0])
goto skip; goto skip;
if (br_fill_ifinfo(skb, br_port_get(dev), if (br_fill_ifinfo(skb, port,
NETLINK_CB(cb->skb).pid, NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq, RTM_NEWLINK, cb->nlh->nlmsg_seq, RTM_NEWLINK,
NLM_F_MULTI) < 0) NLM_F_MULTI) < 0)
...@@ -169,9 +171,9 @@ static int br_rtm_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg) ...@@ -169,9 +171,9 @@ static int br_rtm_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
if (!dev) if (!dev)
return -ENODEV; return -ENODEV;
if (!br_port_exists(dev))
return -EINVAL;
p = br_port_get(dev); p = br_port_get(dev);
if (!p)
return -EINVAL;
/* if kernel STP is running, don't allow changes */ /* if kernel STP is running, don't allow changes */
if (p->br->stp_enabled == BR_KERNEL_STP) if (p->br->stp_enabled == BR_KERNEL_STP)
......
...@@ -32,7 +32,7 @@ struct notifier_block br_device_notifier = { ...@@ -32,7 +32,7 @@ struct notifier_block br_device_notifier = {
static int br_device_event(struct notifier_block *unused, unsigned long event, void *ptr) static int br_device_event(struct notifier_block *unused, unsigned long event, void *ptr)
{ {
struct net_device *dev = ptr; struct net_device *dev = ptr;
struct net_bridge_port *p = br_port_get(dev); struct net_bridge_port *p;
struct net_bridge *br; struct net_bridge *br;
int err; int err;
......
...@@ -151,11 +151,19 @@ struct net_bridge_port ...@@ -151,11 +151,19 @@ struct net_bridge_port
#endif #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) #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 { struct br_cpu_netstats {
u64 rx_packets; u64 rx_packets;
u64 rx_bytes; u64 rx_bytes;
......
...@@ -141,10 +141,6 @@ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, ...@@ -141,10 +141,6 @@ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
struct net_bridge *br; struct net_bridge *br;
const unsigned char *buf; const unsigned char *buf;
if (!br_port_exists(dev))
goto err;
p = br_port_get_rcu(dev);
if (!pskb_may_pull(skb, 4)) if (!pskb_may_pull(skb, 4))
goto err; goto err;
...@@ -153,6 +149,10 @@ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, ...@@ -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) if (buf[0] != 0 || buf[1] != 0 || buf[2] != 0)
goto err; goto err;
p = br_port_get_rcu(dev);
if (!p)
goto err;
br = p->br; br = p->br;
spin_lock(&br->lock); spin_lock(&br->lock);
......
...@@ -128,6 +128,7 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, ...@@ -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 net_device *in, const struct net_device *out)
{ {
const struct ethhdr *h = eth_hdr(skb); const struct ethhdr *h = eth_hdr(skb);
const struct net_bridge_port *p;
__be16 ethproto; __be16 ethproto;
int verdict, i; int verdict, i;
...@@ -148,13 +149,11 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, ...@@ -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)) if (FWINV2(ebt_dev_check(e->out, out), EBT_IOUT))
return 1; return 1;
/* rcu_read_lock()ed by nf_hook_slow */ /* rcu_read_lock()ed by nf_hook_slow */
if (in && br_port_exists(in) && if (in && (p = br_port_get_rcu(in)) != NULL &&
FWINV2(ebt_dev_check(e->logical_in, br_port_get_rcu(in)->br->dev), FWINV2(ebt_dev_check(e->logical_in, p->br->dev), EBT_ILOGICALIN))
EBT_ILOGICALIN))
return 1; return 1;
if (out && br_port_exists(out) && if (out && (p = br_port_get_rcu(out)) != NULL &&
FWINV2(ebt_dev_check(e->logical_out, br_port_get_rcu(out)->br->dev), FWINV2(ebt_dev_check(e->logical_out, p->br->dev), EBT_ILOGICALOUT))
EBT_ILOGICALOUT))
return 1; return 1;
if (e->bitmask & EBT_SOURCEMAC) { if (e->bitmask & EBT_SOURCEMAC) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment