Commit f6c3665b authored by Eric Dumazet's avatar Eric Dumazet Committed by Jakub Kicinski
Browse files

bonding: annotate data-races around slave->last_rx 



slave->last_rx and slave->target_last_arp_rx[...] can be read and written
locklessly. Add READ_ONCE() and WRITE_ONCE() annotations.

syzbot reported:

BUG: KCSAN: data-race in bond_rcv_validate / bond_rcv_validate

write to 0xffff888149f0d428 of 8 bytes by interrupt on cpu 1:
  bond_rcv_validate+0x202/0x7a0 drivers/net/bonding/bond_main.c:3335
  bond_handle_frame+0xde/0x5e0 drivers/net/bonding/bond_main.c:1533
  __netif_receive_skb_core+0x5b1/0x1950 net/core/dev.c:6039
  __netif_receive_skb_one_core net/core/dev.c:6150 [inline]
  __netif_receive_skb+0x59/0x270 net/core/dev.c:6265
  netif_receive_skb_internal net/core/dev.c:6351 [inline]
  netif_receive_skb+0x4b/0x2d0 net/core/dev.c:6410
...

write to 0xffff888149f0d428 of 8 bytes by interrupt on cpu 0:
  bond_rcv_validate+0x202/0x7a0 drivers/net/bonding/bond_main.c:3335
  bond_handle_frame+0xde/0x5e0 drivers/net/bonding/bond_main.c:1533
  __netif_receive_skb_core+0x5b1/0x1950 net/core/dev.c:6039
  __netif_receive_skb_one_core net/core/dev.c:6150 [inline]
  __netif_receive_skb+0x59/0x270 net/core/dev.c:6265
  netif_receive_skb_internal net/core/dev.c:6351 [inline]
  netif_receive_skb+0x4b/0x2d0 net/core/dev.c:6410
  br_netif_receive_skb net/bridge/br_input.c:30 [inline]
  NF_HOOK include/linux/netfilter.h:318 [inline]
...

value changed: 0x0000000100005365 -> 0x0000000100005366

Fixes: f5b2b966 ("[PATCH] bonding: Validate probe replies in ARP monitor")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Link: https://patch.msgid.link/20260122162914.2299312-1-edumazet@google.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 8016dc5e

drivers/net/bonding/bond_main.c

+10 −8
Original line number Diff line number Diff line
@@ -3047,8 +3047,8 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32 
			   __func__, &sip);

		return;

	}

	slave->last_rx = jiffies;

	slave->target_last_arp_rx[i] = jiffies;

	WRITE_ONCE(slave->last_rx, jiffies);

	WRITE_ONCE(slave->target_last_arp_rx[i], jiffies);

}



static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
@@ -3267,8 +3267,8 @@ static void bond_validate_na(struct bonding *bond, struct slave *slave, 
			  __func__, saddr);

		return;

	}

	slave->last_rx = jiffies;

	slave->target_last_arp_rx[i] = jiffies;

	WRITE_ONCE(slave->last_rx, jiffies);

	WRITE_ONCE(slave->target_last_arp_rx[i], jiffies);

}



static int bond_na_rcv(const struct sk_buff *skb, struct bonding *bond,
@@ -3338,7 +3338,7 @@ int bond_rcv_validate(const struct sk_buff *skb, struct bonding *bond, 
		    (slave_do_arp_validate_only(bond) && is_ipv6) ||

#endif

		    !slave_do_arp_validate_only(bond))

			slave->last_rx = jiffies;

			WRITE_ONCE(slave->last_rx, jiffies);

		return RX_HANDLER_ANOTHER;

	} else if (is_arp) {

		return bond_arp_rcv(skb, bond, slave);
@@ -3406,7 +3406,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond) 


		if (slave->link != BOND_LINK_UP) {

			if (bond_time_in_interval(bond, last_tx, 1) &&

			    bond_time_in_interval(bond, slave->last_rx, 1)) {

			    bond_time_in_interval(bond, READ_ONCE(slave->last_rx), 1)) {



				bond_propose_link_state(slave, BOND_LINK_UP);

				slave_state_changed = 1;
@@ -3430,8 +3430,10 @@ static void bond_loadbalance_arp_mon(struct bonding *bond) 
			 * when the source ip is 0, so don't take the link down

			 * if we don't know our ip yet

			 */

			if (!bond_time_in_interval(bond, last_tx, bond->params.missed_max) ||

			    !bond_time_in_interval(bond, slave->last_rx, bond->params.missed_max)) {

			if (!bond_time_in_interval(bond, last_tx,

						   bond->params.missed_max) ||

			    !bond_time_in_interval(bond, READ_ONCE(slave->last_rx),

						   bond->params.missed_max)) {



				bond_propose_link_state(slave, BOND_LINK_DOWN);

				slave_state_changed = 1;

drivers/net/bonding/bond_options.c

+4 −4
Original line number Diff line number Diff line
@@ -1152,7 +1152,7 @@ static void _bond_options_arp_ip_target_set(struct bonding *bond, int slot, 


	if (slot >= 0 && slot < BOND_MAX_ARP_TARGETS) {

		bond_for_each_slave(bond, slave, iter)

			slave->target_last_arp_rx[slot] = last_rx;

			WRITE_ONCE(slave->target_last_arp_rx[slot], last_rx);

		targets[slot] = target;

	}

}
@@ -1221,8 +1221,8 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target) 
	bond_for_each_slave(bond, slave, iter) {

		targets_rx = slave->target_last_arp_rx;

		for (i = ind; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)

			targets_rx[i] = targets_rx[i+1];

		targets_rx[i] = 0;

			WRITE_ONCE(targets_rx[i], READ_ONCE(targets_rx[i+1]));

		WRITE_ONCE(targets_rx[i], 0);

	}

	for (i = ind; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)

		targets[i] = targets[i+1];
@@ -1377,7 +1377,7 @@ static void _bond_options_ns_ip6_target_set(struct bonding *bond, int slot, 


	if (slot >= 0 && slot < BOND_MAX_NS_TARGETS) {

		bond_for_each_slave(bond, slave, iter) {

			slave->target_last_arp_rx[slot] = last_rx;

			WRITE_ONCE(slave->target_last_arp_rx[slot], last_rx);

			slave_set_ns_maddr(bond, slave, target, &targets[slot]);

		}

		targets[slot] = *target;

include/net/bonding.h

+7 −6
Original line number Diff line number Diff line
@@ -521,13 +521,14 @@ static inline int bond_is_ip6_target_ok(struct in6_addr *addr) 
static inline unsigned long slave_oldest_target_arp_rx(struct bonding *bond,

						       struct slave *slave)

{

	unsigned long tmp, ret = READ_ONCE(slave->target_last_arp_rx[0]);

	int i = 1;

	unsigned long ret = slave->target_last_arp_rx[0];



	for (; (i < BOND_MAX_ARP_TARGETS) && bond->params.arp_targets[i]; i++)

		if (time_before(slave->target_last_arp_rx[i], ret))

			ret = slave->target_last_arp_rx[i];



	for (; (i < BOND_MAX_ARP_TARGETS) && bond->params.arp_targets[i]; i++) {

		tmp = READ_ONCE(slave->target_last_arp_rx[i]);

		if (time_before(tmp, ret))

			ret = tmp;

	}

	return ret;

}
@@ -537,7 +538,7 @@ static inline unsigned long slave_last_rx(struct bonding *bond, 
	if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)

		return slave_oldest_target_arp_rx(bond, slave);



	return slave->last_rx;

	return READ_ONCE(slave->last_rx);

}



static inline void slave_update_last_tx(struct slave *slave)