[PATCH iwl-net v2 1/2] i40e: fix set_ringparam error path freeing live Tx rings

Aleksandr Loktionov <[email protected]>
Newsgroups org.osuosl.intel-wired-lan,org.kernel.vger.netdev
Message-ID <[email protected]>
The free_tx: error label in i40e_set_ringparam() is supposed to release
the temporary tx_rings[] array built before i40e_down() is called.
Instead it passes vsi->tx_rings[i] to i40e_free_tx_resources() - a
pointer to the live, NAPI-active ring - rather than &tx_rings[i].

i40e_free_tx_resources() unconditionally NULLs out ring->desc and
ring->tx_bi.  Because i40e_down() has not run at this point, NAPI is
still scheduled and the next i40e_clean_tx_irq() call hits a NULL
descriptor pointer:

    BUG: unable to handle page fault for address: 0000000000002000
    RIP: i40e_napi_poll (i40e_txrx.c:942 i40e_txrx.c:2769)
    RAX: 0000000000000000  RBX: ffff8d0a53ea9800

The trigger is ethtool -G with a TX descriptor count change (so
tx_rings[] is allocated) followed by i40e_alloc_rx_buffers() returning
failure, e.g. under memory pressure.

Pass &tx_rings[i] instead so the temporary rings' DMA descriptors and
software buffer arrays are freed, leaving the live VSI rings intact.

Reproduced on real hardware (XL710/XXV710) by forcing the Rx alloc
failure deterministically instead of relying on real memory pressure;
confirmed the box hangs without this fix and runs clean with it,
including repeated ring resizes and normal (non-error) resize paths.

Verified via git blame that commit 74608d17fe29 ("i40e: add support
for XDP_TX action") introduced the incorrect vsi->tx_rings[i] pointer
at this call site.

Fixes: 74608d17fe29 ("i40e: add support for XDP_TX action")
Cc: [email protected]
Cc: Simon Horman <[email protected]>
Signed-off-by: Aleksandr Loktionov <[email protected]>
---
v2: No functional change to the fix itself. During review, a related
    but distinct pre-existing hazard was pointed out: freeing a
    temporary tx_rings[] clone with i40e_free_tx_resources() also
    resets BQL/dql state for the live queue it was cloned from
    (i40e_clean_tx_ring() -> netdev_tx_reset_queue()), since the
    clone keeps the same ring->netdev/queue_index. That hazard
    predates this bug (Fixes: 9f65e15b4f98, 2013) and is also
    reachable from the i40e_setup_tx_descriptors() failure path
    that this patch does not touch, so it is fixed separately in
    patch 2/2 of this series rather than folded in here.

 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 3da9ec4..6d2b076 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -2249,7 +2249,7 @@ static int i40e_set_ringparam(struct net_device *netdev,
 	if (tx_rings) {
 		for (i = 0; i < tx_alloc_queue_pairs; i++) {
 			if (i40e_active_tx_ring_index(vsi, i))
-				i40e_free_tx_resources(vsi->tx_rings[i]);
+				i40e_free_tx_resources(&tx_rings[i]);
 		}
 		kfree(tx_rings);
 		tx_rings = NULL;
-- 
2.52.0
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.