[PATCH net] net: atlantic: stop TX again before freeing rings

[PATCH net] net: atlantic: stop TX again before freeing rings

[Hanson Wang]

aq_nic_stop() disables TX and drops the carrier, then cancels the
service work, frees the link IRQ and disables NAPI. A link update or
TX completion can turn the carrier back on and wake queues after that
first netif_tx_disable(). ndo_set_features, ethtool ring changes and
XDP setup call aq_ndev_close() directly, without dev_deactivate(), so
a re-woken queue is still reached from ndo_start_xmit. aq_nic_deinit()
then frees buff_ring and leaves sw_tail unchanged.

On a UGREEN DXP2800 GT running 6.18.15 this oopses in aq_nic_map_skb()
from the IGMP timer softirq. The faulting store is dx_buff->flags = 0
at R10+0x28 with R10 == 0xc00: buff_ring is NULL and sw_tail is still
non-zero. Every module is unsigned, so the kernel is tainted E; the
atlantic driver itself is the in-tree one.

 BUG: kernel NULL pointer dereference, address: 0000000000000c28
 #PF: supervisor write access in kernel mode
 Oops: Oops: 0002 [#1] SMP NOPTI
 CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Tainted: G E 6.18.15
 Hardware name: UGREEN DXP2800 GT/DXP2800 GT, BIOS APS15021 04/14/2026
 RIP: 0010:aq_nic_map_skb+0x302/0x570 [atlantic]
 Code: 44 0f b6 78 06 <49> c7 42 28 00 00 00 00
 R10: 0000000000000c00
 CR2: 0000000000000c28
 Call Trace:
  <IRQ>
  aq_nic_xmit+0xc6/0x150 [atlantic]
  dev_hard_start_xmit+0x63/0x1c0
  sch_direct_xmit+0x154/0x3a0
  __dev_queue_xmit+0xb8c/0xe80
  ip_finish_output2+0x25f/0x580
  ip_output+0x65/0x100
  igmp_ifc_timer_expire+0x1d5/0x420
  ...

Stop and drain SKB transmit again after those wake sources are gone,
and drop the carrier in the same place, before hw_stop(). The rings
are freed by the caller only after aq_nic_stop() returns.
netif_tx_disable() holds each queue lock, so it waits until
sch_direct_xmit() has left ndo_start_xmit; the stopped bit then keeps
later SKB transmits out. hw_stop() only masks interrupts and does not
wake queues.

ndo_xdp_xmit is a separate path and is not covered.

The barrier below has not been booted on the failing NAS. The oops is
from the unpatched kernel. The diagnosis and this changelog were
prepared with an AI coding assistant from that oops and the driver
sources.

Fixes: 3aec6412e007 ("aquantia: Fix Tx queue hangups")
Cc: stable@vger.kernel.org
Signed-off-by: Hanson Wang <hanson.wang@ugreen.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 42e0fe9248f2..a6ceec0a22da 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -1448,6 +1448,12 @@ int aq_nic_stop(struct aq_nic_s *self)
 
 	aq_ptp_ring_stop(self);
 
+	/* Link updates and TX completion may have woken queues after the
+	 * first disable. Drain SKB transmit again before the rings are freed.
+	 */
+	netif_tx_disable(self->ndev);
+	netif_carrier_off(self->ndev);
+
 	return self->aq_hw_ops->hw_stop(self->aq_hw);
 }
 

base-commit: 21a7f44fd0765fcc5194130b28767786eba1d1e7

Re: [PATCH net] net: atlantic: stop TX again before freeing rings

[Hanson Wang]

Igor Russkikh is no longer at Marvell. Cc'ing the current atlantic maintainer.