Commit 059b47e8 authored by Nikolay Aleksandrov's avatar Nikolay Aleksandrov Committed by David S. Miller

bonding: convert primary_slave to use RCU

This is necessary mainly for two bonding call sites: procfs and
sysfs as it was dereferenced without any real protection.
Signed-off-by: default avatarNikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent ecfede42
...@@ -708,7 +708,7 @@ static void bond_do_fail_over_mac(struct bonding *bond, ...@@ -708,7 +708,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
static bool bond_should_change_active(struct bonding *bond) static bool bond_should_change_active(struct bonding *bond)
{ {
struct slave *prim = bond->primary_slave; struct slave *prim = rtnl_dereference(bond->primary_slave);
struct slave *curr = bond_deref_active_protected(bond); struct slave *curr = bond_deref_active_protected(bond);
if (!prim || !curr || curr->link != BOND_LINK_UP) if (!prim || !curr || curr->link != BOND_LINK_UP)
...@@ -732,13 +732,14 @@ static bool bond_should_change_active(struct bonding *bond) ...@@ -732,13 +732,14 @@ static bool bond_should_change_active(struct bonding *bond)
*/ */
static struct slave *bond_find_best_slave(struct bonding *bond) static struct slave *bond_find_best_slave(struct bonding *bond)
{ {
struct slave *slave, *bestslave = NULL; struct slave *slave, *bestslave = NULL, *primary;
struct list_head *iter; struct list_head *iter;
int mintime = bond->params.updelay; int mintime = bond->params.updelay;
if (bond->primary_slave && bond->primary_slave->link == BOND_LINK_UP && primary = rtnl_dereference(bond->primary_slave);
if (primary && primary->link == BOND_LINK_UP &&
bond_should_change_active(bond)) bond_should_change_active(bond))
return bond->primary_slave; return primary;
bond_for_each_slave(bond, slave, iter) { bond_for_each_slave(bond, slave, iter) {
if (slave->link == BOND_LINK_UP) if (slave->link == BOND_LINK_UP)
...@@ -1482,7 +1483,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) ...@@ -1482,7 +1483,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
if (bond_uses_primary(bond) && bond->params.primary[0]) { if (bond_uses_primary(bond) && bond->params.primary[0]) {
/* if there is a primary slave, remember it */ /* if there is a primary slave, remember it */
if (strcmp(bond->params.primary, new_slave->dev->name) == 0) { if (strcmp(bond->params.primary, new_slave->dev->name) == 0) {
bond->primary_slave = new_slave; rcu_assign_pointer(bond->primary_slave, new_slave);
bond->force_primary = true; bond->force_primary = true;
} }
} }
...@@ -1596,8 +1597,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) ...@@ -1596,8 +1597,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
bond_hw_addr_flush(bond_dev, slave_dev); bond_hw_addr_flush(bond_dev, slave_dev);
vlan_vids_del_by_dev(slave_dev, bond_dev); vlan_vids_del_by_dev(slave_dev, bond_dev);
if (bond->primary_slave == new_slave) if (rcu_access_pointer(bond->primary_slave) == new_slave)
bond->primary_slave = NULL; RCU_INIT_POINTER(bond->primary_slave, NULL);
if (rcu_access_pointer(bond->curr_active_slave) == new_slave) { if (rcu_access_pointer(bond->curr_active_slave) == new_slave) {
block_netpoll_tx(); block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock); write_lock_bh(&bond->curr_slave_lock);
...@@ -1606,6 +1607,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) ...@@ -1606,6 +1607,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
write_unlock_bh(&bond->curr_slave_lock); write_unlock_bh(&bond->curr_slave_lock);
unblock_netpoll_tx(); unblock_netpoll_tx();
} }
/* either primary_slave or curr_active_slave might've changed */
synchronize_rcu();
slave_disable_netpoll(new_slave); slave_disable_netpoll(new_slave);
err_close: err_close:
...@@ -1714,8 +1717,8 @@ static int __bond_release_one(struct net_device *bond_dev, ...@@ -1714,8 +1717,8 @@ static int __bond_release_one(struct net_device *bond_dev,
bond_dev->name, slave_dev->name); bond_dev->name, slave_dev->name);
} }
if (bond->primary_slave == slave) if (rtnl_dereference(bond->primary_slave) == slave)
bond->primary_slave = NULL; RCU_INIT_POINTER(bond->primary_slave, NULL);
if (oldcurrent == slave) { if (oldcurrent == slave) {
write_lock_bh(&bond->curr_slave_lock); write_lock_bh(&bond->curr_slave_lock);
...@@ -1976,7 +1979,7 @@ static int bond_miimon_inspect(struct bonding *bond) ...@@ -1976,7 +1979,7 @@ static int bond_miimon_inspect(struct bonding *bond)
static void bond_miimon_commit(struct bonding *bond) static void bond_miimon_commit(struct bonding *bond)
{ {
struct list_head *iter; struct list_head *iter;
struct slave *slave; struct slave *slave, *primary;
bond_for_each_slave(bond, slave, iter) { bond_for_each_slave(bond, slave, iter) {
switch (slave->new_link) { switch (slave->new_link) {
...@@ -1987,13 +1990,14 @@ static void bond_miimon_commit(struct bonding *bond) ...@@ -1987,13 +1990,14 @@ static void bond_miimon_commit(struct bonding *bond)
slave->link = BOND_LINK_UP; slave->link = BOND_LINK_UP;
slave->last_link_up = jiffies; slave->last_link_up = jiffies;
primary = rtnl_dereference(bond->primary_slave);
if (BOND_MODE(bond) == BOND_MODE_8023AD) { if (BOND_MODE(bond) == BOND_MODE_8023AD) {
/* prevent it from being the active one */ /* prevent it from being the active one */
bond_set_backup_slave(slave); bond_set_backup_slave(slave);
} else if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) { } else if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {
/* make it immediately active */ /* make it immediately active */
bond_set_active_slave(slave); bond_set_active_slave(slave);
} else if (slave != bond->primary_slave) { } else if (slave != primary) {
/* prevent it from being the active one */ /* prevent it from being the active one */
bond_set_backup_slave(slave); bond_set_backup_slave(slave);
} }
...@@ -2011,8 +2015,7 @@ static void bond_miimon_commit(struct bonding *bond) ...@@ -2011,8 +2015,7 @@ static void bond_miimon_commit(struct bonding *bond)
bond_alb_handle_link_change(bond, slave, bond_alb_handle_link_change(bond, slave,
BOND_LINK_UP); BOND_LINK_UP);
if (!bond->curr_active_slave || if (!bond->curr_active_slave || slave == primary)
(slave == bond->primary_slave))
goto do_failover; goto do_failover;
continue; continue;
...@@ -2633,7 +2636,7 @@ static void bond_ab_arp_commit(struct bonding *bond) ...@@ -2633,7 +2636,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
slave->dev->name); slave->dev->name);
if (!rtnl_dereference(bond->curr_active_slave) || if (!rtnl_dereference(bond->curr_active_slave) ||
(slave == bond->primary_slave)) slave == rtnl_dereference(bond->primary_slave))
goto do_failover; goto do_failover;
} }
...@@ -2860,7 +2863,7 @@ static int bond_master_netdev_event(unsigned long event, ...@@ -2860,7 +2863,7 @@ static int bond_master_netdev_event(unsigned long event,
static int bond_slave_netdev_event(unsigned long event, static int bond_slave_netdev_event(unsigned long event,
struct net_device *slave_dev) struct net_device *slave_dev)
{ {
struct slave *slave = bond_slave_get_rtnl(slave_dev); struct slave *slave = bond_slave_get_rtnl(slave_dev), *primary;
struct bonding *bond; struct bonding *bond;
struct net_device *bond_dev; struct net_device *bond_dev;
u32 old_speed; u32 old_speed;
...@@ -2874,6 +2877,7 @@ static int bond_slave_netdev_event(unsigned long event, ...@@ -2874,6 +2877,7 @@ static int bond_slave_netdev_event(unsigned long event,
return NOTIFY_DONE; return NOTIFY_DONE;
bond_dev = slave->bond->dev; bond_dev = slave->bond->dev;
bond = slave->bond; bond = slave->bond;
primary = rtnl_dereference(bond->primary_slave);
switch (event) { switch (event) {
case NETDEV_UNREGISTER: case NETDEV_UNREGISTER:
...@@ -2921,18 +2925,18 @@ static int bond_slave_netdev_event(unsigned long event, ...@@ -2921,18 +2925,18 @@ static int bond_slave_netdev_event(unsigned long event,
!bond->params.primary[0]) !bond->params.primary[0])
break; break;
if (slave == bond->primary_slave) { if (slave == primary) {
/* slave's name changed - he's no longer primary */ /* slave's name changed - he's no longer primary */
bond->primary_slave = NULL; RCU_INIT_POINTER(bond->primary_slave, NULL);
} else if (!strcmp(slave_dev->name, bond->params.primary)) { } else if (!strcmp(slave_dev->name, bond->params.primary)) {
/* we have a new primary slave */ /* we have a new primary slave */
bond->primary_slave = slave; rcu_assign_pointer(bond->primary_slave, slave);
} else { /* we didn't change primary - exit */ } else { /* we didn't change primary - exit */
break; break;
} }
netdev_info(bond->dev, "Primary slave changed to %s, reselecting active slave\n", netdev_info(bond->dev, "Primary slave changed to %s, reselecting active slave\n",
bond->primary_slave ? slave_dev->name : "none"); primary ? slave_dev->name : "none");
block_netpoll_tx(); block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock); write_lock_bh(&bond->curr_slave_lock);
......
...@@ -443,6 +443,7 @@ static int bond_fill_info(struct sk_buff *skb, ...@@ -443,6 +443,7 @@ static int bond_fill_info(struct sk_buff *skb,
unsigned int packets_per_slave; unsigned int packets_per_slave;
int ifindex, i, targets_added; int ifindex, i, targets_added;
struct nlattr *targets; struct nlattr *targets;
struct slave *primary;
if (nla_put_u8(skb, IFLA_BOND_MODE, BOND_MODE(bond))) if (nla_put_u8(skb, IFLA_BOND_MODE, BOND_MODE(bond)))
goto nla_put_failure; goto nla_put_failure;
...@@ -492,9 +493,9 @@ static int bond_fill_info(struct sk_buff *skb, ...@@ -492,9 +493,9 @@ static int bond_fill_info(struct sk_buff *skb,
bond->params.arp_all_targets)) bond->params.arp_all_targets))
goto nla_put_failure; goto nla_put_failure;
if (bond->primary_slave && primary = rtnl_dereference(bond->primary_slave);
nla_put_u32(skb, IFLA_BOND_PRIMARY, if (primary &&
bond->primary_slave->dev->ifindex)) nla_put_u32(skb, IFLA_BOND_PRIMARY, primary->dev->ifindex))
goto nla_put_failure; goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_PRIMARY_RESELECT, if (nla_put_u8(skb, IFLA_BOND_PRIMARY_RESELECT,
......
...@@ -1090,7 +1090,7 @@ static int bond_option_primary_set(struct bonding *bond, ...@@ -1090,7 +1090,7 @@ static int bond_option_primary_set(struct bonding *bond,
/* check to see if we are clearing primary */ /* check to see if we are clearing primary */
if (!strlen(primary)) { if (!strlen(primary)) {
netdev_info(bond->dev, "Setting primary slave to None\n"); netdev_info(bond->dev, "Setting primary slave to None\n");
bond->primary_slave = NULL; RCU_INIT_POINTER(bond->primary_slave, NULL);
memset(bond->params.primary, 0, sizeof(bond->params.primary)); memset(bond->params.primary, 0, sizeof(bond->params.primary));
bond_select_active_slave(bond); bond_select_active_slave(bond);
goto out; goto out;
...@@ -1100,16 +1100,16 @@ static int bond_option_primary_set(struct bonding *bond, ...@@ -1100,16 +1100,16 @@ static int bond_option_primary_set(struct bonding *bond,
if (strncmp(slave->dev->name, primary, IFNAMSIZ) == 0) { if (strncmp(slave->dev->name, primary, IFNAMSIZ) == 0) {
netdev_info(bond->dev, "Setting %s as primary slave\n", netdev_info(bond->dev, "Setting %s as primary slave\n",
slave->dev->name); slave->dev->name);
bond->primary_slave = slave; rcu_assign_pointer(bond->primary_slave, slave);
strcpy(bond->params.primary, slave->dev->name); strcpy(bond->params.primary, slave->dev->name);
bond_select_active_slave(bond); bond_select_active_slave(bond);
goto out; goto out;
} }
} }
if (bond->primary_slave) { if (rtnl_dereference(bond->primary_slave)) {
netdev_info(bond->dev, "Setting primary slave to None\n"); netdev_info(bond->dev, "Setting primary slave to None\n");
bond->primary_slave = NULL; RCU_INIT_POINTER(bond->primary_slave, NULL);
bond_select_active_slave(bond); bond_select_active_slave(bond);
} }
strncpy(bond->params.primary, primary, IFNAMSIZ); strncpy(bond->params.primary, primary, IFNAMSIZ);
......
...@@ -66,7 +66,7 @@ static void bond_info_show_master(struct seq_file *seq) ...@@ -66,7 +66,7 @@ static void bond_info_show_master(struct seq_file *seq)
{ {
struct bonding *bond = seq->private; struct bonding *bond = seq->private;
const struct bond_opt_value *optval; const struct bond_opt_value *optval;
struct slave *curr; struct slave *curr, *primary;
int i; int i;
curr = rcu_dereference(bond->curr_active_slave); curr = rcu_dereference(bond->curr_active_slave);
...@@ -92,10 +92,10 @@ static void bond_info_show_master(struct seq_file *seq) ...@@ -92,10 +92,10 @@ static void bond_info_show_master(struct seq_file *seq)
} }
if (bond_uses_primary(bond)) { if (bond_uses_primary(bond)) {
primary = rcu_dereference(bond->primary_slave);
seq_printf(seq, "Primary Slave: %s", seq_printf(seq, "Primary Slave: %s",
(bond->primary_slave) ? primary ? primary->dev->name : "None");
bond->primary_slave->dev->name : "None"); if (primary) {
if (bond->primary_slave) {
optval = bond_opt_get_val(BOND_OPT_PRIMARY_RESELECT, optval = bond_opt_get_val(BOND_OPT_PRIMARY_RESELECT,
bond->params.primary_reselect); bond->params.primary_reselect);
seq_printf(seq, " (primary_reselect %s)", seq_printf(seq, " (primary_reselect %s)",
......
...@@ -425,11 +425,15 @@ static ssize_t bonding_show_primary(struct device *d, ...@@ -425,11 +425,15 @@ static ssize_t bonding_show_primary(struct device *d,
struct device_attribute *attr, struct device_attribute *attr,
char *buf) char *buf)
{ {
int count = 0;
struct bonding *bond = to_bond(d); struct bonding *bond = to_bond(d);
struct slave *primary;
int count = 0;
if (bond->primary_slave) rcu_read_lock();
count = sprintf(buf, "%s\n", bond->primary_slave->dev->name); primary = rcu_dereference(bond->primary_slave);
if (primary)
count = sprintf(buf, "%s\n", primary->dev->name);
rcu_read_unlock();
return count; return count;
} }
......
...@@ -195,7 +195,7 @@ struct bonding { ...@@ -195,7 +195,7 @@ struct bonding {
struct net_device *dev; /* first - useful for panic debug */ struct net_device *dev; /* first - useful for panic debug */
struct slave __rcu *curr_active_slave; struct slave __rcu *curr_active_slave;
struct slave __rcu *current_arp_slave; struct slave __rcu *current_arp_slave;
struct slave *primary_slave; struct slave __rcu *primary_slave;
bool force_primary; bool force_primary;
s32 slave_cnt; /* never change this value outside the attach/detach wrappers */ s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
int (*recv_probe)(const struct sk_buff *, struct bonding *, int (*recv_probe)(const struct sk_buff *, struct bonding *,
......
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