[PATCH iwl-net v2 1/5] iavf: fix null pointer dereference in iavf_detect_recover_hung
flat view
COOLING12d
From: Aleksandr Loktionov <hidden>
Date: 2026-09-15 12:56:08
Also in:
intel-wired-lan
Subsystem:
intel ethernet drivers, networking drivers, the rest · Maintainers:
Tony Nguyen, Przemek Kitszel, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
Revision v2 of 2 in this series.
Revisions (2)
- v1 [diff vs current]
- v2 current
From: Kiran Patil <redacted>
iavf_watchdog_task() and iavf_reset_task() both run as work items on the
same ordered adapter->wq, so they can't race with each other. However,
iavf_set_ringparam() (and other ethtool ops) call iavf_reset_step()
directly from process context under the netdev instance lock, without
going through that workqueue at all. iavf_reset_step() can free and
reallocate adapter->tx_rings and the q_vectors array via
iavf_reinit_interrupt_scheme() while adapter->state still reads
__IAVF_RUNNING, so the watchdog task can concurrently call
iavf_detect_recover_hung() and dereference a NULL q_vector inside
iavf_force_wb(), or index into a NULL tx_rings array, causing a crash.
Guard against this by:
- returning early if vsi->back->tx_rings itself is NULL, since
num_active_queues can still be nonzero while the array is being
reallocated;
- skipping rings whose q_vector is NULL;
- reading tx_ring->q_vector once with READ_ONCE() into a local variable
and reusing that same value for both the NULL check and the
iavf_force_wb() call, instead of re-reading the field right before
use, which would leave a window for the concurrent reset to swap it
from underneath us in between.
Also move the tx_ring declaration into the loop body and drop the
redundant outer NULL initialisation, which the compiler can never
observe since an array-element address is always non-NULL.
Fixes: 07d44190a389 ("i40e/i40evf: Detect and recover hung queue scenario")
Cc: stable@vger.kernel.org
Signed-off-by: Kiran Patil <redacted>
Signed-off-by: Aleksandr Loktionov <redacted>
---
drivers/net/ethernet/intel/iavf/iavf_txrx.c | 50 ++++++++++++---------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
index c30abf1..f1c26a9 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c@@ -176,7 +176,6 @@ static void iavf_force_wb(struct iavf_vsi *vsi, struct iavf_q_vector *q_vector) **/ void iavf_detect_recover_hung(struct iavf_vsi *vsi) { - struct iavf_ring *tx_ring = NULL; struct net_device *netdev; unsigned int i; int packets;
@@ -194,29 +193,38 @@ void iavf_detect_recover_hung(struct iavf_vsi *vsi) if (!netif_carrier_ok(netdev)) return; + /* tx_rings can be freed/reallocated by a concurrent reset */ + if (!vsi->back->tx_rings) + return; + for (i = 0; i < vsi->back->num_active_queues; i++) { - tx_ring = &vsi->back->tx_rings[i]; - if (tx_ring && tx_ring->desc) { - /* If packet counter has not changed the queue is - * likely stalled, so force an interrupt for this - * queue. - * - * prev_pkt_ctr would be negative if there was no - * pending work. - */ - packets = tx_ring->stats.packets & INT_MAX; - if (tx_ring->prev_pkt_ctr == packets) { - iavf_force_wb(vsi, tx_ring->q_vector); - continue; - } + struct iavf_ring *tx_ring = &vsi->back->tx_rings[i]; + struct iavf_q_vector *q_vector; - /* Memory barrier between read of packet count and call - * to iavf_get_tx_pending() - */ - smp_rmb(); - tx_ring->prev_pkt_ctr = - iavf_get_tx_pending(tx_ring, true) ? packets : -1; + /* read once, q_vector can be reassigned by a concurrent reset */ + q_vector = READ_ONCE(tx_ring->q_vector); + if (!q_vector || !tx_ring->desc) + continue; + + /* If packet counter has not changed the queue is + * likely stalled, so force an interrupt for this + * queue. + * + * prev_pkt_ctr would be negative if there was no + * pending work. + */ + packets = tx_ring->stats.packets & INT_MAX; + if (tx_ring->prev_pkt_ctr == packets) { + iavf_force_wb(vsi, q_vector); + continue; } + + /* Memory barrier between read of packet count and call + * to iavf_get_tx_pending() + */ + smp_rmb(); + tx_ring->prev_pkt_ctr = + iavf_get_tx_pending(tx_ring, true) ? packets : -1; } }
--
2.52.0