netdev - [PATCH] drivers:atlx: Prevent integer overflow in statistics aggregation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20241007092936.53445-1-rand.sec96@gmail.com>
Date: Mon,  7 Oct 2024 12:29:36 +0300
From: Rand Deeb <rand.sec96@...il.com>
To: Chris Snook <chris.snook@...il.com>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Christian Marangi <ansuelsmth@...il.com>,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: deeb.rand@...fident.ru,
	lvc-project@...uxtesting.org,
	voskresenski.stanislav@...fident.ru,
	Rand Deeb <rand.sec96@...il.com>
Subject: [PATCH] drivers:atlx: Prevent integer overflow in statistics aggregation

The `atl1_inc_smb` function aggregates various RX and TX error counters
from the `stats_msg_block` structure. Currently, the arithmetic operations
are performed using `u32` types, which can lead to integer overflow when
summing large values. This overflow occurs before the result is cast to
a `u64`, potentially resulting in inaccurate network statistics.

To mitigate this risk, each operand in the summation is explicitly cast to
`u64` before performing the addition. This ensures that the arithmetic is
executed in 64-bit space, preventing overflow and maintaining accurate
statistics regardless of the system architecture.

Additionally, the aggregation of collision counters is also subject to
integer overflow. The operands in the summation for `collisions` are now
cast to `u64` to prevent overflow in this aggregation as well.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Rand Deeb <rand.sec96@...il.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c | 30 ++++++++++++------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index a9014d7932db..d61f46799713 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1656,17 +1656,17 @@ static void atl1_inc_smb(struct atl1_adapter *adapter)
 	struct net_device *netdev = adapter->netdev;
 	struct stats_msg_block *smb = adapter->smb.smb;
 
-	u64 new_rx_errors = smb->rx_frag +
-			    smb->rx_fcs_err +
-			    smb->rx_len_err +
-			    smb->rx_sz_ov +
-			    smb->rx_rxf_ov +
-			    smb->rx_rrd_ov +
-			    smb->rx_align_err;
-	u64 new_tx_errors = smb->tx_late_col +
-			    smb->tx_abort_col +
-			    smb->tx_underrun +
-			    smb->tx_trunc;
+	u64 new_rx_errors = (u64)smb->rx_frag +
+			    (u64)smb->rx_fcs_err +
+			    (u64)smb->rx_len_err +
+			    (u64)smb->rx_sz_ov +
+			    (u64)smb->rx_rxf_ov +
+			    (u64)smb->rx_rrd_ov +
+			    (u64)smb->rx_align_err;
+	u64 new_tx_errors = (u64)smb->tx_late_col +
+			    (u64)smb->tx_abort_col +
+			    (u64)smb->tx_underrun +
+			    (u64)smb->tx_trunc;
 
 	/* Fill out the OS statistics structure */
 	adapter->soft_stats.rx_packets += smb->rx_ok + new_rx_errors;
@@ -1674,10 +1674,10 @@ static void atl1_inc_smb(struct atl1_adapter *adapter)
 	adapter->soft_stats.rx_bytes += smb->rx_byte_cnt;
 	adapter->soft_stats.tx_bytes += smb->tx_byte_cnt;
 	adapter->soft_stats.multicast += smb->rx_mcast;
-	adapter->soft_stats.collisions += smb->tx_1_col +
-					  smb->tx_2_col +
-					  smb->tx_late_col +
-					  smb->tx_abort_col;
+	adapter->soft_stats.collisions += (u64)smb->tx_1_col +
+					  (u64)smb->tx_2_col +
+					  (u64)smb->tx_late_col +
+					  (u64)smb->tx_abort_col;
 
 	/* Rx Errors */
 	adapter->soft_stats.rx_errors += new_rx_errors;
-- 
2.34.1