[PATCH net v2] bonding: fix u32 overflow in compute_gap()

Hangbin Liu <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
From: Hangbin Liu <[email protected]>

The TLB fields tx_bytes, load_history, load, and unbalanced_load are
all u32, which can overflow when sustained throughput exceeds ~3.2
Gbit/s over the 10-second rebalance interval. On modern high-speed
NICs under heavy load this is easily reached, causing the gap
calculation in compute_gap() to wrap and produce incorrect slave
selection.

Widen these fields to s64 so the load arithmetic stays correct.
This also lets compute_gap() naturally return negative values when a
slave is oversubscribed, which the existing max-gap selection already
handles.

Additionally, the NIC speed is left-shifted before being cast to s64.
For speeds >= 4 Gbit/s (slave->speed >= 4096), the u32 shift
(4096 << 20 = 0x100000000) overflows before the cast takes effect.
Cast slave->speed to s64 before shifting so the arithmetic is
performed in 64 bits throughout.

Also cast SPEED_UNKNOWN to 0; otherwise, it would be the largest speed
after the shift.

Detected by AI code review.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Hangbin Liu <[email protected]>
---
Changes in v2:
- update comment description, including AI-detected info.
- fix tx_bytes/load type detected by sashiko
- cast SPEED_UNKNOWN to 0 before shift, detected by sashiko
- Link to v1: https://lore.kernel.org/r/[email protected]
---
 drivers/net/bonding/bond_alb.c | 12 ++++++++++--
 include/net/bond_alb.h         |  8 ++++----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 839f7482dc18..052ab6beb661 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -6,6 +6,7 @@
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
+#include <linux/ethtool.h>
 #include <linux/pkt_sched.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
@@ -160,8 +161,15 @@ static void tlb_deinitialize(struct bonding *bond)
 
 static long long compute_gap(struct slave *slave)
 {
-	return (s64) (slave->speed << 20) - /* Convert to Megabit per sec */
-	       (s64) (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
+	s64 speed;
+
+	if (slave->speed == SPEED_UNKNOWN)
+		speed = 0;
+	else
+		speed = (s64)slave->speed;
+
+	return (speed << 20) - /* Mbit/s -> bit/s */
+	       (SLAVE_TLB_INFO(slave).load << 3); /* Byte/s -> bit/s */
 }
 
 static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
index e5945427f38d..4933a855a438 100644
--- a/include/net/bond_alb.h
+++ b/include/net/bond_alb.h
@@ -57,12 +57,12 @@ struct tlb_client_info {
 				 * packets to a Client that the Hash function
 				 * gave this entry index.
 				 */
-	u32 tx_bytes;		/* Each Client accumulates the BytesTx that
+	s64 tx_bytes;		/* Each Client accumulates the BytesTx that
 				 * were transmitted to it, and after each
 				 * CallBack the LoadHistory is divided
 				 * by the balance interval
 				 */
-	u32 load_history;	/* This field contains the amount of Bytes
+	s64 load_history;	/* This field contains the amount of Bytes
 				 * that were transmitted to this client by
 				 * the server on the previous balance
 				 * interval in Bps.
@@ -118,14 +118,14 @@ struct tlb_slave_info {
 			 * are the entries that were assigned to use this
 			 * slave for transmit.
 			 */
-	u32 load;	/* Each slave sums the loadHistory of all clients
+	s64 load;	/* Each slave sums the loadHistory of all clients
 			 * assigned to it
 			 */
 };
 
 struct alb_bond_info {
 	struct tlb_client_info	*tx_hashtbl; /* Dynamically allocated */
-	u32			unbalanced_load;
+	s64			unbalanced_load;
 	atomic_t		tx_rebalance_counter;
 	int			lp_counter;
 	/* -------- rlb parameters -------- */

---
base-commit: 447c9303942c439a117d9b76ce6d6e2116b38ee7
change-id: 20260806-bond_overflow-ac6a6a78d6a0

Best regards,
-- 
Hangbin Liu <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.