[PATCH net v5 0/6] ice: fix stats array overflow via proper realloc

[PATCH net v5 0/6] ice: fix stats array overflow via proper realloc

[Przemek Kitszel]

Fix OOB access to the stats arrays.

The first commit (new in v5) fixes stats code against VSI_CHNL case (ADQ).

Next three commits are simple refactors to make the rest smaller,
the fifth one untangles the logic/lifetime of the stats array entries,
then we have the final fix for OOB access to the stats array (combined
for both PF and VF VSIs.

v5 handles now also the PF VSI stats resizes better, fixing devlink reload
issue reported in v4 by Clashiko.

v0
https://lore.kernel.org/netdev/20260520183501.3360810-3-anthony.l.nguyen@intel.com
v2
https://sashiko.dev/#/message/20260812204619.32253-3-przemyslaw.kitszel%40intel.com
v4 (a resend of ~the same v3)
https://lore.kernel.org/netdev/20260921182106.1015019-1-anthony.l.nguyen@intel.com

Most notable changes in v5 already noted in the cover letter, individual patches
have much more notes.
---


Przemek Kitszel (6):
  ice: skip stats handling for channel VSIs on rebuild
  ice: extract __ice_vsi_free_stats()
  ice: extract ice_vsi_new_stat_arrays()
  ice: extract ice_vsi_get_num_qs()
  ice: rebuild ring stats arrays instead of reallocating them in place
  ice: size ring stats arrays from the final queue count

 drivers/net/ethernet/intel/ice/ice.h     |   8 +-
 drivers/net/ethernet/intel/ice/ice_lib.c | 343 ++++++++++++++---------
 2 files changed, 210 insertions(+), 141 deletions(-)

-- 
2.51.1

[PATCH net v5 1/6] ice: skip stats handling for channel VSIs on rebuild

[Przemek Kitszel]

ice_vsi_alloc_stat_arrays() returns early for ICE_VSI_CHNL, so a channel
VSI never gets an entry in pf->vsi_stats[]. ice_vsi_realloc_stat_arrays()
dereferences that entry unconditionally, and ice_vsi_rebuild() calls it
before anything else, so the NULL is not filtered out anywhere.

The path is live. ice_prepare_for_reset() removes the queue channels only
for resets other than a PFR, so a PFR leaves the channel VSIs in place,
and ice_rebuild() then calls ice_rebuild_channels(), which rebuilds every
ICE_VSI_CHNL VSI it finds. A PF reset with ADQ (mqprio hardware offload)
configured thus dereferences NULL.

That combination is reset recovery on top of an active mqprio offload,
which is why it went unnoticed. It was found while refactoring this code,
not reported by a user.

I'm leaning towards removing our limited (compared to OOT ADQ) support,
but it's outside of this series. Bug could be triggered by:

  tc qdisc del dev "$IF" root 2>/dev/null || true
  tc qdisc add dev "$IF" root mqprio num_tc 2 \
    map 0 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 \
    queues 2@0 2@2 hw 1 mode channel
  sleep 3
  ethtool --reset "$IF" irq dma filter offload

Workqueue: ice ice_service_task [ice]
RIP: 0010:ice_vsi_rebuild+0x289/0x380 [ice]
 ice_rebuild_channels+0xd6/0x320 [ice]
 ice_rebuild+0x4f6/0x540 [ice]
 ice_do_reset+0x9f/0x1a0 [ice]
 ice_service_task+0x4a/0x460 [ice]

Fixes: 5995ef88e3a8 ("ice: realloc VSI stats arrays")
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
v5: new patch, split out of "ice: rebuild ring stats arrays instead of
    reallocating them in place" (Clashiko)
---
 drivers/net/ethernet/intel/ice/ice_lib.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 9e08db376d3d..31af378aa0e7 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3031,6 +3031,10 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
 	u16 prev_rxq = vsi->alloc_rxq;
 	int i;
 
+	/* channel VSIs have no entry in pf->vsi_stats[] */
+	if (vsi->type == ICE_VSI_CHNL)
+		return 0;
+
 	vsi_stat = pf->vsi_stats[vsi->idx];
 
 	if (req_txq < prev_txq) {
-- 
2.51.1

[PATCH net v5 2/6] ice: extract __ice_vsi_free_stats()

[Przemek Kitszel]

Record the length of each ring stats array in struct ice_vsi_stats, so
that freeing the array entries no longer needs the owning VSI. Keep the
new fields in sync in both places that size the arrays.

With that, the body of ice_vsi_free_stats() becomes independent of the
VSI and can be split out as __ice_vsi_free_stats(). The @free_entries
parameter is always true here; a later commit adds a caller that frees
only the array container, after its entries have been handed over to a
freshly allocated stats structure.

The free path now stops at the recorded length instead of the VSI queue
count, so it follows the real allocation rather than a number that can
disagree with it. Sizing still does not: "ice: rebuild ring stats arrays
instead of reallocating them in place" starts using the lengths to carry
entries over, and only "ice: size ring stats arrays from the final queue
count" makes them authoritative for the allocation path.

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
v5:
 - drop the "no functional change intended" claim, the bound of the free
   loop changes owner here (Clashiko)
 - state that the recorded lengths govern only the free path at this
   point in the series (Clashiko)
---
 drivers/net/ethernet/intel/ice/ice.h     |  2 +
 drivers/net/ethernet/intel/ice/ice_lib.c | 52 ++++++++++++++----------
 2 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index db3c7015c56c..fadfe94bf1c8 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -328,6 +328,8 @@ enum ice_vsi_state {
 struct ice_vsi_stats {
 	struct ice_ring_stats **tx_ring_stats;  /* Tx ring stats array */
 	struct ice_ring_stats **rx_ring_stats;  /* Rx ring stats array */
+	u16 tx_ring_stats_len;  /* Length of the Tx ring stats array */
+	u16 rx_ring_stats_len;  /* Length of the Rx ring stats array */
 };
 
 /* struct that defines a VSI, associated with a dev */
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 31af378aa0e7..1414127d32cd 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -330,42 +330,48 @@ static void ice_vsi_free_arrays(struct ice_vsi *vsi)
 	vsi->rxq_map = NULL;
 }
 
+/* free single stats memory */
+static void __ice_vsi_free_stats(struct ice_vsi_stats *vsi_stat, bool free_entries)
+{
+	if (!vsi_stat)
+		return;
+
+	if (free_entries) {
+		for (int i = 0; i < vsi_stat->tx_ring_stats_len; i++) {
+			if (vsi_stat->tx_ring_stats[i]) {
+				kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
+				WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
+			}
+		}
+		for (int i = 0; i < vsi_stat->rx_ring_stats_len; i++) {
+			if (vsi_stat->rx_ring_stats[i]) {
+				kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
+				WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
+			}
+		}
+	}
+
+	kfree(vsi_stat->tx_ring_stats);
+	kfree(vsi_stat->rx_ring_stats);
+	kfree(vsi_stat);
+}
+
 /**
  * ice_vsi_free_stats - Free the ring statistics structures
  * @vsi: VSI pointer
  */
 static void ice_vsi_free_stats(struct ice_vsi *vsi)
 {
 	struct ice_vsi_stats *vsi_stat;
 	struct ice_pf *pf = vsi->back;
-	int i;
 
 	if (vsi->type == ICE_VSI_CHNL)
 		return;
 	if (!pf->vsi_stats)
 		return;
 
 	vsi_stat = pf->vsi_stats[vsi->idx];
-	if (!vsi_stat)
-		return;
-
-	ice_for_each_alloc_txq(vsi, i) {
-		if (vsi_stat->tx_ring_stats[i]) {
-			kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
-			WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
-		}
-	}
-
-	ice_for_each_alloc_rxq(vsi, i) {
-		if (vsi_stat->rx_ring_stats[i]) {
-			kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
-			WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
-		}
-	}
-
-	kfree(vsi_stat->tx_ring_stats);
-	kfree(vsi_stat->rx_ring_stats);
-	kfree(vsi_stat);
+	__ice_vsi_free_stats(vsi_stat, true);
 	pf->vsi_stats[vsi->idx] = NULL;
 }
 
@@ -539,11 +545,13 @@ static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
 		kzalloc_objs(*vsi_stat->tx_ring_stats, vsi->alloc_txq);
 	if (!vsi_stat->tx_ring_stats)
 		goto err_alloc_tx;
+	vsi_stat->tx_ring_stats_len = vsi->alloc_txq;
 
 	vsi_stat->rx_ring_stats =
 		kzalloc_objs(*vsi_stat->rx_ring_stats, vsi->alloc_rxq);
 	if (!vsi_stat->rx_ring_stats)
 		goto err_alloc_rx;
+	vsi_stat->rx_ring_stats_len = vsi->alloc_rxq;
 
 	pf->vsi_stats[vsi->idx] = vsi_stat;
 
@@ -3055,6 +3063,7 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
 		vsi_stat->tx_ring_stats = tx_ring_stats;
 		return -ENOMEM;
 	}
+	vsi_stat->tx_ring_stats_len = req_txq;
 
 	if (req_rxq < prev_rxq) {
 		for (i = req_rxq; i < prev_rxq; i++) {
@@ -3074,6 +3083,7 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
 		vsi_stat->rx_ring_stats = rx_ring_stats;
 		return -ENOMEM;
 	}
+	vsi_stat->rx_ring_stats_len = req_rxq;
 
 	return 0;
 }
-- 
2.51.1

[PATCH net v5 3/6] ice: extract ice_vsi_new_stat_arrays()

[Przemek Kitszel]

Split the allocation of a struct ice_vsi_stats and its two ring stats
arrays out of ice_vsi_alloc_stat_arrays(), which is left with just the
lookup and the store into pf->vsi_stats[].

The sole caller keeps passing the very same queue counts as before,
vsi->alloc_txq and vsi->alloc_rxq. A later commit adds a second caller
in the rebuild path, which cannot use vsi->alloc_* because it has to
size the arrays before the VSI is reconfigured.

Note that the error path no longer clears pf->vsi_stats[vsi->idx]; the
early return above guarantees it is already NULL.

No functional change intended.

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_lib.c | 47 +++++++++++++-----------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 1414127d32cd..ed32ae2667c5 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -519,6 +519,30 @@ static irqreturn_t ice_msix_clean_rings(int __always_unused irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static struct ice_vsi_stats *ice_vsi_new_stat_arrays(int txq, int rxq)
+{
+	struct ice_ring_stats **tx_ring_stats;
+	struct ice_ring_stats **rx_ring_stats;
+	struct ice_vsi_stats *vsi_stat;
+
+	vsi_stat = kzalloc_obj(*vsi_stat);
+	tx_ring_stats = kzalloc_objs(*tx_ring_stats, txq);
+	rx_ring_stats = kzalloc_objs(*rx_ring_stats, rxq);
+	if (!vsi_stat || !tx_ring_stats || !rx_ring_stats) {
+		kfree(vsi_stat);
+		kfree(tx_ring_stats);
+		kfree(rx_ring_stats);
+		return NULL;
+	}
+
+	vsi_stat->tx_ring_stats = tx_ring_stats;
+	vsi_stat->rx_ring_stats = rx_ring_stats;
+	vsi_stat->tx_ring_stats_len = txq;
+	vsi_stat->rx_ring_stats_len = rxq;
+
+	return vsi_stat;
+}
+
 /**
  * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
  * @vsi: VSI pointer
@@ -537,33 +561,12 @@ static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
 	/* realloc will happen in rebuild path */
 		return 0;
 
-	vsi_stat = kzalloc_obj(*vsi_stat);
+	vsi_stat = ice_vsi_new_stat_arrays(vsi->alloc_txq, vsi->alloc_rxq);
 	if (!vsi_stat)
 		return -ENOMEM;
 
-	vsi_stat->tx_ring_stats =
-		kzalloc_objs(*vsi_stat->tx_ring_stats, vsi->alloc_txq);
-	if (!vsi_stat->tx_ring_stats)
-		goto err_alloc_tx;
-	vsi_stat->tx_ring_stats_len = vsi->alloc_txq;
-
-	vsi_stat->rx_ring_stats =
-		kzalloc_objs(*vsi_stat->rx_ring_stats, vsi->alloc_rxq);
-	if (!vsi_stat->rx_ring_stats)
-		goto err_alloc_rx;
-	vsi_stat->rx_ring_stats_len = vsi->alloc_rxq;
-
 	pf->vsi_stats[vsi->idx] = vsi_stat;
-
 	return 0;
-
-err_alloc_rx:
-	kfree(vsi_stat->rx_ring_stats);
-err_alloc_tx:
-	kfree(vsi_stat->tx_ring_stats);
-	kfree(vsi_stat);
-	pf->vsi_stats[vsi->idx] = NULL;
-	return -ENOMEM;
 }
 
 /**
-- 
2.51.1

[PATCH net v5 4/6] ice: extract ice_vsi_get_num_qs()

[Przemek Kitszel]

ice_vsi_set_num_qs() mixed two things: deciding how many queues a VSI
gets, and applying all the side effects of that decision. Split the
decision out into ice_vsi_get_num_qs(), returning both counts at once.

To make that possible, group alloc_txq and alloc_rxq in struct ice_vsi
with struct_group_tagged(), which gives the return type without adding
any new storage. Field order changes slightly, num_txq now follows
alloc_rxq, but nothing depends on it.

ice_vsi_get_num_qs() takes @held_txq and @held_rxq from the start, even
though the only caller so far passes zero for both. They let a caller
ask what the counts would be once the queues the VSI currently owns are
back in the PF pool. A later commit adds the caller that needs this: the
rebuild path has to size its ring stats arrays before ice_vsi_decfg()
releases the queues, yet the arrays must match what ice_vsi_set_num_qs()
computes afterwards.

While reshaping the whole body, drop the kernel-doc line promising a
return value; ice_vsi_set_num_qs() returns void.

No functional change intended.

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
v5:
 - drop the stale "Return 0 on success and a negative value on error"
   from the kernel-doc of the void ice_vsi_set_num_qs() (Clashiko)
---
 drivers/net/ethernet/intel/ice/ice.h     |  6 +-
 drivers/net/ethernet/intel/ice/ice_lib.c | 88 ++++++++++++++----------
 2 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index fadfe94bf1c8..f1ba86d066be 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -403,9 +403,11 @@ struct ice_vsi {
 	u8 rx_mapping_mode;		 /* ICE_MAP_MODE_[CONTIG|SCATTER] */
 	u16 *txq_map;			 /* index in pf->avail_txqs */
 	u16 *rxq_map;			 /* index in pf->avail_rxqs */
-	u16 alloc_txq;			 /* Allocated Tx queues */
+	struct_group_tagged(ice_vsi_alloc_queues_params, alloc_txq_rxq,
+		u16 alloc_txq;		 /* Allocated Tx queues */
+		u16 alloc_rxq;		 /* Allocated Rx queues */
+	);
 	u16 num_txq;			 /* Used Tx queues */
-	u16 alloc_rxq;			 /* Allocated Rx queues */
 	u16 num_rxq;			 /* Used Rx queues */
 	u16 req_txq;			 /* User requested Tx queues */
 	u16 req_rxq;			 /* User requested Rx queues */
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index ed32ae2667c5..134984077d5d 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -153,23 +153,61 @@ static void ice_vsi_set_num_desc(struct ice_vsi *vsi)
 	}
 }
 
