Offloaded xfrm states are currently added to a new device after curr_active_slave is updated to direct traffic to it. This could result in the following race, which could lead to unencrypted IPSec packets on the wire: CPU1 (xfrm_output) CPU2 (bond_change_active_slave) bond_ipsec_offload_ok -> true bond->curr_active_slave = new_dev bond_ipsec_migrate_sa_all bond_xmit_activebackup bond_dev_queue_xmit dev_queue_xmit on new_dev bond_ipsec_migrate_sa_all finishes So the packet can make it out to new_dev before its xfrm_state is offloaded to it. The result: an unencrypted IPSec packet on the wire. This patch closes this race by introducing a new pointer in the bond device, named 'xfrm_active_slave'. All xfrm_states offloaded to the bond device get offloaded to it. Changing the current active slave will now first update this new pointer, then migrate all xfrm states on the bond device, then flip curr_active_slave. This closes the above race and makes sure that any IPSec packets transmitted on the new device will find an offloaded xfrm_state on the device. Fixes: 9a5605505d9c ("bonding: Add struct bond_ipesc to manage SA") Signed-off-by: Cosmin Ratiu --- drivers/net/bonding/bond_main.c | 15 ++++++++------- include/net/bonding.h | 2 ++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index e45e89179236..98d5f9974086 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -438,7 +438,7 @@ static struct net_device *bond_ipsec_dev(struct xfrm_state *xs) if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) return NULL; - slave = rcu_dereference(bond->curr_active_slave); + slave = rcu_dereference(bond->xfrm_active_slave); if (!slave) return NULL; @@ -474,7 +474,7 @@ static int bond_ipsec_add_sa(struct net_device *bond_dev, rcu_read_lock(); bond = netdev_priv(bond_dev); - slave = rcu_dereference(bond->curr_active_slave); + slave = rcu_dereference(bond->xfrm_active_slave); real_dev = slave ? slave->dev : NULL; netdev_hold(real_dev, &tracker, GFP_ATOMIC); rcu_read_unlock(); @@ -515,7 +515,7 @@ static int bond_ipsec_add_sa(struct net_device *bond_dev, static void bond_ipsec_migrate_sa_all(struct bonding *bond) { - struct slave *new_active = rtnl_dereference(bond->curr_active_slave); + struct slave *new_active = rtnl_dereference(bond->xfrm_active_slave); struct net_device *bond_dev = bond->dev; struct net *net = dev_net(bond_dev); struct bond_ipsec *ipsec, *tmp; @@ -1216,6 +1216,11 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active) if (bond_uses_primary(bond)) bond_hw_addr_swap(bond, new_active, old_active); +#ifdef CONFIG_XFRM_OFFLOAD + rcu_assign_pointer(bond->xfrm_active_slave, new_active); + bond_ipsec_migrate_sa_all(bond); +#endif + if (bond_is_lb(bond)) { bond_alb_handle_active_change(bond, new_active); if (old_active) @@ -1228,10 +1233,6 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active) rcu_assign_pointer(bond->curr_active_slave, new_active); } -#ifdef CONFIG_XFRM_OFFLOAD - bond_ipsec_migrate_sa_all(bond); -#endif - if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP) { if (old_active) bond_set_slave_inactive_flags(old_active, diff --git a/include/net/bonding.h b/include/net/bonding.h index 49edc7da0586..256fe96fcfda 100644 --- a/include/net/bonding.h +++ b/include/net/bonding.h @@ -260,6 +260,8 @@ struct bonding { #endif /* CONFIG_DEBUG_FS */ struct rtnl_link_stats64 bond_stats; #ifdef CONFIG_XFRM_OFFLOAD + /* The device where new xfrm states will be offloaded */ + struct slave __rcu *xfrm_active_slave; struct list_head ipsec_list; /* protecting ipsec_list */ struct mutex ipsec_lock; -- 2.45.0