[PATCH iwl-net v2 1/5] iavf: fix null pointer dereference in iavf_detect_recover_hung

[Aleksandr Loktionov <aleksandr.loktionov@intel.com>]

From: Kiran Patil <kiran.patil@intel.com>

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 <kiran.patil@intel.com>
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 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