-static u16 ice_get_rxq_count(struct ice_pf *pf)
+static u16 ice_get_rxq_count(struct ice_pf *pf, u16 held)
 {
-	return min(ice_get_avail_rxq_count(pf),
-		   netif_get_num_default_rss_queues());
+	return min_t(u16, ice_get_avail_rxq_count(pf) + held,
+		     netif_get_num_default_rss_queues());
 }
 
-static u16 ice_get_txq_count(struct ice_pf *pf)
+static u16 ice_get_txq_count(struct ice_pf *pf, u16 held)
 {
-	return min(ice_get_avail_txq_count(pf),
-		   netif_get_num_default_rss_queues());
+	return min_t(u16, ice_get_avail_txq_count(pf) + held,
+		     netif_get_num_default_rss_queues());
+}
+
+/* @held_txq, @held_rxq: queues the VSI still owns but is about to return to the
+ * PF pool, so that the result matches what it will be once they are back there.
+ */
+static struct ice_vsi_alloc_queues_params
+ice_vsi_get_num_qs(struct ice_vsi *vsi, u16 held_txq, u16 held_rxq)
+{
+	struct ice_vsi_alloc_queues_params qs = {};
+	struct ice_pf *pf = vsi->back;
+
+	switch (vsi->type) {
+	case ICE_VSI_PF:
+		qs.alloc_txq = vsi->req_txq ?: ice_get_txq_count(pf, held_txq);
+
+		/* only 1 Rx queue unless RSS is enabled */
+		if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags))
+			qs.alloc_rxq = 1;
+		else
+			qs.alloc_rxq = vsi->req_rxq ?:
+				       ice_get_rxq_count(pf, held_rxq);
+		break;
+	case ICE_VSI_SF:
+	case ICE_VSI_CTRL:
+	case ICE_VSI_LB:
+		qs.alloc_txq = 1;
+		qs.alloc_rxq = 1;
+		break;
+	case ICE_VSI_VF:
+		qs.alloc_txq = vsi->vf->num_req_qs ?: vsi->vf->num_vf_qs;
+		qs.alloc_rxq = qs.alloc_txq;
+		break;
+	case ICE_VSI_CHNL:
+		break;
+	default:
+		dev_warn(ice_pf_to_dev(pf), "Unknown VSI type %d\n", vsi->type);
+		return vsi->alloc_txq_rxq;
+	}
+
+	return qs;
 }
 
 /**
  * ice_vsi_set_num_qs - Set number of queues, descriptors and vectors for a VSI
  * @vsi: the VSI being configured
- *
- * Return 0 on success and a negative value on error
  */
 static void ice_vsi_set_num_qs(struct ice_vsi *vsi)
 {
@@ -180,68 +218,44 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi)
 	if (WARN_ON(vsi_type == ICE_VSI_VF && !vf))
 		return;
 
+	vsi->alloc_txq_rxq = ice_vsi_get_num_qs(vsi, 0, 0);
+
 	switch (vsi_type) {
 	case ICE_VSI_PF:
-		if (vsi->req_txq) {
-			vsi->alloc_txq = vsi->req_txq;
+		if (vsi->req_txq)
 			vsi->num_txq = vsi->req_txq;
-		} else {
-			vsi->alloc_txq = ice_get_txq_count(pf);
-		}
+		if (vsi->req_rxq && test_bit(ICE_FLAG_RSS_ENA, pf->flags))
+			vsi->num_rxq = vsi->req_rxq;
 
 		pf->num_lan_tx = vsi->alloc_txq;
-
-		/* only 1 Rx queue unless RSS is enabled */
-		if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) {
-			vsi->alloc_rxq = 1;
-		} else {
-			if (vsi->req_rxq) {
-				vsi->alloc_rxq = vsi->req_rxq;
-				vsi->num_rxq = vsi->req_rxq;
-			} else {
-				vsi->alloc_rxq = ice_get_rxq_count(pf);
-			}
-		}
-
 		pf->num_lan_rx = vsi->alloc_rxq;
 
 		vsi->num_q_vectors = max(vsi->alloc_rxq, vsi->alloc_txq);
 		break;
 	case ICE_VSI_SF:
-		vsi->alloc_txq = 1;
-		vsi->alloc_rxq = 1;
 		vsi->num_q_vectors = 1;
 		vsi->irq_dyn_alloc = true;
 		break;
 	case ICE_VSI_VF:
 		if (vf->num_req_qs)
 			vf->num_vf_qs = vf->num_req_qs;
-		vsi->alloc_txq = vf->num_vf_qs;
-		vsi->alloc_rxq = vf->num_vf_qs;
 		/* pf->vfs.num_msix_per includes (VF miscellaneous vector +
 		 * data queue interrupts). Since vsi->num_q_vectors is number
 		 * of queues vectors, subtract 1 (ICE_NONQ_VECS_VF) from the
 		 * original vector count
 		 */
 		vsi->num_q_vectors = vf->num_msix - ICE_NONQ_VECS_VF;
 		break;
 	case ICE_VSI_CTRL:
-		vsi->alloc_txq = 1;
-		vsi->alloc_rxq = 1;
 		vsi->num_q_vectors = 1;
 		break;
 	case ICE_VSI_CHNL:
-		vsi->alloc_txq = 0;
-		vsi->alloc_rxq = 0;
 		break;
 	case ICE_VSI_LB:
-		vsi->alloc_txq = 1;
-		vsi->alloc_rxq = 1;
 		/* A dummy q_vector, no actual IRQ. */
 		vsi->num_q_vectors = 1;
 		break;
 	default:
-		dev_warn(ice_pf_to_dev(pf), "Unknown VSI type %d\n", vsi_type);
 		break;
 	}
 
-- 
2.51.1

[PATCH net v5 5/6] ice: rebuild ring stats arrays instead of reallocating them in place

[Przemek Kitszel]

ice_vsi_realloc_stat_arrays() resized the ring stats arrays in place
with krealloc_array(), sizing them from vsi->req_txq/req_rxq. That is
not what ice_vsi_set_num_qs() computes later in ice_vsi_cfg_def(), so
after a rebuild the arrays could end up shorter than vsi->alloc_txq /
vsi->alloc_rxq, and ice_vsi_alloc_ring_stats() then walked past their
end.

The VF case shows the divergence. ice_sriov_set_msix_vec_count() writes
vsi->req_txq and then calls ice_vsi_rebuild(), so the old resizer sized
from req_txq, while ice_vsi_get_num_qs() reads vf->num_req_qs ?:
vf->num_vf_qs and ignores req_txq entirely. Once a guest has raised
vf->num_req_qs through VIRTCHNL_OP_REQUEST_QUEUES the two disagree and
the arrays come out shorter than the alloc_txq/alloc_rxq the rebuild
installs. The reset that such a request triggers is a different path,
ice_vf_reconfig_vsi(), which does not use this resizer at all; that one
is handled by the next patch.

Replace it with ice_vsi_resize_stat_arrays(), which allocates a fresh
struct ice_vsi_stats instead, sized with ice_vsi_get_num_qs() and the
queues ice_vsi_decfg() is about to return to the PF pool, which is
exactly what ice_vsi_set_num_qs() will compute once they are back
there, barring a concurrent change of the PF pool; the next patch stops
relying on that. Copy the surviving entry pointers over and install
the new structure, all before ice_vsi_decfg() runs. The entries that
did not fit are freed by ice_vsi_free_unused_stat_arrays(); the old
container itself then goes through __ice_vsi_free_stats() with
@free_entries set to false, since the entries it still points at now
belong to the new structure.

Allocating up front also means a failure no longer leaves a half-updated
ice_vsi_stats behind, with the Tx array already swapped and its surplus
Tx entries already freed, but the Rx allocation failed.

Put the allocate-install-free-surplus sequence in
ice_vsi_install_stat_arrays(), next to ice_vsi_new_stat_arrays(), so
that only the sizing decision is left in the resizer.

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
this removes some tech debt, but it is not independently backportable,
as it sits on three preceding refactors; the NULL dereference and the
sizing mismatch are fixed by their own patches, which carry Fixes: tags

v5:
 - drop the false "requesting fewer queues than the PF pool can hand
   out" reproducer, use the VF one that the code can exhibit (Clashiko)
 - describe the real failure-path benefit, the old code already returned
   before ice_vsi_decfg() with the VSI fully configured (Clashiko)
 - split the ICE_VSI_CHNL guard into its own patch with a Fixes: tag
   (Clashiko)
 - factor out ice_vsi_install_stat_arrays(), reused by the new sizing
   patch (Clashiko)
---
 drivers/net/ethernet/intel/ice/ice_lib.c | 144 ++++++++++++++---------
 1 file changed, 88 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 134984077d5d..448d3c7780ad 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -557,6 +557,80 @@ static struct ice_vsi_stats *ice_vsi_new_stat_arrays(int txq, int rxq)
 	return vsi_stat;
 }
 
+static void ice_vsi_free_unused_stat_arrays(struct ice_vsi_stats *vsi_stat,
+					    struct ice_vsi_stats *new_vsi_stat)
+{
+	int new_txq = new_vsi_stat->tx_ring_stats_len;
+	int new_rxq = new_vsi_stat->rx_ring_stats_len;
+	int prev_txq = vsi_stat->tx_ring_stats_len;
+	int prev_rxq = vsi_stat->rx_ring_stats_len;
+
+	for (int i = new_txq; i < prev_txq; i++) {
+		if (vsi_stat->tx_ring_stats[i]) {
+			kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
+			WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
+		}
+	}
+	for (int i = new_rxq; i < prev_rxq; i++) {
+		if (vsi_stat->rx_ring_stats[i]) {
+			kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
+			WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
+		}
+	}
+}
+
+static void ice_vsi_set_stat_arrays(struct ice_vsi *vsi,
+				    struct ice_vsi_stats *new_vsi_stat)
+{
+	u16 new_txq, new_rxq, prev_txq, prev_rxq;
+	struct ice_vsi_stats *vsi_stat;
+	struct ice_pf *pf = vsi->back;
+
+	new_txq = new_vsi_stat->tx_ring_stats_len;
+	new_rxq = new_vsi_stat->rx_ring_stats_len;
+	vsi_stat = pf->vsi_stats[vsi->idx];
+	pf->vsi_stats[vsi->idx] = new_vsi_stat;
+	if (!vsi_stat)
+		return; /* don't copy if there is no source */
+
+	prev_txq = vsi_stat->tx_ring_stats_len;
+	prev_rxq = vsi_stat->rx_ring_stats_len;
+
+	memcpy(new_vsi_stat->tx_ring_stats, vsi_stat->tx_ring_stats,
+	       sizeof(*vsi_stat->tx_ring_stats) * min(prev_txq, new_txq));
+	memcpy(new_vsi_stat->rx_ring_stats, vsi_stat->rx_ring_stats,
+	       sizeof(*vsi_stat->rx_ring_stats) * min(prev_rxq, new_rxq));
+}
+
+/**
+ * ice_vsi_install_stat_arrays - swap in freshly sized ring stats arrays
+ * @vsi: VSI to install the ring stats arrays of
+ * @txq: number of Tx ring stats entries to make room for
+ * @rxq: number of Rx ring stats entries to make room for
+ *
+ * Surviving entries are carried over, the rest is freed together with the old
+ * container.
+ *
+ * Return: 0 on success and negative value on failure.
+ */
+static int ice_vsi_install_stat_arrays(struct ice_vsi *vsi, u16 txq, u16 rxq)
+{
+	struct ice_vsi_stats *old_stat, *new_stat;
+
+	new_stat = ice_vsi_new_stat_arrays(txq, rxq);
+	if (!new_stat)
+		return -ENOMEM;
+
+	old_stat = vsi->back->vsi_stats[vsi->idx];
+	ice_vsi_set_stat_arrays(vsi, new_stat);
+	if (old_stat) {
+		ice_vsi_free_unused_stat_arrays(old_stat, new_stat);
+		__ice_vsi_free_stats(old_stat, false);
+	}
+
+	return 0;
+}
+
 /**
  * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
  * @vsi: VSI pointer
@@ -3040,69 +3114,27 @@ ice_vsi_rebuild_set_coalesce(struct ice_vsi *vsi,
 }
 
 /**
- * ice_vsi_realloc_stat_arrays - Frees unused stat structures or alloc new ones
- * @vsi: VSI pointer
+ * ice_vsi_resize_stat_arrays - resize ring stats arrays for new queue count
+ * @vsi: VSI to swap the ring stats arrays of
+ *
+ * Call while @vsi still owns its queues and before ice_vsi_decfg() returns them
+ * to the PF pool, so that the new size is what ice_vsi_set_num_qs() will compute
+ * afterwards. Surviving entries are carried over, the rest is freed.
+ *
+ * Return: 0 on success and negative value on failure.
  */
