[PATCH] net: atheros: atl1e: use atomic functions with memory barriers for next_to_clean

[PATCH] net: atheros: atl1e: use atomic functions with memory barriers for next_to_clean

[Gui-Dong Han]

next_to_clean synchronizes Tx ring cleanup in atl1e_clean_tx_irq()
with slot reuse in atl1e_tpd_avail().

atomic_set() and atomic_read() do not provide ordering. Without memory
barriers, out-of-order execution can lead to concurrency bugs on weak
memory architectures like ARM. For example, this can let the transmit
path reuse a slot before tx_buffer->skb = NULL is visible and trigger
the BUG_ON() in atl1e_tx_map(). This is a constructed scenario, and
there might be other undiscovered, potentially more harmful bugs caused
by this lack of ordering.

Use atomic_set_release() and atomic_read_acquire() for next_to_clean.

Fixes: a6a5325239c2 ("atl1e: Atheros L1E Gigabit Ethernet driver")
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
Found by auditing atomic operations used for synchronization.
A similar fix can be found in 6df8e84aa6b5.

Do not change atl1e_init_ring_ptrs(). Its atomic_set() runs during
bring-up before NAPI and interrupts are enabled, so it is not a runtime
publication point between Tx cleanup and Tx submission.

In my opinion, implementing ad-hoc lockless algorithms directly within
individual drivers is highly error-prone. To avoid these subtle memory
ordering and barrier bugs, drivers should rely on established, well-tested
kernel libraries like kfifo to handle this type of concurrency.
---
 drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
index 40290028580b..4ac8d4786820 100644
--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
@@ -1232,7 +1232,7 @@ static bool atl1e_clean_tx_irq(struct atl1e_adapter *adapter)
 	struct atl1e_tx_ring *tx_ring = &adapter->tx_ring;
 	struct atl1e_tx_buffer *tx_buffer = NULL;
 	u16 hw_next_to_clean = AT_READ_REGW(&adapter->hw, REG_TPD_CONS_IDX);
-	u16 next_to_clean = atomic_read(&tx_ring->next_to_clean);
+	u16 next_to_clean = atomic_read_acquire(&tx_ring->next_to_clean);
 
 	while (next_to_clean != hw_next_to_clean) {
 		tx_buffer = &tx_ring->tx_buffer[next_to_clean];
@@ -1259,7 +1259,7 @@ static bool atl1e_clean_tx_irq(struct atl1e_adapter *adapter)
 			next_to_clean = 0;
 	}
 
-	atomic_set(&tx_ring->next_to_clean, next_to_clean);
+	atomic_set_release(&tx_ring->next_to_clean, next_to_clean);
 
 	if (netif_queue_stopped(adapter->netdev) &&
 			netif_carrier_ok(adapter->netdev)) {
@@ -1562,7 +1562,7 @@ static inline u16 atl1e_tpd_avail(struct atl1e_adapter *adapter)
 	u16 next_to_use = 0;
 	u16 next_to_clean = 0;
 
-	next_to_clean = atomic_read(&tx_ring->next_to_clean);
+	next_to_clean = atomic_read_acquire(&tx_ring->next_to_clean);
 	next_to_use   = tx_ring->next_to_use;
 
 	return (u16)(next_to_clean > next_to_use) ?
-- 
2.43.0