Commit 2dab80a8 authored by Nikolay Aleksandrov's avatar Nikolay Aleksandrov Committed by David S. Miller

bridge: fix br_stp_set_bridge_priority race conditions

After the ->set() spinlocks were removed br_stp_set_bridge_priority
was left running without any protection when used via sysfs. It can
race with port add/del and could result in use-after-free cases and
corrupted lists. Tested by running port add/del in a loop with stp
enabled while setting priority in a loop, crashes are easily
reproducible.
The spinlocks around sysfs ->set() were removed in commit:
14f98f25 ("bridge: range check STP parameters")
There's also a race condition in the netlink priority support that is
fixed by this change, but it was introduced recently and the fixes tag
covers it, just in case it's needed the commit is:
af615762 ("bridge: add ageing_time, stp_state, priority over netlink")
Signed-off-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
Fixes: 14f98f25 ("bridge: range check STP parameters")
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent ac0a72a3
...@@ -247,9 +247,7 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) ...@@ -247,9 +247,7 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN))
return -EPERM; return -EPERM;
spin_lock_bh(&br->lock);
br_stp_set_bridge_priority(br, args[1]); br_stp_set_bridge_priority(br, args[1]);
spin_unlock_bh(&br->lock);
return 0; return 0;
case BRCTL_SET_PORT_PRIORITY: case BRCTL_SET_PORT_PRIORITY:
......
...@@ -243,12 +243,13 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br) ...@@ -243,12 +243,13 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br)
return true; return true;
} }
/* called under bridge lock */ /* Acquires and releases bridge lock */
void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio) void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio)
{ {
struct net_bridge_port *p; struct net_bridge_port *p;
int wasroot; int wasroot;
spin_lock_bh(&br->lock);
wasroot = br_is_root_bridge(br); wasroot = br_is_root_bridge(br);
list_for_each_entry(p, &br->port_list, list) { list_for_each_entry(p, &br->port_list, list) {
...@@ -266,6 +267,7 @@ void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio) ...@@ -266,6 +267,7 @@ void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio)
br_port_state_selection(br); br_port_state_selection(br);
if (br_is_root_bridge(br) && !wasroot) if (br_is_root_bridge(br) && !wasroot)
br_become_root_bridge(br); br_become_root_bridge(br);
spin_unlock_bh(&br->lock);
} }
/* called under bridge lock */ /* called under bridge lock */
......
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