-static int
-ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
+static int ice_vsi_resize_stat_arrays(struct ice_vsi *vsi)
 {
-	u16 req_txq = vsi->req_txq ? vsi->req_txq : vsi->alloc_txq;
-	u16 req_rxq = vsi->req_rxq ? vsi->req_rxq : vsi->alloc_rxq;
-	struct ice_ring_stats **tx_ring_stats;
-	struct ice_ring_stats **rx_ring_stats;
-	struct ice_vsi_stats *vsi_stat;
-	struct ice_pf *pf = vsi->back;
-	u16 prev_txq = vsi->alloc_txq;
-	u16 prev_rxq = vsi->alloc_rxq;
-	int i;
+	struct ice_vsi_alloc_queues_params qs;
 
 	/* channel VSIs have no entry in pf->vsi_stats[] */
 	if (vsi->type == ICE_VSI_CHNL)
 		return 0;
 
-	vsi_stat = pf->vsi_stats[vsi->idx];
-
-	if (req_txq < prev_txq) {
-		for (i = req_txq; i < prev_txq; i++) {
-			if (vsi_stat->tx_ring_stats[i]) {
-				kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
-				WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
-			}
-		}
-	}
-
-	tx_ring_stats = vsi_stat->tx_ring_stats;
-	vsi_stat->tx_ring_stats =
-		krealloc_array(vsi_stat->tx_ring_stats, req_txq,
-			       sizeof(*vsi_stat->tx_ring_stats),
-			       GFP_KERNEL | __GFP_ZERO);
-	if (!vsi_stat->tx_ring_stats) {
-		vsi_stat->tx_ring_stats = tx_ring_stats;
-		return -ENOMEM;
-	}
-	vsi_stat->tx_ring_stats_len = req_txq;
+	qs = ice_vsi_get_num_qs(vsi, vsi->alloc_txq + vsi->num_xdp_txq,
+				vsi->alloc_rxq);
 
-	if (req_rxq < prev_rxq) {
-		for (i = req_rxq; i < prev_rxq; i++) {
-			if (vsi_stat->rx_ring_stats[i]) {
-				kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
-				WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
-			}
-		}
-	}
-
-	rx_ring_stats = vsi_stat->rx_ring_stats;
-	vsi_stat->rx_ring_stats =
-		krealloc_array(vsi_stat->rx_ring_stats, req_rxq,
-			       sizeof(*vsi_stat->rx_ring_stats),
-			       GFP_KERNEL | __GFP_ZERO);
-	if (!vsi_stat->rx_ring_stats) {
-		vsi_stat->rx_ring_stats = rx_ring_stats;
-		return -ENOMEM;
-	}
-	vsi_stat->rx_ring_stats_len = req_rxq;
-
-	return 0;
+	return ice_vsi_install_stat_arrays(vsi, qs.alloc_txq, qs.alloc_rxq);
 }
 
 /**
@@ -3132,7 +3164,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
 
 	mutex_lock(&vsi->xdp_state_lock);
 
-	ret = ice_vsi_realloc_stat_arrays(vsi);
+	ret = ice_vsi_resize_stat_arrays(vsi);
 	if (ret)
 		goto unlock;
 
-- 
2.51.1

[PATCH net v5 6/6] ice: size ring stats arrays from the final queue count

[Przemek Kitszel]

The ring stats arrays were sized in one place and indexed in another,
with nothing reconciling the two. ice_vsi_alloc_stat_arrays() gave up as
soon as a container existed:

	if (pf->vsi_stats[vsi->idx])
	/* realloc will happen in rebuild path */
		return 0;

while ice_vsi_alloc_ring_stats() walks the arrays with
ice_for_each_alloc_txq() / ice_for_each_alloc_rxq(), that is, up to
vsi->alloc_txq / vsi->alloc_rxq. Whenever the arrays are shorter than
the count ice_vsi_set_num_qs() ends up computing, that loop reads and
stores past the end of the kmalloc'ed pointer arrays.

Deferring to "the rebuild path" does not hold. It cannot get the size
right, and some paths never call it at all.

ice_devlink_reinit_down() is one of the latter. It calls ice_vsi_decfg()
directly, which does not free pf->vsi_stats[], and nothing on the way
down releases the main VSI's entry either: ice_dealloc_vsis() runs only
from ice_init()/ice_deinit(), and the single entry that ice_unload()
does drop is the control VSI's, via ice_deinit_fdir(). The main VSI's
arrays therefore survive the reload, while ice_devlink_reinit_up() runs
ice_vsi_cfg() and recomputes the count from
netif_get_num_default_rss_queues(), which follows cpu_online_mask.
Sizing the arrays with most CPUs offline and then onlining them is
enough, no VF involved:

  for c in /sys/devices/system/cpu/cpu[0-9]*/online; do echo 0 > $c; done
  echo 0000:18:00.0 > /sys/bus/pci/drivers/ice/unbind
  echo 0000:18:00.0 > /sys/bus/pci/drivers/ice/bind
  for c in /sys/devices/system/cpu/cpu[0-9]*/online; do echo 1 > $c; done
  devlink dev reload pci/0000:18:00.0 action driver_reinit

The unbind/bind is what makes the arrays actually be allocated at the
small size; a reload alone only reuses them.

The read then runs off the end of tx_ring_stats[] and picks up whatever
follows it in kmalloc-32. Here that was a string, 0x74756f5f6c697475
spelling "util_out" in memory order. It is stored into ring->ring_stats
and faults on the first dereference, in ice_setup_tx_ring(), call trace:

 RIP: 0010:ice_setup_tx_ring+0x8d/0xe0 [ice]
  ice_vsi_setup_tx_rings+0x2a/0x80 [ice]
  ice_vsi_open+0x28/0x170 [ice]
  ice_open_internal+0xc9/0x160 [ice]
  __dev_open+0x138/0x2b0
  __dev_change_flags+0x1d5/0x250
  netif_change_flags+0x21/0x60
  do_setlink.constprop.0+0x336/0xcf0
  rtnl_newlink+0x4a4/0x9b0

Even where the rebuild path does run, it sizes the arrays before
ice_vsi_decfg() returns the queues to the PF pool, and for an
ICE_VSI_PF VSI with no explicit request it derives the count from that
shared pool. Nothing serializes it against the recount in
ice_vsi_set_num_qs(), so a concurrent release, say
"echo 0 > sriov_numvfs", leaves the final count larger than the arrays
that were already installed.

ice_vsi_alloc_stat_arrays() runs from ice_vsi_cfg_def(), right after
ice_vsi_alloc_def() called ice_vsi_set_num_qs(), so it is the first
place where the queue count is final. Make it the only authority: keep
the existing arrays when they are already long enough, and otherwise
grow them with ice_vsi_install_stat_arrays(), which carries the
surviving entries over. The check only forces a grow: while both arrays
are long enough they are kept, an oversized one being merely wasteful.
Once either is too short the whole container is replaced, and then the
other one follows the final count too, shrinking if that is what it
takes.

Michal Schmidt hit the same corruption through the VF path, where a
guest raises its queue count with VIRTCHNL_OP_REQUEST_QUEUES and the
following VF reset reconfigures the VSI with the larger count while the
old, shorter arrays are still installed:

 BUG: KASAN: slab-out-of-bounds in ice_vsi_alloc_ring_stats+0x385/0x4a0 [ice]
 Workqueue: ice ice_service_task [ice]
 Call Trace:
  kasan_report+0xd7/0x120
  ice_vsi_alloc_ring_stats+0x385/0x4a0 [ice]
  ice_vsi_cfg_def+0x12e2/0x2060 [ice]
  ice_vsi_cfg+0xb5/0x3c0 [ice]
  ice_reset_vf+0x858/0xf80 [ice]
  ice_vc_request_qs_msg+0x1da/0x290 [ice]
  ice_vc_process_vf_msg+0xb15/0x1430 [ice]
  __ice_clean_ctrlq+0x70d/0x9d0 [ice]
  ice_service_task+0x840/0xf20 [ice]
  process_one_work+0x690/0xff0
  worker_thread+0x4d9/0xd20
  kthread+0x322/0x410
  ret_from_fork+0x332/0x660
  ret_from_fork_asm+0x1a/0x30

 Allocated by task 2439:
  kasan_save_stack+0x1c/0x40
  kasan_save_track+0x10/0x30
  __kasan_kmalloc+0x96/0xb0
  __kmalloc_noprof+0x1d8/0x580
  ice_vsi_cfg_def+0x115c/0x2060 [ice]
  ice_vsi_cfg+0xb5/0x3c0 [ice]
  ice_vsi_setup+0x180/0x320 [ice]
  ice_start_vfs+0x1f3/0x590 [ice]
  ice_ena_vfs+0x66d/0x798 [ice]
  ice_sriov_configure.cold+0xe4/0x121 [ice]
  sriov_numvfs_store+0x279/0x480
  kernfs_fop_write_iter+0x331/0x4f0
  vfs_write+0x4c4/0xe40
  ksys_write+0x10c/0x240
  do_syscall_64+0xd9/0x650
  entry_SYSCALL_64_after_hwframe+0x76/0x7e

 The buggy address belongs to the object at ffff88810affea40
                which belongs to the cache kmalloc-32 of size 32
 The buggy address is located 0 bytes to the right of
                allocated 32-byte region [ffff88810affea40, ffff88810affea60)

Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset")
Reported-by: Michal Schmidt <mschmidt@redhat.com>
Closes: https://redhat.atlassian.net/browse/RHEL-164321
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
v5:
 - new patch, replaces the VF-only fix with one that covers every path
   reaching ice_vsi_alloc_ring_stats() (Clashiko)
 - the reproducers both postdate the cited tag: devlink reinit arrived
   with 31c8db2c4fa7 ("ice: implement devlink reinit action"), the VF
   path with 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with
   ice_vf_reconfig_vsi()"). The sizing itself went wrong in the cited
   commit, hence the older tag (Clashiko)
 - drop the follow-up VF resize patch, it could abort a VFLR after the
   hardware had already reset the VF (Clashiko)
