[PATCH iwl-net v2 0/2] i40e: fix set_ringparam error path crashes

[PATCH iwl-net v2 0/2] i40e: fix set_ringparam error path crashes

[Aleksandr Loktionov]

i40e_set_ringparam() has two independent ways to crash a live queue
while resizing descriptors. Both need a failure injected into the
middle of the resize to hit, but both are 100% reliable once you know
where.

Patch 1 is the reported bug: the free_tx: label frees vsi->tx_rings[i]
(the live, NAPI-active ring) instead of &tx_rings[i] (the staged copy),
on Rx allocation failure. desc/tx_bi go to NULL under a running NAPI,
next poll dereferences them. Reproduced by forcing an Rx alloc failure
right after a Tx descriptor count change; panic address matched the
original report exactly (CR2 == old_count * sizeof(desc)).

Patch 2 was found reviewing patch 1. The staged tx_rings[] copies are
shallow copies of the live rings, so they carry over the same
ring->netdev/queue_index. Freeing one of these copies before
i40e_down() has run - either from this same free_tx: label or from the
i40e_setup_tx_descriptors() failure unwind loop above it - resets BQL
state for the live queue it was copied from. dql_completed() then
BUG_ONs the next time the live ring reports a completion that was
queued before the reset. This one predates patch 1's bug by four
years (Fixes: 9f65e15b4f98, 2013) and doesn't need patch 1 applied to
be reachable via the setup_tx_descriptors() path.

Reproduced by forcing i40e_setup_tx_descriptors() to fail on a later
queue while flooding real Tx traffic on the earlier ones:

  kernel BUG at lib/dynamic_queue_limits.c:99!
  RIP: dql_completed+0x285/0x2a0
  Call Trace:
   <IRQ>
   i40e_napi_poll+0x74b/0x1700 [i40e]
   __napi_poll+0x10a/0x200
   net_rx_action+0x2f7/0x380
   handle_softirqs+0xcc/0x270

Confirmed clean with the fix applied, same traffic, repeated
resize cycles.

Patch 2 touches both error paths above, but only the free_tx: hunk
needs patch 1 applied first to match its diff context - the
setup_tx_descriptors() unwind hunk applies, and matters, on its own.

Cc: Simon Horman <horms@kernel.org>

Aleksandr Loktionov (2):
  i40e: fix set_ringparam error path freeing live Tx rings
  i40e: avoid resetting BQL state when freeing temporary Tx rings in
    set_ringparam

 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.52.0

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

[Aleksandr Loktionov]

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: stable@vger.kernel.org
Cc: Simon Horman <horms@kernel.org>
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
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

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

[Nowlin, Alexander]

> -----Original Message-----
> From: Aleksandr Loktionov <aleksandr.loktionov@intel.com> 
> Sent: Friday, August 21, 2026 6:02 AM
> To: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: netdev@vger.kernel.org; Simon Horman <horms@kernel.org>
> Subject: [PATCH iwl-net v2 1/2] i40e: fix set_ringparam error path freeing live Tx rings
> 
> 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: stable@vger.kernel.org
> Cc: Simon Horman <horms@kernel.org>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> 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(-)

Tested-by: Alexander Nowlin <alexander.nowlin@intel.com>

[PATCH iwl-net v2 2/2] i40e: avoid resetting BQL state when freeing temporary Tx rings in set_ringparam

[Aleksandr Loktionov]

Temporary Tx rings built by i40e_set_ringparam() while resizing
descriptor counts are shallow struct copies of the live vsi->tx_rings[]
entries, so they retain the same ring->netdev and ring->queue_index as
the live ring they are staged to replace.

If an error path frees one of these clones with
i40e_free_tx_resources() before i40e_down() has run - either the
i40e_setup_tx_descriptors() failure unwind loop, or the free_tx: error
label - i40e_clean_tx_ring() will call netdev_tx_reset_queue() on
txring_txq(tx_ring), which resolves to the *same* netdev_queue as the
live ring because netdev/queue_index alias. That call resets the
queue's BQL/dql state (dql_reset(): num_queued = num_completed = 0)
while the live ring is still actively transmitting and completing
Tx on that same queue.

The next time the live ring reports completions for bytes that were
queued before the reset, dql_completed() can underflow its own
sanity check and hit:

	BUG_ON(count > num_queued - dql->num_completed);

i.e. a guaranteed kernel panic, not just a warning, given a queue with
outstanding (queued but not yet completed) Tx traffic at the moment
the clone is freed.

