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: f5b2b966f032 ("[PATCH] bonding: Validate probe replies in ARP monitor")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Link: https://patch.msgid.link/20260122162914.2299312-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e7caf400a59cbd9680adea3d1b8ab7a22c78f7e6..a909ebcf1102d46beadc216a1b1a8a10e227ecb4 100644 (file)
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -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;

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 384499c869b8da9f036c43eb081095f1bf2141af..f1c6e9d8f61671e817bb417af39271dc14dd7bd9 100644 (file)
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -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;

diff --git a/include/net/bonding.h b/include/net/bonding.h
index 49edc7da05867f355f880329817bdc2a371f226b..46207840355709ee28d771c803b8ee6ef8920538 100644 (file)
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -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)