---
 drivers/net/ethernet/intel/ice/ice_lib.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 448d3c7780ad..c393d913d037 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -634,27 +634,29 @@ static int ice_vsi_install_stat_arrays(struct ice_vsi *vsi, u16 txq, u16 rxq)
 /**
  * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
  * @vsi: VSI pointer
+ *
+ * Runs after ice_vsi_set_num_qs(), so this is the first point where the queue
+ * count is final. Grow the arrays if an earlier sizing guessed too low.
+ *
+ * Return: 0 on success, negative error code otherwise.
  */
 static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
 {
-	struct ice_vsi_stats *vsi_stat;
+	struct ice_vsi_stats *old_stat;
 	struct ice_pf *pf = vsi->back;
 
 	if (vsi->type == ICE_VSI_CHNL)
 		return 0;
 	if (!pf->vsi_stats)
 		return -ENOENT;
 
-	if (pf->vsi_stats[vsi->idx])
-	/* realloc will happen in rebuild path */
+	old_stat = pf->vsi_stats[vsi->idx];
+	if (old_stat && old_stat->tx_ring_stats_len >= vsi->alloc_txq &&
+	    old_stat->rx_ring_stats_len >= vsi->alloc_rxq)
 		return 0;
 
-	vsi_stat = ice_vsi_new_stat_arrays(vsi->alloc_txq, vsi->alloc_rxq);
-	if (!vsi_stat)
-		return -ENOMEM;
-
-	pf->vsi_stats[vsi->idx] = vsi_stat;
-	return 0;
+	return ice_vsi_install_stat_arrays(vsi, vsi->alloc_txq,
+					   vsi->alloc_rxq);
 }
 
 /**
-- 
2.51.1

RE: [PATCH net v5 1/6] ice: skip stats handling for channel VSIs on rebuild

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Sent: Friday, September 25, 2026 3:16 PM
> To: netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Schmidt, Michal
> <mschmidt@redhat.com>; intel-wired-lan@lists.osuosl.org;
> edumazet@google.com; horms@kernel.org; pabeni@redhat.com;
> davem@davemloft.net; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [PATCH net v5 1/6] ice: skip stats handling for channel VSIs
> on rebuild
> 
> ice_vsi_alloc_stat_arrays() returns early for ICE_VSI_CHNL, so a
> channel VSI never gets an entry in pf->vsi_stats[].
> ice_vsi_realloc_stat_arrays() dereferences that entry unconditionally,
> and ice_vsi_rebuild() calls it before anything else, so the NULL is
> not filtered out anywhere.
> 
> The path is live. ice_prepare_for_reset() removes the queue channels
> only for resets other than a PFR, so a PFR leaves the channel VSIs in
> place, and ice_rebuild() then calls ice_rebuild_channels(), which
> rebuilds every ICE_VSI_CHNL VSI it finds. A PF reset with ADQ (mqprio
> hardware offload) configured thus dereferences NULL.
> 
> That combination is reset recovery on top of an active mqprio offload,
> which is why it went unnoticed. It was found while refactoring this
> code, not reported by a user.
> 
> I'm leaning towards removing our limited (compared to OOT ADQ)
> support, but it's outside of this series. Bug could be triggered by:
> 
>   tc qdisc del dev "$IF" root 2>/dev/null || true
>   tc qdisc add dev "$IF" root mqprio num_tc 2 \
>     map 0 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 \
>     queues 2@0 2@2 hw 1 mode channel
>   sleep 3
>   ethtool --reset "$IF" irq dma filter offload
> 
> Workqueue: ice ice_service_task [ice]
> RIP: 0010:ice_vsi_rebuild+0x289/0x380 [ice]
>  ice_rebuild_channels+0xd6/0x320 [ice]
>  ice_rebuild+0x4f6/0x540 [ice]
>  ice_do_reset+0x9f/0x1a0 [ice]
>  ice_service_task+0x4a/0x460 [ice]
> 
> Fixes: 5995ef88e3a8 ("ice: realloc VSI stats arrays")
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> v5: new patch, split out of "ice: rebuild ring stats arrays instead of
>     reallocating them in place" (Clashiko)
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 9e08db376d3d..31af378aa0e7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -3031,6 +3031,10 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi
> *vsi)
>  	u16 prev_rxq = vsi->alloc_rxq;
>  	int i;
> 
> +	/* channel VSIs have no entry in pf->vsi_stats[] */
> +	if (vsi->type == ICE_VSI_CHNL)
> +		return 0;
> +
>  	vsi_stat = pf->vsi_stats[vsi->idx];
> 
>  	if (req_txq < prev_txq) {
> --
> 2.51.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [PATCH net v5 2/6] ice: extract __ice_vsi_free_stats()

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Sent: Friday, September 25, 2026 3:16 PM
> To: netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Schmidt, Michal
> <mschmidt@redhat.com>; intel-wired-lan@lists.osuosl.org;
> edumazet@google.com; horms@kernel.org; pabeni@redhat.com;
> davem@davemloft.net; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [PATCH net v5 2/6] ice: extract __ice_vsi_free_stats()
> 
> Record the length of each ring stats array in struct ice_vsi_stats, so
> that freeing the array entries no longer needs the owning VSI. Keep
> the new fields in sync in both places that size the arrays.
> 
> With that, the body of ice_vsi_free_stats() becomes independent of the
> VSI and can be split out as __ice_vsi_free_stats(). The @free_entries
> parameter is always true here; a later commit adds a caller that frees
> only the array container, after its entries have been handed over to a
> freshly allocated stats structure.
> 
> The free path now stops at the recorded length instead of the VSI
> queue count, so it follows the real allocation rather than a number
> that can disagree with it. Sizing still does not: "ice: rebuild ring
> stats arrays instead of reallocating them in place" starts using the
> lengths to carry entries over, and only "ice: size ring stats arrays
> from the final queue count" makes them authoritative for the
> allocation path.
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> v5:
>  - drop the "no functional change intended" claim, the bound of the
> free
>    loop changes owner here (Clashiko)
>  - state that the recorded lengths govern only the free path at this
>    point in the series (Clashiko)
> ---
>  drivers/net/ethernet/intel/ice/ice.h     |  2 +
>  drivers/net/ethernet/intel/ice/ice_lib.c | 52 ++++++++++++++---------
> -
>  2 files changed, 33 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index db3c7015c56c..fadfe94bf1c8 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -328,6 +328,8 @@ enum ice_vsi_state {  struct ice_vsi_stats {
>  	struct ice_ring_stats **tx_ring_stats;  /* Tx ring stats array
> */
>  	struct ice_ring_stats **rx_ring_stats;  /* Rx ring stats array
> */
> +	u16 tx_ring_stats_len;  /* Length of the Tx ring stats array */
> +	u16 rx_ring_stats_len;  /* Length of the Rx ring stats array */
>  };
> 
>  /* struct that defines a VSI, associated with a dev */ diff --git
> a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 31af378aa0e7..1414127d32cd 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -330,42 +330,48 @@ static void ice_vsi_free_arrays(struct ice_vsi
> *vsi)
>  	vsi->rxq_map = NULL;
>  }
> 
> +/* free single stats memory */
> +static void __ice_vsi_free_stats(struct ice_vsi_stats *vsi_stat, bool
> +free_entries) {
> +	if (!vsi_stat)
> +		return;
> +
> +	if (free_entries) {
> +		for (int i = 0; i < vsi_stat->tx_ring_stats_len; i++) {
> +			if (vsi_stat->tx_ring_stats[i]) {
> +				kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
> +				WRITE_ONCE(vsi_stat->tx_ring_stats[i],
> NULL);
> +			}
> +		}
> +		for (int i = 0; i < vsi_stat->rx_ring_stats_len; i++) {
> +			if (vsi_stat->rx_ring_stats[i]) {
> +				kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
> +				WRITE_ONCE(vsi_stat->rx_ring_stats[i],
> NULL);
> +			}
> +		}
> +	}
> +
> +	kfree(vsi_stat->tx_ring_stats);
> +	kfree(vsi_stat->rx_ring_stats);
> +	kfree(vsi_stat);
> +}
> +
>  /**
>   * ice_vsi_free_stats - Free the ring statistics structures
>   * @vsi: VSI pointer
>   */
>  static void ice_vsi_free_stats(struct ice_vsi *vsi)  {
>  	struct ice_vsi_stats *vsi_stat;
>  	struct ice_pf *pf = vsi->back;
> -	int i;
> 
>  	if (vsi->type == ICE_VSI_CHNL)
>  		return;
>  	if (!pf->vsi_stats)
>  		return;
> 
>  	vsi_stat = pf->vsi_stats[vsi->idx];
> -	if (!vsi_stat)
> -		return;
> -
> -	ice_for_each_alloc_txq(vsi, i) {
> -		if (vsi_stat->tx_ring_stats[i]) {
> -			kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
> -			WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
> -		}
> -	}
> -
> -	ice_for_each_alloc_rxq(vsi, i) {
> -		if (vsi_stat->rx_ring_stats[i]) {
> -			kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
> -			WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
> -		}
> -	}
> -
> -	kfree(vsi_stat->tx_ring_stats);
> -	kfree(vsi_stat->rx_ring_stats);
> -	kfree(vsi_stat);
> +	__ice_vsi_free_stats(vsi_stat, true);
>  	pf->vsi_stats[vsi->idx] = NULL;
>  }
> 
> @@ -539,11 +545,13 @@ static int ice_vsi_alloc_stat_arrays(struct
> ice_vsi *vsi)
>  		kzalloc_objs(*vsi_stat->tx_ring_stats, vsi->alloc_txq);
>  	if (!vsi_stat->tx_ring_stats)
>  		goto err_alloc_tx;
> +	vsi_stat->tx_ring_stats_len = vsi->alloc_txq;
> 
>  	vsi_stat->rx_ring_stats =
>  		kzalloc_objs(*vsi_stat->rx_ring_stats, vsi->alloc_rxq);
>  	if (!vsi_stat->rx_ring_stats)
>  		goto err_alloc_rx;
> +	vsi_stat->rx_ring_stats_len = vsi->alloc_rxq;
> 
>  	pf->vsi_stats[vsi->idx] = vsi_stat;
> 
> @@ -3055,6 +3063,7 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
>  		vsi_stat->tx_ring_stats = tx_ring_stats;
>  		return -ENOMEM;
>  	}
> +	vsi_stat->tx_ring_stats_len = req_txq;
> 
>  	if (req_rxq < prev_rxq) {
>  		for (i = req_rxq; i < prev_rxq; i++) { @@ -3074,6
> +3083,7 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
>  		vsi_stat->rx_ring_stats = rx_ring_stats;
>  		return -ENOMEM;
>  	}
> +	vsi_stat->rx_ring_stats_len = req_rxq;
> 
>  	return 0;
>  }
> --
> 2.51.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [PATCH net v5 3/6] ice: extract ice_vsi_new_stat_arrays()

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Sent: Friday, September 25, 2026 3:16 PM
> To: netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Schmidt, Michal
> <mschmidt@redhat.com>; intel-wired-lan@lists.osuosl.org;
> edumazet@google.com; horms@kernel.org; pabeni@redhat.com;
> davem@davemloft.net; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [PATCH net v5 3/6] ice: extract ice_vsi_new_stat_arrays()
> 
> Split the allocation of a struct ice_vsi_stats and its two ring stats
> arrays out of ice_vsi_alloc_stat_arrays(), which is left with just the
> lookup and the store into pf->vsi_stats[].
> 
> The sole caller keeps passing the very same queue counts as before,
> vsi->alloc_txq and vsi->alloc_rxq. A later commit adds a second caller
> in the rebuild path, which cannot use vsi->alloc_* because it has to
> size the arrays before the VSI is reconfigured.
> 
> Note that the error path no longer clears pf->vsi_stats[vsi->idx]; the
> early return above guarantees it is already NULL.
> 
> No functional change intended.
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.c | 47 +++++++++++++----------
> -
>  1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 1414127d32cd..ed32ae2667c5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -519,6 +519,30 @@ static irqreturn_t ice_msix_clean_rings(int
> __always_unused irq, void *data)
>  	return IRQ_HANDLED;
>  }
> 
> +static struct ice_vsi_stats *ice_vsi_new_stat_arrays(int txq, int
> rxq)
> +{
> +	struct ice_ring_stats **tx_ring_stats;
> +	struct ice_ring_stats **rx_ring_stats;
> +	struct ice_vsi_stats *vsi_stat;
> +
> +	vsi_stat = kzalloc_obj(*vsi_stat);
> +	tx_ring_stats = kzalloc_objs(*tx_ring_stats, txq);
> +	rx_ring_stats = kzalloc_objs(*rx_ring_stats, rxq);
> +	if (!vsi_stat || !tx_ring_stats || !rx_ring_stats) {
> +		kfree(vsi_stat);
> +		kfree(tx_ring_stats);
> +		kfree(rx_ring_stats);
> +		return NULL;
> +	}
> +
> +	vsi_stat->tx_ring_stats = tx_ring_stats;
> +	vsi_stat->rx_ring_stats = rx_ring_stats;
> +	vsi_stat->tx_ring_stats_len = txq;
> +	vsi_stat->rx_ring_stats_len = rxq;
> +
> +	return vsi_stat;
> +}
> +
>  /**
>   * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
>   * @vsi: VSI pointer
> @@ -537,33 +561,12 @@ static int ice_vsi_alloc_stat_arrays(struct
> ice_vsi *vsi)
>  	/* realloc will happen in rebuild path */
>  		return 0;
> 
> -	vsi_stat = kzalloc_obj(*vsi_stat);
> +	vsi_stat = ice_vsi_new_stat_arrays(vsi->alloc_txq, vsi-
> >alloc_rxq);
>  	if (!vsi_stat)
>  		return -ENOMEM;
> 
> -	vsi_stat->tx_ring_stats =
> -		kzalloc_objs(*vsi_stat->tx_ring_stats, vsi->alloc_txq);
> -	if (!vsi_stat->tx_ring_stats)
> -		goto err_alloc_tx;
> -	vsi_stat->tx_ring_stats_len = vsi->alloc_txq;
> -
> -	vsi_stat->rx_ring_stats =
> -		kzalloc_objs(*vsi_stat->rx_ring_stats, vsi->alloc_rxq);
> -	if (!vsi_stat->rx_ring_stats)
> -		goto err_alloc_rx;
> -	vsi_stat->rx_ring_stats_len = vsi->alloc_rxq;
> -
>  	pf->vsi_stats[vsi->idx] = vsi_stat;
> -
>  	return 0;
> -
> -err_alloc_rx:
> -	kfree(vsi_stat->rx_ring_stats);
> -err_alloc_tx:
> -	kfree(vsi_stat->tx_ring_stats);
> -	kfree(vsi_stat);
> -	pf->vsi_stats[vsi->idx] = NULL;
> -	return -ENOMEM;
>  }
> 
>  /**
> --
> 2.51.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [PATCH net v5 4/6] ice: extract ice_vsi_get_num_qs()

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Sent: Friday, September 25, 2026 3:16 PM
> To: netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Schmidt, Michal
> <mschmidt@redhat.com>; intel-wired-lan@lists.osuosl.org;
> edumazet@google.com; horms@kernel.org; pabeni@redhat.com;
> davem@davemloft.net; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [PATCH net v5 4/6] ice: extract ice_vsi_get_num_qs()
> 
> ice_vsi_set_num_qs() mixed two things: deciding how many queues a VSI
> gets, and applying all the side effects of that decision. Split the
> decision out into ice_vsi_get_num_qs(), returning both counts at once.
> 
> To make that possible, group alloc_txq and alloc_rxq in struct ice_vsi
> with struct_group_tagged(), which gives the return type without adding
> any new storage. Field order changes slightly, num_txq now follows
> alloc_rxq, but nothing depends on it.
> 
> ice_vsi_get_num_qs() takes @held_txq and @held_rxq from the start,
> even though the only caller so far passes zero for both. They let a
> caller ask what the counts would be once the queues the VSI currently
> owns are back in the PF pool. A later commit adds the caller that
> needs this: the rebuild path has to size its ring stats arrays before
> ice_vsi_decfg() releases the queues, yet the arrays must match what
> ice_vsi_set_num_qs() computes afterwards.
> 
> While reshaping the whole body, drop the kernel-doc line promising a
> return value; ice_vsi_set_num_qs() returns void.
> 
> No functional change intended.
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> v5:
>  - drop the stale "Return 0 on success and a negative value on error"
>    from the kernel-doc of the void ice_vsi_set_num_qs() (Clashiko)
> ---
>  drivers/net/ethernet/intel/ice/ice.h     |  6 +-
>  drivers/net/ethernet/intel/ice/ice_lib.c | 88 ++++++++++++++---------
> -
>  2 files changed, 55 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index fadfe94bf1c8..f1ba86d066be 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -403,9 +403,11 @@ struct ice_vsi {
>  	u8 rx_mapping_mode;		 /* ICE_MAP_MODE_[CONTIG|SCATTER] */
>  	u16 *txq_map;			 /* index in pf->avail_txqs */
>  	u16 *rxq_map;			 /* index in pf->avail_rxqs */