Reproduced on real hardware by forcing i40e_setup_tx_descriptors() to
fail partway through building tx_rings[] while flooding real Tx
traffic on the affected queues:

    kernel BUG at lib/dynamic_queue_limits.c:99!
    RIP: 0010:dql_completed+0x285/0x2a0
    Call Trace:
     <IRQ>
     i40e_napi_poll+0x74b/0x1700 [i40e]
     __napi_poll+0x10a/0x200
     net_rx_action+0x2f7/0x380
     handle_softirqs+0xcc/0x270

Confirmed the same trigger no longer panics with this fix applied,
including repeated resize/traffic cycles.

Clear the clone ring netdev before freeing it in both error paths, so
i40e_clean_tx_ring()'s existing `if (!tx_ring->netdev) return;` guard
skips the netdev_tx_reset_queue() call for the clone. This mirrors the
same idiom i40e already uses for XDP Tx rings, which also have no
netdev_queue of their own (ring->netdev = NULL in i40e_vsi_setup_tx()).
Only the temporary, about-to-be-freed clone is touched; the live ring
in vsi->tx_rings[] keeps its netdev.

Fixes: 9f65e15b4f98 ("i40e: Move rings from pointer to array to array of pointers")
Cc: stable@vger.kernel.org
Cc: Simon Horman <horms@kernel.org>
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
v2: New patch, split out of the set_ringparam NULL-deref fix (1/2)
    after review pointed out this related but distinct, older,
    pre-existing hazard on the same error path. Trimmed the inline
    comments to one line each.

 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 6d2b076..0e5b456 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -2143,6 +2143,8 @@ static int i40e_set_ringparam(struct net_device *netdev,
 					i--;
 					if (!i40e_active_tx_ring_index(vsi, i))
 						continue;
+					/* not live yet, skip its BQL reset on free */
+					tx_rings[i].netdev = NULL;
 					i40e_free_tx_resources(&tx_rings[i]);
 				}
 				kfree(tx_rings);
