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

Aleksandr Loktionov <[email protected]>
Newsgroups org.osuosl.intel-wired-lan,org.kernel.vger.netdev
Message-ID <[email protected]>
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 <[email protected]>

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
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.