...

>  		break;
>  	}
> 
> --
> 2.51.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [PATCH net v5 5/6] ice: rebuild ring stats arrays instead of reallocating them in place

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Sent: Friday, September 25, 2026 3:16 PM
> To: netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Schmidt, Michal
> <mschmidt@redhat.com>; intel-wired-lan@lists.osuosl.org;
> edumazet@google.com; horms@kernel.org; pabeni@redhat.com;
> davem@davemloft.net; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [PATCH net v5 5/6] ice: rebuild ring stats arrays instead of
> reallocating them in place
> 
> ice_vsi_realloc_stat_arrays() resized the ring stats arrays in place
> with krealloc_array(), sizing them from vsi->req_txq/req_rxq. That is
> not what ice_vsi_set_num_qs() computes later in ice_vsi_cfg_def(), so
> after a rebuild the arrays could end up shorter than vsi->alloc_txq /
> vsi->alloc_rxq, and ice_vsi_alloc_ring_stats() then walked past their
> end.
> 
> The VF case shows the divergence. ice_sriov_set_msix_vec_count()
> writes
> vsi->req_txq and then calls ice_vsi_rebuild(), so the old resizer
> sized
> from req_txq, while ice_vsi_get_num_qs() reads vf->num_req_qs ?:
> vf->num_vf_qs and ignores req_txq entirely. Once a guest has raised
> vf->num_req_qs through VIRTCHNL_OP_REQUEST_QUEUES the two disagree and
> the arrays come out shorter than the alloc_txq/alloc_rxq the rebuild
> installs. The reset that such a request triggers is a different path,
> ice_vf_reconfig_vsi(), which does not use this resizer at all; that
> one is handled by the next patch.
> 
> Replace it with ice_vsi_resize_stat_arrays(), which allocates a fresh
> struct ice_vsi_stats instead, sized with ice_vsi_get_num_qs() and the
> queues ice_vsi_decfg() is about to return to the PF pool, which is
> exactly what ice_vsi_set_num_qs() will compute once they are back
> there, barring a concurrent change of the PF pool; the next patch
> stops relying on that. Copy the surviving entry pointers over and
> install the new structure, all before ice_vsi_decfg() runs. The
> entries that did not fit are freed by
> ice_vsi_free_unused_stat_arrays(); the old container itself then goes
> through __ice_vsi_free_stats() with @free_entries set to false, since
> the entries it still points at now belong to the new structure.
> 
> Allocating up front also means a failure no longer leaves a half-
> updated ice_vsi_stats behind, with the Tx array already swapped and
> its surplus Tx entries already freed, but the Rx allocation failed.
> 
> Put the allocate-install-free-surplus sequence in
> ice_vsi_install_stat_arrays(), next to ice_vsi_new_stat_arrays(), so
> that only the sizing decision is left in the resizer.
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> this removes some tech debt, but it is not independently backportable,
> as it sits on three preceding refactors; the NULL dereference and the
> sizing mismatch are fixed by their own patches, which carry Fixes:
> tags
> 
> v5:
>  - drop the false "requesting fewer queues than the PF pool can hand
>    out" reproducer, use the VF one that the code can exhibit
> (Clashiko)
>  - describe the real failure-path benefit, the old code already
> returned
>    before ice_vsi_decfg() with the VSI fully configured (Clashiko)
>  - split the ICE_VSI_CHNL guard into its own patch with a Fixes: tag
>    (Clashiko)
>  - factor out ice_vsi_install_stat_arrays(), reused by the new sizing
>    patch (Clashiko)
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.c | 144 ++++++++++++++--------
> -
>  1 file changed, 88 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 134984077d5d..448d3c7780ad 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -557,6 +557,80 @@ static struct ice_vsi_stats
> *ice_vsi_new_stat_arrays(int txq, int rxq)
>  	return vsi_stat;
>  }
> 

...

>  	if (ret)
>  		goto unlock;
> 
> --
> 2.51.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [PATCH net v5 6/6] ice: size ring stats arrays from the final queue count

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Sent: Friday, September 25, 2026 3:16 PM
> To: netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Schmidt, Michal
> <mschmidt@redhat.com>; intel-wired-lan@lists.osuosl.org;
> edumazet@google.com; horms@kernel.org; pabeni@redhat.com;
> davem@davemloft.net; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [PATCH net v5 6/6] ice: size ring stats arrays from the final
> queue count
> 
> The ring stats arrays were sized in one place and indexed in another,
> with nothing reconciling the two. ice_vsi_alloc_stat_arrays() gave up
> as soon as a container existed:
> 
> 	if (pf->vsi_stats[vsi->idx])
> 	/* realloc will happen in rebuild path */
> 		return 0;
> 
> while ice_vsi_alloc_ring_stats() walks the arrays with
> ice_for_each_alloc_txq() / ice_for_each_alloc_rxq(), that is, up to
> vsi->alloc_txq / vsi->alloc_rxq. Whenever the arrays are shorter than
> the count ice_vsi_set_num_qs() ends up computing, that loop reads and
> stores past the end of the kmalloc'ed pointer arrays.
> 
> Deferring to "the rebuild path" does not hold. It cannot get the size
> right, and some paths never call it at all.
> 
> ice_devlink_reinit_down() is one of the latter. It calls
> ice_vsi_decfg() directly, which does not free pf->vsi_stats[], and
> nothing on the way down releases the main VSI's entry either:
> ice_dealloc_vsis() runs only from ice_init()/ice_deinit(), and the
> single entry that ice_unload() does drop is the control VSI's, via
> ice_deinit_fdir(). The main VSI's arrays therefore survive the reload,
> while ice_devlink_reinit_up() runs
> ice_vsi_cfg() and recomputes the count from
> netif_get_num_default_rss_queues(), which follows cpu_online_mask.
> Sizing the arrays with most CPUs offline and then onlining them is
> enough, no VF involved:
> 
>   for c in /sys/devices/system/cpu/cpu[0-9]*/online; do echo 0 > $c;
> done
>   echo 0000:18:00.0 > /sys/bus/pci/drivers/ice/unbind
>   echo 0000:18:00.0 > /sys/bus/pci/drivers/ice/bind
>   for c in /sys/devices/system/cpu/cpu[0-9]*/online; do echo 1 > $c;
> done
>   devlink dev reload pci/0000:18:00.0 action driver_reinit
> 
> The unbind/bind is what makes the arrays actually be allocated at the
> small size; a reload alone only reuses them.
> 
> The read then runs off the end of tx_ring_stats[] and picks up
> whatever follows it in kmalloc-32. Here that was a string,
> 0x74756f5f6c697475 spelling "util_out" in memory order. It is stored
> into ring->ring_stats and faults on the first dereference, in
> ice_setup_tx_ring(), call trace:
> 
>  RIP: 0010:ice_setup_tx_ring+0x8d/0xe0 [ice]
>   ice_vsi_setup_tx_rings+0x2a/0x80 [ice]
>   ice_vsi_open+0x28/0x170 [ice]
>   ice_open_internal+0xc9/0x160 [ice]
>   __dev_open+0x138/0x2b0
>   __dev_change_flags+0x1d5/0x250
>   netif_change_flags+0x21/0x60
>   do_setlink.constprop.0+0x336/0xcf0
>   rtnl_newlink+0x4a4/0x9b0
> 
> Even where the rebuild path does run, it sizes the arrays before
> ice_vsi_decfg() returns the queues to the PF pool, and for an
> ICE_VSI_PF VSI with no explicit request it derives the count from that
> shared pool. Nothing serializes it against the recount in
> ice_vsi_set_num_qs(), so a concurrent release, say "echo 0 >
> sriov_numvfs", leaves the final count larger than the arrays that were
> already installed.
> 
> ice_vsi_alloc_stat_arrays() runs from ice_vsi_cfg_def(), right after
> ice_vsi_alloc_def() called ice_vsi_set_num_qs(), so it is the first
> place where the queue count is final. Make it the only authority: keep
> the existing arrays when they are already long enough, and otherwise
> grow them with ice_vsi_install_stat_arrays(), which carries the
> surviving entries over. The check only forces a grow: while both
> arrays are long enough they are kept, an oversized one being merely
> wasteful.
> Once either is too short the whole container is replaced, and then the
> other one follows the final count too, shrinking if that is what it
> takes.
> 
> Michal Schmidt hit the same corruption through the VF path, where a
> guest raises its queue count with VIRTCHNL_OP_REQUEST_QUEUES and the
> following VF reset reconfigures the VSI with the larger count while
> the old, shorter arrays are still installed:
> 
>  BUG: KASAN: slab-out-of-bounds in
> ice_vsi_alloc_ring_stats+0x385/0x4a0 [ice]
>  Workqueue: ice ice_service_task [ice]
>  Call Trace:
>   kasan_report+0xd7/0x120
>   ice_vsi_alloc_ring_stats+0x385/0x4a0 [ice]
>   ice_vsi_cfg_def+0x12e2/0x2060 [ice]
>   ice_vsi_cfg+0xb5/0x3c0 [ice]
>   ice_reset_vf+0x858/0xf80 [ice]
>   ice_vc_request_qs_msg+0x1da/0x290 [ice]
>   ice_vc_process_vf_msg+0xb15/0x1430 [ice]
>   __ice_clean_ctrlq+0x70d/0x9d0 [ice]
>   ice_service_task+0x840/0xf20 [ice]
>   process_one_work+0x690/0xff0
>   worker_thread+0x4d9/0xd20
>   kthread+0x322/0x410
>   ret_from_fork+0x332/0x660
>   ret_from_fork_asm+0x1a/0x30
> 
>  Allocated by task 2439:
>   kasan_save_stack+0x1c/0x40
>   kasan_save_track+0x10/0x30
>   __kasan_kmalloc+0x96/0xb0
>   __kmalloc_noprof+0x1d8/0x580
>   ice_vsi_cfg_def+0x115c/0x2060 [ice]
>   ice_vsi_cfg+0xb5/0x3c0 [ice]
>   ice_vsi_setup+0x180/0x320 [ice]
>   ice_start_vfs+0x1f3/0x590 [ice]
>   ice_ena_vfs+0x66d/0x798 [ice]
>   ice_sriov_configure.cold+0xe4/0x121 [ice]
>   sriov_numvfs_store+0x279/0x480
>   kernfs_fop_write_iter+0x331/0x4f0
>   vfs_write+0x4c4/0xe40
>   ksys_write+0x10c/0x240
>   do_syscall_64+0xd9/0x650
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
> 
>  The buggy address belongs to the object at ffff88810affea40
>                 which belongs to the cache kmalloc-32 of size 32  The
> buggy address is located 0 bytes to the right of
>                 allocated 32-byte region [ffff88810affea40,
> ffff88810affea60)
> 
> Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset")
> Reported-by: Michal Schmidt <mschmidt@redhat.com>
> Closes: https://redhat.atlassian.net/browse/RHEL-164321
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> v5:
>  - new patch, replaces the VF-only fix with one that covers every path
>    reaching ice_vsi_alloc_ring_stats() (Clashiko)
>  - the reproducers both postdate the cited tag: devlink reinit arrived
>    with 31c8db2c4fa7 ("ice: implement devlink reinit action"), the VF
>    path with 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with
>    ice_vf_reconfig_vsi()"). The sizing itself went wrong in the cited
>    commit, hence the older tag (Clashiko)
>  - drop the follow-up VF resize patch, it could abort a VFLR after the
>    hardware had already reset the VF (Clashiko)
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 448d3c7780ad..c393d913d037 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -634,27 +634,29 @@ static int ice_vsi_install_stat_arrays(struct
> ice_vsi *vsi, u16 txq, u16 rxq)
>  /**
>   * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
>   * @vsi: VSI pointer
> + *
> + * Runs after ice_vsi_set_num_qs(), so this is the first point where
> + the queue
> + * count is final. Grow the arrays if an earlier sizing guessed too
> low.
> + *
> + * Return: 0 on success, negative error code otherwise.
>   */
>  static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)  {
> -	struct ice_vsi_stats *vsi_stat;
> +	struct ice_vsi_stats *old_stat;
>  	struct ice_pf *pf = vsi->back;
> 
>  	if (vsi->type == ICE_VSI_CHNL)
>  		return 0;
>  	if (!pf->vsi_stats)
>  		return -ENOENT;
> 
> -	if (pf->vsi_stats[vsi->idx])
> -	/* realloc will happen in rebuild path */
> +	old_stat = pf->vsi_stats[vsi->idx];
> +	if (old_stat && old_stat->tx_ring_stats_len >= vsi->alloc_txq
> &&
> +	    old_stat->rx_ring_stats_len >= vsi->alloc_rxq)
>  		return 0;
> 
> -	vsi_stat = ice_vsi_new_stat_arrays(vsi->alloc_txq, vsi-
> >alloc_rxq);
> -	if (!vsi_stat)
> -		return -ENOMEM;
> -
> -	pf->vsi_stats[vsi->idx] = vsi_stat;
> -	return 0;
> +	return ice_vsi_install_stat_arrays(vsi, vsi->alloc_txq,
> +					   vsi->alloc_rxq);
>  }
> 
>  /**
> --
> 2.51.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Re: [PATCH net v5 1/6] ice: skip stats handling for channel VSIs on rebuild

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In ice_vsi_realloc_stat_arrays()
  (drivers/net/ethernet/intel/ice/ice_lib.c), the new guard skips only…

--

ice: skip stats handling for channel VSIs on rebuild

This patch makes ice_vsi_realloc_stat_arrays() return early for
ICE_VSI_CHNL VSIs. Those VSIs never get an entry in pf->vsi_stats[], so
this avoids a NULL dereference when a PF reset rebuilds channel VSIs
while an mqprio hardware offload is active.

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 9e08db376d3d0..31af378aa0e7a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -3031,6 +3031,10 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
>  	u16 prev_rxq = vsi->alloc_rxq;
>  	int i;
>  
> +	/* channel VSIs have no entry in pf->vsi_stats[] */
> +	if (vsi->type == ICE_VSI_CHNL)
> +		return 0;
> +
>  	vsi_stat = pf->vsi_stats[vsi->idx];
>  
>  	if (req_txq < prev_txq) {

[Severity: Medium]

This guard only checks the VSI type. Can pf->vsi_stats[vsi->idx] also be
NULL here for a PF, VF or other non-channel VSI?

If an earlier ice_vsi_rebuild() failed inside ice_vsi_cfg_def(), the
unroll path frees the entry:

ice_vsi_cfg_def() {
    ...
unroll_vsi_alloc_stat:
	ice_vsi_free_stats(vsi);
    ...
}

and ice_vsi_free_stats() ends with:

	pf->vsi_stats[vsi->idx] = NULL;

ice_vsi_alloc_ring_stats() does the same on its err_out path.
ice_vsi_rebuild() then returns through goto unlock without restoring the
entry, and the VSI stays in pf->vsi[].

Some callers then call ice_vsi_rebuild() on the same VSI again. One
example is the fallback in ice_setup_tc_mqprio_qdisc():

	ret = ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT);
	if (ret) {
		...
		if (ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT)) {

The same thing can happen through the unroll path in
ice_sriov_set_msix_vec_count(), when userspace retries ethtool -L or -G
through ice_vsi_recfg_qs(), or on a later PF or VF reset.

On that second call vsi_stat is NULL and the type check does not catch
it. If req_txq < prev_txq, the oops happens at
vsi_stat->tx_ring_stats[i]. Otherwise it happens at:

	tx_ring_stats = vsi_stat->tx_ring_stats;

ice_vsi_free_stats() and ice_qp_reset_stats() already handle a NULL
entry. Would a !vsi_stat check here cover both the channel VSI case and
this one?

The new comment, "channel VSIs have no entry in pf->vsi_stats[]", also
implies that only channel VSIs can be missing an entry.

A later patch in this series, "ice: rebuild ring stats arrays instead of
reallocating them in place", appears to fix this. It replaces the
function with ice_vsi_resize_stat_arrays() -> ice_vsi_install_stat_arrays(),
and ice_vsi_set_stat_arrays() handles a NULL entry with:

	if (!vsi_stat)
		return;

Should this patch check for a NULL vsi_stat as well, so that it is a
complete fix on its own?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 2/6] ice: extract __ice_vsi_free_stats()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message understates what the change does and the patch
  has no Fixes: tag.