@@ -2248,8 +2250,11 @@ static int i40e_set_ringparam(struct net_device *netdev,
 	/* error cleanup if the Rx allocations failed after getting Tx */
 	if (tx_rings) {
 		for (i = 0; i < tx_alloc_queue_pairs; i++) {
-			if (i40e_active_tx_ring_index(vsi, i))
-				i40e_free_tx_resources(&tx_rings[i]);
+			if (i40e_active_tx_ring_index(vsi, i)) {
+				/* not live yet, skip its BQL reset on free */
+				tx_rings[i].netdev = NULL;
+				i40e_free_tx_resources(&tx_rings[i]);
+			}
 		}
 		kfree(tx_rings);
 		tx_rings = NULL;
-- 
2.52.0

RE: [PATCH iwl-net v2 2/2] i40e: avoid resetting BQL state when freeing temporary Tx rings in set_ringparam

[Nowlin, Alexander]

> -----Original Message-----
> From: Aleksandr Loktionov <aleksandr.loktionov@intel.com> 
> Sent: Friday, August 21, 2026 6:02 AM
> To: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: netdev@vger.kernel.org; Simon Horman <horms@kernel.org>
> Subject: [PATCH iwl-net v2 2/2] i40e: avoid resetting BQL state when freeing temporary Tx rings in set_ringparam
> 
> Temporary Tx rings built by i40e_set_ringparam() while resizing descriptor counts are shallow struct copies of the live vsi->tx_rings[] entries, so they retain the same ring->netdev and ring->queue_index as the live ring they are staged to replace.
> 
> If an error path frees one of these clones with
> i40e_free_tx_resources() before i40e_down() has run - either the
> i40e_setup_tx_descriptors() failure unwind loop, or the free_tx: error label - i40e_clean_tx_ring() will call netdev_tx_reset_queue() on txring_txq(tx_ring), which resolves to the *same* netdev_queue as the live > ring because netdev/queue_index alias. That call resets the queue's BQL/dql state (dql_reset(): num_queued = num_completed = 0) while the live ring is still actively transmitting and completing Tx on that same > queue.
> 
> The next time the live ring reports completions for bytes that were queued before the reset, dql_completed() can underflow its own sanity check and hit:
> 
> 	BUG_ON(count > num_queued - dql->num_completed);
> 
> i.e. a guaranteed kernel panic, not just a warning, given a queue with outstanding (queued but not yet completed) Tx traffic at the moment the clone is freed.
> 
> Reproduced on real hardware by forcing i40e_setup_tx_descriptors() to fail partway through building tx_rings[] while flooding real Tx traffic on the affected queues:
> 
>     kernel BUG at lib/dynamic_queue_limits.c:99!
>     RIP: 0010:dql_completed+0x285/0x2a0
>     Call Trace:
>      <IRQ>
>      i40e_napi_poll+0x74b/0x1700 [i40e]
>      __napi_poll+0x10a/0x200
>      net_rx_action+0x2f7/0x380
>      handle_softirqs+0xcc/0x270
> 
> Confirmed the same trigger no longer panics with this fix applied, including repeated resize/traffic cycles.
> 
> Clear the clone ring netdev before freeing it in both error paths, so i40e_clean_tx_ring()'s existing `if (!tx_ring->netdev) return;` guard skips the netdev_tx_reset_queue() call for the clone. This mirrors the same > idiom i40e already uses for XDP Tx rings, which also have no netdev_queue of their own (ring->netdev = NULL in i40e_vsi_setup_tx()).
> Only the temporary, about-to-be-freed clone is touched; the live ring in vsi->tx_rings[] keeps its netdev.
> 
> Fixes: 9f65e15b4f98 ("i40e: Move rings from pointer to array to array of pointers")
> Cc: stable@vger.kernel.org
> Cc: Simon Horman <horms@kernel.org>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> v2: New patch, split out of the set_ringparam NULL-deref fix (1/2)
>     after review pointed out this related but distinct, older,
>     pre-existing hazard on the same error path. Trimmed the inline
>     comments to one line each.

Tested-by: Alexander Nowlin <alexander.nowlin@intel.com>

Re: [PATCH iwl-net v2 0/2] i40e: fix set_ringparam error path crashes

[Simon Horman]

On Fri, Aug 21, 2026 at 03:01:55PM +0200, Aleksandr Loktionov wrote:
> i40e_set_ringparam() has two independent ways to crash a live queue
> while resizing descriptors. Both need a failure injected into the
> middle of the resize to hit, but both are 100% reliable once you know
> where.
> 
> Patch 1 is the reported bug: the free_tx: label frees vsi->tx_rings[i]
> (the live, NAPI-active ring) instead of &tx_rings[i] (the staged copy),
> on Rx allocation failure. desc/tx_bi go to NULL under a running NAPI,
> next poll dereferences them. Reproduced by forcing an Rx alloc failure
> right after a Tx descriptor count change; panic address matched the
> original report exactly (CR2 == old_count * sizeof(desc)).
> 
> Patch 2 was found reviewing patch 1. The staged tx_rings[] copies are
> shallow copies of the live rings, so they carry over the same
> ring->netdev/queue_index. Freeing one of these copies before
> i40e_down() has run - either from this same free_tx: label or from the
> i40e_setup_tx_descriptors() failure unwind loop above it - resets BQL
> state for the live queue it was copied from. dql_completed() then
> BUG_ONs the next time the live ring reports a completion that was
> queued before the reset. This one predates patch 1's bug by four
> years (Fixes: 9f65e15b4f98, 2013) and doesn't need patch 1 applied to
> be reachable via the setup_tx_descriptors() path.
> 
> Reproduced by forcing i40e_setup_tx_descriptors() to fail on a later
> queue while flooding real Tx traffic on the earlier ones:
> 
>   kernel BUG at lib/dynamic_queue_limits.c:99!
>   RIP: dql_completed+0x285/0x2a0
>   Call Trace:
>    <IRQ>
>    i40e_napi_poll+0x74b/0x1700 [i40e]
>    __napi_poll+0x10a/0x200
>    net_rx_action+0x2f7/0x380
>    handle_softirqs+0xcc/0x270
> 
> Confirmed clean with the fix applied, same traffic, repeated
> resize cycles.
> 
> Patch 2 touches both error paths above, but only the free_tx: hunk
> needs patch 1 applied first to match its diff context - the
> setup_tx_descriptors() unwind hunk applies, and matters, on its own.
> 
> Cc: Simon Horman <horms@kernel.org>
> 
> Aleksandr Loktionov (2):
>   i40e: fix set_ringparam error path freeing live Tx rings
>   i40e: avoid resetting BQL state when freeing temporary Tx rings in
>     set_ringparam

Thanks for the update.

For the series:
Reviewed-by: Simon Horman <horms@kernel.org>