Pre-existing issues:
- [Critical] Heap out-of-bounds write in ice_vsi_alloc_ring_stats()
  (drivers/net/ethernet/intel/ice/ice_lib.c).
- [Medium] The shrink loops in ice_vsi_realloc_stat_arrays()
  (drivers/net/ethernet/intel/ice/ice_lib.c) still use…

--

ice: extract __ice_vsi_free_stats()

This commit records the length of each ring stats array in struct
ice_vsi_stats and moves the VSI-independent body of ice_vsi_free_stats()
into __ice_vsi_free_stats(). The loops that free the entries now stop at
the recorded length instead of vsi->alloc_txq/alloc_rxq.

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 31af378aa0e7a..1414127d32cd8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -330,6 +330,32 @@ static void ice_vsi_free_arrays(struct ice_vsi *vsi)
>  	vsi->rxq_map = NULL;
>  }
>  
> +/* free single stats memory */
> +static void __ice_vsi_free_stats(struct ice_vsi_stats *vsi_stat, bool free_entries)
> +{
> +	if (!vsi_stat)
> +		return;
> +
> +	if (free_entries) {
> +		for (int i = 0; i < vsi_stat->tx_ring_stats_len; i++) {

[Severity: Low]

The old loop in ice_vsi_free_stats() used ice_for_each_alloc_txq(), so it
stopped at vsi->alloc_txq. This one stops at the recorded array length.
Could the old mismatch actually happen?

It looks reachable through ethtool -L with fewer Tx and more Rx queues:

ice_vsi_recfg_qs()
  ice_vsi_rebuild()
    ice_vsi_realloc_stat_arrays()
      krealloc_array(tx_ring_stats, req_txq, ...)   /* shrink succeeds */
      krealloc_array(rx_ring_stats, req_rxq, ...)   /* grow fails */
      return -ENOMEM;
    goto unlock;                                    /* skips ice_vsi_cfg_def() */

After that, vsi->alloc_txq still has the old, larger value. Later,
ice_vsi_free()->ice_vsi_free_stats() walked tx_ring_stats[] past the end of
the shrunken kmalloc object. Any non-NULL word it found there, for example
SLUB redzone poison under slub_debug, was passed to kfree_rcu().

So this patch appears to fix a heap out-of-bounds read and an invalid
kfree_rcu(). The commit message only says:

  The free path now stops at the recorded length instead of the VSI queue
  count, so it follows the real allocation rather than a number that can
  disagree with it.

Could the message describe the memory-safety effect? Should the patch also
carry a Fixes: tag?

The last patch in the series, "ice: size ring stats arrays from the final
queue count", has Fixes: 288ecf491b16 for the same class of bug and depends
on this one. A stable backport would probably pull this patch in anyway.

> +			if (vsi_stat->tx_ring_stats[i]) {
> +				kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
> +				WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
> +			}
> +		}

[ ... ]

> @@ -539,11 +545,13 @@ static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
>  		kzalloc_objs(*vsi_stat->tx_ring_stats, vsi->alloc_txq);
>  	if (!vsi_stat->tx_ring_stats)
>  		goto err_alloc_tx;
> +	vsi_stat->tx_ring_stats_len = vsi->alloc_txq;

[Severity: Critical]

This is a pre-existing issue, not introduced by this patch. The length is
only set when the container is new. Earlier in ice_vsi_alloc_stat_arrays()
there is an early return:

	if (pf->vsi_stats[vsi->idx])
	/* realloc will happen in rebuild path */
		return 0;

ice_vsi_alloc_ring_stats() fills the arrays up to the queue count and never
checks the recorded length:

	ice_for_each_alloc_txq(vsi, i) {
		...
		ring_stats = tx_ring_stats[i];
		if (!ring_stats) {
			...
			WRITE_ONCE(tx_ring_stats[i], ring_stats);
		}
		...
	}

On the VF reset path a guest can raise the queue count while the arrays
keep their old length:

ice_vc_request_qs_msg()
  ice_reset_vf()
    ice_vf_reconfig_vsi()
      ice_vsi_decfg()
      ice_vsi_cfg()
        ice_vsi_cfg_def()
          ice_vsi_alloc_def()
            ice_vsi_set_num_qs()      /* raises alloc_txq/alloc_rxq */
          ice_vsi_alloc_stat_arrays() /* early return, arrays not resized */
          ice_vsi_alloc_ring_stats()  /* reads/writes past tx_ring_stats[] */

The PF rebuild path has the same mismatch. ice_vsi_realloc_stat_arrays()
sizes the arrays from req_txq or the old alloc_txq before the count is
recomputed.

Can this write kernel pointers past the kmalloc object? After this patch
those pointers would also leak, because __ice_vsi_free_stats() stops at
tx_ring_stats_len.

The commit message says sizing is not authoritative yet. The last patch in
the series, "ice: size ring stats arrays from the final queue count", fixes
this: ice_vsi_alloc_stat_arrays() compares the recorded lengths with
vsi->alloc_txq/alloc_rxq and grows the arrays before
ice_vsi_alloc_ring_stats() runs.

>  
>  	vsi_stat->rx_ring_stats =
>  		kzalloc_objs(*vsi_stat->rx_ring_stats, vsi->alloc_rxq);
>  	if (!vsi_stat->rx_ring_stats)
>  		goto err_alloc_rx;
> +	vsi_stat->rx_ring_stats_len = vsi->alloc_rxq;
>  
>  	pf->vsi_stats[vsi->idx] = vsi_stat;
>  
> @@ -3055,6 +3063,7 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
>  		vsi_stat->tx_ring_stats = tx_ring_stats;
>  		return -ENOMEM;
>  	}
> +	vsi_stat->tx_ring_stats_len = req_txq;

[Severity: Medium]

This isn't a bug introduced by this patch. The shrink loops earlier in
ice_vsi_realloc_stat_arrays() still take their upper bound from the queue
count, not from the new length:

	u16 prev_txq = vsi->alloc_txq;
	...
	if (req_txq < prev_txq) {
		for (i = req_txq; i < prev_txq; i++) {
			if (vsi_stat->tx_ring_stats[i]) {
				kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);

Tx is committed here, before the Rx krealloc_array(). If that allocation
fails, ice_vsi_rebuild() exits through "goto unlock" before ice_vsi_cfg_def()
runs, so vsi->alloc_txq stays larger than tx_ring_stats_len.

On the next rebuild, does this loop read tx_ring_stats[] past the end of the
array? It would also call kfree_rcu() on whatever it finds there.

In the opposite case, len larger than alloc_txq, krealloc_array() seems to
drop the entries in [alloc_txq, len) without freeing them.

Later in the series, "ice: rebuild ring stats arrays instead of reallocating
them in place" removes ice_vsi_realloc_stat_arrays(). Its replacement,
ice_vsi_free_unused_stat_arrays(), frees surplus entries only up to
tx_ring_stats_len/rx_ring_stats_len. It also allocates a fresh container up
front, so the partial-failure state can no longer happen.

>  
>  	if (req_rxq < prev_rxq) {
>  		for (i = req_rxq; i < prev_rxq; i++) {

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 5/6] ice: rebuild ring stats arrays instead of reallocating them in place

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 1 · High: 0 (1) · Medium: 1 (1) · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] ice_vsi_resize_stat_arrays() sizes the arrays from a
  snapshot taken before ice_vsi_decfg(): the PF pool,…
- [Medium] ice_vsi_set_stat_arrays() publishes the new container with a
  plain store (`pf->vsi_stats[vsi->idx] = new_vsi_stat;`, ice_lib.c:592)…
- [Low] This patch fixes a real heap out-of-bounds access but has no
  Fixes: tag, and the Fixes-tagged fix later in the series cannot be…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but a fallback
  rebuild dereferences NULL queue maps.
- [Medium] ice_vsi_free_unused_stat_arrays() calls kfree_rcu() on surplus
  ice_ring_stats entries while vsi->tx_rings[i]->ring_stats and…

--

ice: rebuild ring stats arrays instead of reallocating them in place

This patch replaces ice_vsi_realloc_stat_arrays() with
ice_vsi_resize_stat_arrays(). The new function allocates a fresh struct
ice_vsi_stats before ice_vsi_decfg() runs, sized with
ice_vsi_get_num_qs(). Surviving ring stats entries are copied into the new
container, surplus entries go to kfree_rcu(), and the old container is
freed without its entries, using a new helper,
ice_vsi_install_stat_arrays().

> ice_vsi_realloc_stat_arrays() resized the ring stats arrays in place
> with krealloc_array(), sizing them from vsi->req_txq/req_rxq. That is
> not what ice_vsi_set_num_qs() computes later in ice_vsi_cfg_def(), so
> after a rebuild the arrays could end up shorter than vsi->alloc_txq /
> vsi->alloc_rxq, and ice_vsi_alloc_ring_stats() then walked past their
> end.

[ ... ]

> this removes some tech debt, but it is not independently backportable,
> as it sits on three preceding refactors; the NULL dereference and the
> sizing mismatch are fixed by their own patches, which carry Fixes: tags

[Severity: Low]
The commit message leads with a heap out-of-bounds access. An admin
reaches it by writing sriov_vf_msix_count after a guest has raised
vf->num_req_qs through VIRTCHNL_OP_REQUEST_QUEUES:

ice_sriov_set_msix_vec_count()
  vsi->req_txq = queues;
  ice_vsi_rebuild()

The switch to ice_vsi_get_num_qs() in ice_vsi_resize_stat_arrays() is what
fixes that. Should this patch carry a Fixes: tag, instead of being
described below the cut as tech debt removal?

The later patch "ice: size ring stats arrays from the final queue count"
carries Fixes: 288ecf491b16 and calls ice_vsi_install_stat_arrays(),
which this patch adds. A stable backport of that fix would also need this
patch and the three refactors before it (__ice_vsi_free_stats(),
ice_vsi_new_stat_arrays(), ice_vsi_get_num_qs()).

Could these be marked as stable dependencies so a backporter knows?

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 134984077d5da..448d3c7780adf 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -557,6 +557,80 @@ static struct ice_vsi_stats *ice_vsi_new_stat_arrays(int txq, int rxq)
>  	return vsi_stat;
>  }
>  
> +static void ice_vsi_free_unused_stat_arrays(struct ice_vsi_stats *vsi_stat,
> +					    struct ice_vsi_stats *new_vsi_stat)
> +{
> +	int new_txq = new_vsi_stat->tx_ring_stats_len;
> +	int new_rxq = new_vsi_stat->rx_ring_stats_len;
> +	int prev_txq = vsi_stat->tx_ring_stats_len;
> +	int prev_rxq = vsi_stat->rx_ring_stats_len;
> +
> +	for (int i = new_txq; i < prev_txq; i++) {
> +		if (vsi_stat->tx_ring_stats[i]) {
> +			kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
> +			WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
> +		}
> +	}

[Severity: Medium]
This is a pre-existing issue, and the old krealloc_array() code used the
same order. Are these entries still reachable when kfree_rcu() is called?

At this point vsi->tx_rings[i]->ring_stats and vsi->rx_rings[i]->ring_stats
still point at the surplus entries. The rings are only unpublished later:

ice_vsi_rebuild()
  ice_vsi_resize_stat_arrays()
    ice_vsi_install_stat_arrays()
      ice_vsi_free_unused_stat_arrays()
        kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
  ice_vsi_decfg()
    ice_vsi_clear_rings(vsi);

That grace period does not cover an RCU reader that starts after the
kfree_rcu() and reaches the entry through a ring, such as
__ice_get_ethtool_stats():

	tx_ring = READ_ONCE(vsi->tx_rings[j]);
	if (!tx_ring || !tx_ring->ring_stats) {

The reader paths that were checked look serialized by rtnl,
ICE_VSI_DOWN/ICE_CFG_BUSY, or netif_device_detach(), so the remaining
window looks narrow.

Now that the free is in its own function, could the surplus entries be
freed after ice_vsi_decfg() instead?

> +	for (int i = new_rxq; i < prev_rxq; i++) {

[ ... ]

> +static void ice_vsi_set_stat_arrays(struct ice_vsi *vsi,
> +				    struct ice_vsi_stats *new_vsi_stat)
> +{
> +	u16 new_txq, new_rxq, prev_txq, prev_rxq;
> +	struct ice_vsi_stats *vsi_stat;
> +	struct ice_pf *pf = vsi->back;
> +
> +	new_txq = new_vsi_stat->tx_ring_stats_len;
> +	new_rxq = new_vsi_stat->rx_ring_stats_len;
> +	vsi_stat = pf->vsi_stats[vsi->idx];
> +	pf->vsi_stats[vsi->idx] = new_vsi_stat;
> +	if (!vsi_stat)
> +		return; /* don't copy if there is no source */
> +
> +	prev_txq = vsi_stat->tx_ring_stats_len;
> +	prev_rxq = vsi_stat->rx_ring_stats_len;
> +
> +	memcpy(new_vsi_stat->tx_ring_stats, vsi_stat->tx_ring_stats,
> +	       sizeof(*vsi_stat->tx_ring_stats) * min(prev_txq, new_txq));
> +	memcpy(new_vsi_stat->rx_ring_stats, vsi_stat->rx_ring_stats,
> +	       sizeof(*vsi_stat->rx_ring_stats) * min(prev_rxq, new_rxq));
> +}

[ ... ]

> +static int ice_vsi_install_stat_arrays(struct ice_vsi *vsi, u16 txq, u16 rxq)
> +{
> +	struct ice_vsi_stats *old_stat, *new_stat;
> +
> +	new_stat = ice_vsi_new_stat_arrays(txq, rxq);
> +	if (!new_stat)
> +		return -ENOMEM;
> +
> +	old_stat = vsi->back->vsi_stats[vsi->idx];
> +	ice_vsi_set_stat_arrays(vsi, new_stat);
> +	if (old_stat) {
> +		ice_vsi_free_unused_stat_arrays(old_stat, new_stat);
> +		__ice_vsi_free_stats(old_stat, false);
> +	}

[Severity: Medium]
Can a lockless reader see the new container before the entries are copied
into it? ice_vsi_set_stat_arrays() publishes pf->vsi_stats[vsi->idx] with
a plain store, and only then memcpy()s in the surviving entry pointers.

ice_qp_reset_stats() dereferences an entry without a NULL check:

	vsi_stat = pf->vsi_stats[vsi->idx];
	if (!vsi_stat)
		return;

	memset(&vsi_stat->rx_ring_stats[q_idx]->stats, 0,

It is reached through ice_cfg_txtime() -> ice_qp_dis(), which holds only
rtnl and the ICE_CFG_BUSY bit. The reset path ice_rebuild() ->
ice_vsi_rebuild_by_type(pf, ICE_VSI_PF) -> ice_vsi_rebuild() takes
neither.

A related question: __ice_vsi_free_stats(old_stat, false) kfree()s the
old container and its arrays straight away, with no grace period. Could
ice_qp_reset_stats() still hold the old vsi_stat pointer at that point?

The ETF-vs-reset concurrency itself predates this patch, and the old
krealloc_array() also freed the old arrays synchronously. What is new is
publishing before the copy and freeing the whole container synchronously.

Would doing the memcpy() before the store to pf->vsi_stats[vsi->idx] close
the first window?

[ ... ]

> @@ -3040,69 +3114,27 @@ ice_vsi_rebuild_set_coalesce(struct ice_vsi *vsi,

[ ... ]

> -static int
> -ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
> +static int ice_vsi_resize_stat_arrays(struct ice_vsi *vsi)
>  {

[ ... ]

> +	struct ice_vsi_alloc_queues_params qs;
>  
>  	/* channel VSIs have no entry in pf->vsi_stats[] */
>  	if (vsi->type == ICE_VSI_CHNL)
>  		return 0;
>  

[ ... ]

> +	qs = ice_vsi_get_num_qs(vsi, vsi->alloc_txq + vsi->num_xdp_txq,
> +				vsi->alloc_rxq);

[Severity: Critical]
Can the count computed here differ from the one ice_vsi_set_num_qs()
computes after ice_vsi_decfg()?

This snapshot depends on the PF pool, netif_get_num_default_rss_queues()
and vf->num_req_qs. ice_vsi_set_num_qs() later recomputes from the current
state:

	vsi->alloc_txq_rxq = ice_vsi_get_num_qs(vsi, 0, 0);

Nothing serializes the two calls. At this revision
ice_vsi_alloc_stat_arrays() keeps the installed arrays without checking
their length:

	if (pf->vsi_stats[vsi->idx])
	/* realloc will happen in rebuild path */
		return 0;

ice_vsi_alloc_ring_stats() then walks ice_for_each_alloc_txq() over
tx_ring_stats[]. Could it write past the end of tx_ring_stats[] or
rx_ring_stats[] if the second count is larger? One example is a PF rebuild
with req_txq == 0 while pool queues are freed, or CPUs come online, in
between.

The commit message acknowledges this ("barring a concurrent change of the
PF pool; the next patch stops relying on that"), and the baseline had the
same class of problem.

The later patch "ice: size ring stats arrays from the final queue count"
closes it. There, ice_vsi_alloc_stat_arrays() grows the arrays whenever
the stored lengths are below the final alloc_txq/alloc_rxq. This
intermediate revision is still exposed.

>  
> -	if (req_rxq < prev_rxq) {

[ ... ]

> -	return 0;
> +	return ice_vsi_install_stat_arrays(vsi, qs.alloc_txq, qs.alloc_rxq);
>  }
>  
>  /**
> @@ -3132,7 +3164,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
>  
>  	mutex_lock(&vsi->xdp_state_lock);
>  
> -	ret = ice_vsi_realloc_stat_arrays(vsi);
> +	ret = ice_vsi_resize_stat_arrays(vsi);
>  	if (ret)
>  		goto unlock;
>  

[Severity: High]
This isn't a bug introduced by this patch, but what happens when
ice_vsi_rebuild() is retried after a failure that already ran
ice_vsi_decfg()?

Both ice_setup_tc_mqprio_qdisc() and ice_sriov_set_msix_vec_count() call
ice_vsi_rebuild() again as a fallback:

	if (ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT)) {
		dev_err(dev, "Rebuild of main VSI failed again\n");

The first attempt can fail late. One case is the coalesce allocation in
ice_vsi_rebuild():

	coalesce = kzalloc_objs(struct ice_coalesce_stored, vsi->num_q_vectors);
	if (!coalesce) {
		ret = -ENOMEM;
		goto decfg;
	}

The other is ice_vsi_cfg_def() unrolling. Either way,
ice_vsi_free_arrays() has set vsi->txq_map = NULL, but alloc_txq and
alloc_rxq keep their values.

The retry now gets past ice_vsi_resize_stat_arrays(), which handles a NULL
old container, and calls ice_vsi_decfg() unconditionally:

ice_vsi_rebuild()
  ice_vsi_decfg()
    ice_vsi_put_qs()
      clear_bit(vsi->txq_map[i], pf->avail_txqs);

Does this dereference the NULL txq_map? After a cfg_def unroll,
num_q_vectors is also still non-zero. Can ice_vsi_free_q_vectors() index
the NULL q_vectors before that?

In the baseline the retry crashed earlier, in
ice_vsi_realloc_stat_arrays(). This patch moves the crash point but does
not add a guard.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 6/6] ice: size ring stats arrays from the final queue count

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (2) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message says ice_vsi_alloc_stat_arrays() becomes
  "the only authority" on the size of the ring stats arrays.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  ice_vsi_rebuild() calls ice_vsi_resize_stat_arrays() before…
- [Medium] This problem wasn't introduced by this patch, but in
  ice_vsi_cfg_def(), if ice_vsi_alloc_ring_stats() fails after…

--

ice: size ring stats arrays from the final queue count

This patch changes ice_vsi_alloc_stat_arrays() so it no longer returns early
whenever a stats container already exists. It now checks the existing ring
stats arrays against the final vsi->alloc_txq / vsi->alloc_rxq, and grows
them with ice_vsi_install_stat_arrays() when they are too short. This fixes
out-of-bounds accesses in ice_vsi_alloc_ring_stats() that are reachable
through devlink reinit after CPU hotplug and through VF queue count requests.

Closes: https://redhat.atlassian.net/browse/RHEL-164321

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 448d3c7780adf..c393d913d0379 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -634,10 +634,15 @@ static int ice_vsi_install_stat_arrays(struct ice_vsi *vsi, u16 txq, u16 rxq)
>  /**
>   * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
>   * @vsi: VSI pointer
> + *
> + * Runs after ice_vsi_set_num_qs(), so this is the first point where the queue
> + * count is final. Grow the arrays if an earlier sizing guessed too low.
> + *
> + * Return: 0 on success, negative error code otherwise.
>   */

[ ... ]

> @@ -645,16 +650,13 @@ static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
>  	if (!pf->vsi_stats)
>  		return -ENOENT;
>  
> -	if (pf->vsi_stats[vsi->idx])
> -	/* realloc will happen in rebuild path */
> +	old_stat = pf->vsi_stats[vsi->idx];
> +	if (old_stat && old_stat->tx_ring_stats_len >= vsi->alloc_txq &&
> +	    old_stat->rx_ring_stats_len >= vsi->alloc_rxq)
>  		return 0;

[Severity: Medium]
The commit message says:

  "Make it the only authority: keep the existing arrays when they are
  already long enough, and otherwise grow them with
  ice_vsi_install_stat_arrays()"

Is ice_vsi_alloc_stat_arrays() really the only authority after this patch?
ice_vsi_rebuild() still sizes the arrays before ice_vsi_decfg():

drivers/net/ethernet/intel/ice/ice_lib.c:ice_vsi_rebuild() {
    ...
	ret = ice_vsi_resize_stat_arrays(vsi);
	if (ret)
		goto unlock;

	ice_vsi_decfg(vsi);
	ret = ice_vsi_cfg_def(vsi);
    ...
}

ice_vsi_resize_stat_arrays() can still grow or shrink the arrays to the
pre-decfg estimate:

	qs = ice_vsi_get_num_qs(vsi, vsi->alloc_txq + vsi->num_xdp_txq,
				vsi->alloc_rxq);

	return ice_vsi_install_stat_arrays(vsi, qs.alloc_txq, qs.alloc_rxq);

So every rebuild still has two sizing points. The order is resize, then
decfg, then cfg_def, then alloc_stat_arrays.

Two comments also still describe the old guarantee. The kernel-doc of
ice_vsi_resize_stat_arrays() says:

 * Call while @vsi still owns its queues and before ice_vsi_decfg() returns them
 * to the PF pool, so that the new size is what ice_vsi_set_num_qs() will compute
 * afterwards.

The comment above ice_vsi_get_num_qs() says:

/* @held_txq, @held_rxq: queues the VSI still owns but is about to return to the
 * PF pool, so that the result matches what it will be once they are back there.
 */

Both conflict with the new kernel-doc here ("an earlier sizing guessed too
low") and with the commit message ("It cannot get the size right"). Should
those comments be updated, or should ice_vsi_resize_stat_arrays() go away?

There may also be a functional side effect. Suppose the estimate is below
the current length. Then ice_vsi_install_stat_arrays() calls
ice_vsi_free_unused_stat_arrays(), which runs kfree_rcu() on the accumulated
ring_stats in [estimate, prev_len).

If ice_vsi_set_num_qs() then computes a larger count, the grow here leaves
the new slots NULL, and ice_vsi_alloc_ring_stats() fills them with zeroed
ring_stats. Doesn't that lose the counters for those rings, which
288ecf491b16 ("ice: Accumulate ring statistics over reset") is meant to
keep?

This needs netif_get_num_default_rss_queues() to change between the two
sizing points, for example CPU hotplug during a rebuild. The sriov_numvfs
race described in the commit message only makes the resize grow too little,
because held_txq already covers the VSI's own queues. The out-of-bounds
access itself does look fixed.

> -	vsi_stat = ice_vsi_new_stat_arrays(vsi->alloc_txq, vsi->alloc_rxq);
> -	if (!vsi_stat)
> -		return -ENOMEM;
> -
> -	pf->vsi_stats[vsi->idx] = vsi_stat;
> -	return 0;
> +	return ice_vsi_install_stat_arrays(vsi, vsi->alloc_txq,
> +					   vsi->alloc_rxq);
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but the new slots left by this
grow are NULL, so ice_vsi_alloc_ring_stats() has to allocate them. If that
allocation fails in ice_vsi_cfg_def(), the code jumps to unroll_vector_base
after ice_vsi_alloc_rings() has already succeeded:

		ret = ice_vsi_alloc_rings(vsi);
		if (ret)
			goto unroll_vector_base;

		ret = ice_vsi_alloc_ring_stats(vsi);
		if (ret)
			goto unroll_vector_base;

The unwind is:

unroll_vector_base:
	/* reclaim SW interrupts back to the common pool */
unroll_alloc_q_vector:
	ice_vsi_free_q_vectors(vsi);
unroll_vsi_init:
	ice_vsi_delete_from_hw(vsi);
unroll_get_qs:
	ice_vsi_put_qs(vsi);
unroll_vsi_alloc_stat:
	ice_vsi_free_stats(vsi);
unroll_vsi_alloc:
	ice_vsi_free_arrays(vsi);

Nothing in it calls ice_vsi_clear_rings(). ice_free_q_vector() only clears
tx_ring->q_vector / rx_ring->q_vector. ice_vsi_free_arrays() devm_kfree()s
vsi->tx_rings / vsi->rx_rings and sets them to NULL.

Does this leak every ice_tx_ring and ice_rx_ring that ice_vsi_alloc_rings()
allocated?

The VF and LB cases take the same label when ice_vsi_alloc_ring_stats()
fails. The PF case also takes it when the XDP setup fails. This path could
be reached just as easily before this patch, both on first configuration and
after the rebuild-path grow.

[Severity: Medium]
This is a pre-existing issue, but it is on the other install path, the one
through ice_vsi_resize_stat_arrays(). When the estimate is smaller than the
current length, ice_vsi_free_unused_stat_arrays() frees the entries and
clears only the array slot:

	for (int i = new_txq; i < prev_txq; i++) {
		if (vsi_stat->tx_ring_stats[i]) {
			kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
			WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
		}
	}

vsi->tx_rings[i]->ring_stats and vsi->rx_rings[i]->ring_stats keep pointing
at those objects until ice_vsi_decfg()->ice_vsi_clear_rings() runs. In
ice_vsi_rebuild(), that happens after ice_vsi_resize_stat_arrays(). Doesn't
this mean the objects are handed to kfree_rcu() while they are still
reachable?

Suppose an RCU reader starts after the kfree_rcu() and before
ice_vsi_clear_rings(). Examples are __ice_get_ethtool_stats():

		tx_ring = READ_ONCE(vsi->tx_rings[j]);
		if (!tx_ring || !tx_ring->ring_stats) {

and ice_update_vsi_ring_stats(). Could such a reader see a freed
ice_ring_stats?

In practice these readers look gated:

- by rtnl, for ethtool and for the ethtool -L and mqprio callers of
  ice_vsi_rebuild()
- by netif_device_detach() in ice_prepare_for_reset()
- by the ICE_VSI_DOWN checks in ice_get_stats64() and
  ice_update_vsi_stats()

So only a reader that got past those gates before the rebuild started, and
then stalled across it, could hit the freed entry.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 1/6] ice: skip stats handling for channel VSIs on rebuild

[Jacob Keller]

On 9/25/2026 6:15 AM, Przemek Kitszel wrote:
> I'm leaning towards removing our limited (compared to OOT ADQ) support,
> but it's outside of this series.
+1. I support removing it, but it is out of scope for this kind of series.

Re: [PATCH net v5 1/6] ice: skip stats handling for channel VSIs on rebuild

[Przemek Kitszel]

Summary: Ignore.

> [Severity: Medium]
> 
> This guard only checks the VSI type. 

yes, this is a fix for a scenario reachable w/o memory failures
added with hope that bots will complain less on other patches

> Can pf->vsi_stats[vsi->idx] also be
> NULL here for a PF, VF or other non-channel VSI?
 > [...]

TLDR: preexising issue for low memory conditions

> 
> A later patch in this series, "ice: rebuild ring stats arrays instead of
> reallocating them in place", appears to fix this

Then please simply ignore the comment now.
Anyway, patch 1 has fixes tag for newer commit than the last.

side comment:
This was (a screenful...) report to suggest minor change in a very small
patch, normally I would do that, but with our current "new world" it
would simply mean delaying me by 3+ days.

 > Sashiko AI review · 
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 2/6] ice: extract __ice_vsi_free_stats()

[Przemek Kitszel]

On 9/29/26 4:04 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.

Summary: Ignore.

> 
> Low priority / nit picks for your consideration:
> 
> - [Low] The commit message understates what the change does and the patch
>    has no Fixes: tag.

does not matter, it will be pulled as dependency by @stable maintainers

BTW, I have no idea if we should add Fixes tag for "smaller half" of the
fix if it belongs in the same series (and does something a tad better
than before).

If that would be a preference, the fixes tag is the same as for last
patch (with 3 more patches not carrying that exact tag)

> 
> Pre-existing issues:
> - [Critical] Heap out-of-bounds write in ice_vsi_alloc_ring_stats()
>    (drivers/net/ethernet/intel/ice/ice_lib.c).
> - [Medium] The shrink loops in ice_vsi_realloc_stat_arrays()
>    (drivers/net/ethernet/intel/ice/ice_lib.c) still use…
> 

pre-exisiting: for another time, and must compete with other driver
hardening ideas in our backlog

critical one has even a note:
The last patch in the series, "ice: size ring stats arrays from the
final queue count", fixes this

 > Sashiko AI review · 
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 5/6] ice: rebuild ring stats arrays instead of reallocating them in place

[Przemek Kitszel]

On 9/29/26 4:04 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.

Summary: Ignore, please merge.

> 
> Critical: 1 · High: 0 (1) · Medium: 1 (1) · Low: 1
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Critical] ice_vsi_resize_stat_arrays() sizes the arrays from a
>    snapshot taken before ice_vsi_decfg(): the PF pool,…

AI says that weird stuff, like CPU hotplug, must happen to trigger
a problem, then, that the last patch fixes the problem.
I will simply ignore (driver is not ready for such stuff in general).

> - [Medium] ice_vsi_set_stat_arrays() publishes the new container with a
>    plain store (`pf->vsi_stats[vsi->idx] = new_vsi_stat;`, ice_lib.c:592)…

In the body of the comment AI says that the ETF-vs-reset issue is
pre-existing, reading more, the old code was doining the same level of
changes prior to the series/patch, but due to "being new code" its not
marked. Also, issue "seems plausible", but all works, perhaps due to
other locks.

I would like to leave it out of the series.

> - [Low] This patch fixes a real heap out-of-bounds access but has no
>    Fixes: tag, and the Fixes-tagged fix later in the series cannot be…

same comment as in patch 2, I'm against Fixes tags for "first half" of
the fix, when for the same target commit and in the same series.

> 
> Pre-existing issues:

I simply want to merge code instead of endless growth of fixed area.

> - [High] This problem wasn't introduced by this patch, but a fallback
>    rebuild dereferences NULL queue maps.
> - [Medium] ice_vsi_free_unused_stat_arrays() calls kfree_rcu() on surplus
>    ice_ring_stats entries while vsi->tx_rings[i]->ring_stats and…
> 

 > Sashiko AI review · 
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 6/6] ice: size ring stats arrays from the final queue count

[Przemek Kitszel]

On 9/29/26 4:04 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 (2) · Low: 0

Summary: Please merge. One not-pre-existing issue to dismiss.

> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The commit message says ice_vsi_alloc_stat_arrays() becomes
>    "the only authority" on the size of the ring stats arrays.

the commit message and code comment describe intuition/logic/reasoning
for the source/methodology of the size calc

AI tells that, as computed multiple times, it could be different by
principle, but only really weird cases (which were most likely not
working before). So, keeping in mind that this was meant to fix issues
observable via normal usage, fixing "but when you hotplug a CPU during
rebuild" cases is simply out of scope.

Also points out that we loose some stats counters when shrinking, then
re-growing queues - this is pre-exsiting behavior.

> 
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but
>    ice_vsi_rebuild() calls ice_vsi_resize_stat_arrays() before…
> - [Medium] This problem wasn't introduced by this patch, but in
>    ice_vsi_cfg_def(), if ice_vsi_alloc_ring_stats() fails after…
> 
> --
> 
> ice: size ring stats arrays from the final queue count
> 
> This patch changes ice_vsi_alloc_stat_arrays() so it no longer returns early
> whenever a stats container already exists. It now checks the existing ring
> stats arrays against the final vsi->alloc_txq / vsi->alloc_rxq, and grows
> them with ice_vsi_install_stat_arrays() when they are too short. This fixes
> out-of-bounds accesses in ice_vsi_alloc_ring_stats() that are reachable
> through devlink reinit after CPU hotplug and through VF queue count requests.
> 
> Closes: https://redhat.atlassian.net/browse/RHEL-164321
> 
>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
>> index 448d3c7780adf..c393d913d0379 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
>> @@ -634,10 +634,15 @@ static int ice_vsi_install_stat_arrays(struct ice_vsi *vsi, u16 txq, u16 rxq)
>>   /**
>>    * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
>>    * @vsi: VSI pointer
>> + *
>> + * Runs after ice_vsi_set_num_qs(), so this is the first point where the queue
>> + * count is final. Grow the arrays if an earlier sizing guessed too low.
>> + *
>> + * Return: 0 on success, negative error code otherwise.
>>    */
> 
> [ ... ]
> 
>> @@ -645,16 +650,13 @@ static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
>>   	if (!pf->vsi_stats)
>>   		return -ENOENT;
>>   
>> -	if (pf->vsi_stats[vsi->idx])
>> -	/* realloc will happen in rebuild path */
>> +	old_stat = pf->vsi_stats[vsi->idx];
>> +	if (old_stat && old_stat->tx_ring_stats_len >= vsi->alloc_txq &&
>> +	    old_stat->rx_ring_stats_len >= vsi->alloc_rxq)
>>   		return 0;
> 
> [Severity: Medium]

[...]

> 
> This needs netif_get_num_default_rss_queues() to change between the two
> sizing points, for example CPU hotplug during a rebuild. The sriov_numvfs
> race described in the commit message only makes the resize grow too little,
> because held_txq already covers the VSI's own queues. The out-of-bounds
> access itself does look fixed.

this is the only not pre-existing thing that I'm arguing to ignore

 > Sashiko AI review · 
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925132636.123300-1-przemyslaw.kitszel%40intel.com

Re: [PATCH net v5 0/6] ice: fix stats array overflow via proper realloc

[Przemek Kitszel]

On 9/25/26 3:15 PM, Przemek Kitszel wrote:
> Fix OOB access to the stats arrays.
> 
> The first commit (new in v5) fixes stats code against VSI_CHNL case (ADQ).
> 
> Next three commits are simple refactors to make the rest smaller,
> the fifth one untangles the logic/lifetime of the stats array entries,
> then we have the final fix for OOB access to the stats array (combined
> for both PF and VF VSIs.
> 
> v5 handles now also the PF VSI stats resizes better, fixing devlink reload
> issue reported in v4 by Clashiko.
> 
> v0
> https://lore.kernel.org/netdev/20260520183501.3360810-3-anthony.l.nguyen@intel.com
> v2
> https://sashiko.dev/#/message/20260812204619.32253-3-przemyslaw.kitszel%40intel.com
> v4 (a resend of ~the same v3)
> https://lore.kernel.org/netdev/20260921182106.1015019-1-anthony.l.nguyen@intel.com
> 
> Most notable changes in v5 already noted in the cover letter, individual patches
> have much more notes.
> ---
> 
> 
> Przemek Kitszel (6):
>    ice: skip stats handling for channel VSIs on rebuild
>    ice: extract __ice_vsi_free_stats()
>    ice: extract ice_vsi_new_stat_arrays()
>    ice: extract ice_vsi_get_num_qs()
>    ice: rebuild ring stats arrays instead of reallocating them in place
>    ice: size ring stats arrays from the final queue count
> 
>   drivers/net/ethernet/intel/ice/ice.h     |   8 +-
>   drivers/net/ethernet/intel/ice/ice_lib.c | 343 ++++++++++++++---------
>   2 files changed, 210 insertions(+), 141 deletions(-)
> 

I've replied to all not pre-existing issues. My ask is to merge as-is.

Re: [PATCH net v5 0/6] ice: fix stats array overflow via proper realloc

[Jakub Kicinski]

On Wed, 30 Sep 2026 13:55:51 +0200 Przemek Kitszel wrote:
> I've replied to all not pre-existing issues. My ask is to merge as-is.

Ack, justifications seem reasonable enough.
FWIW I'll take this via net-next because we promised Linus a smaller PR,
and the errors here don't seem likely to hit in normal use.

Re: [PATCH net v5 0/6] ice: fix stats array overflow via proper realloc

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 25 Sep 2026 15:15:42 +0200 you wrote:
> Fix OOB access to the stats arrays.
> 
> The first commit (new in v5) fixes stats code against VSI_CHNL case (ADQ).
> 
> Next three commits are simple refactors to make the rest smaller,
> the fifth one untangles the logic/lifetime of the stats array entries,
> then we have the final fix for OOB access to the stats array (combined
> for both PF and VF VSIs.
> 
> [...]

Here is the summary with links:
  - [net,v5,1/6] ice: skip stats handling for channel VSIs on rebuild
    https://git.kernel.org/netdev/net-next/c/3fca849c9653
  - [net,v5,2/6] ice: extract __ice_vsi_free_stats()
    (no matching commit)
  - [net,v5,3/6] ice: extract ice_vsi_new_stat_arrays()
    https://git.kernel.org/netdev/net-next/c/ed3cbc4430c2
  - [net,v5,4/6] ice: extract ice_vsi_get_num_qs()
    https://git.kernel.org/netdev/net-next/c/4425fa055c0d
  - [net,v5,5/6] ice: rebuild ring stats arrays instead of reallocating them in place
    https://git.kernel.org/netdev/net-next/c/66444f06a9fe
  - [net,v5,6/6] ice: size ring stats arrays from the final queue count
    https://git.kernel.org/netdev/net-next/c/f1c3b87896e6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html