Re: [Intel-wired-lan] [PATCH net v2 2/2] ice: fix stats array overflow via proper realloc
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-08-17 20:50:53
Also in:
intel-wired-lan
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ice: fix stats array overflow via proper realloc
This patch removes ice_vsi_alloc_stat_arrays() and makes
ice_vsi_realloc_stat_arrays() handle the initial (no vsi_stat) case, tracking
the array sizes in the new tx_ring_stats_len / rx_ring_stats_len fields. The
single remaining call site is in ice_vsi_cfg_def(), and the call in
ice_vsi_rebuild() is dropped.
Closes: https://redhat.atlassian.net/browse/RHEL-164321
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index e48ee5940f17..420e72df7f5e 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2379,7 +2353,7 @@ static int ice_vsi_cfg_def(struct ice_vsi *vsi) return ret; /* allocate memory for Tx/Rx ring stat pointers */ - ret = ice_vsi_alloc_stat_arrays(vsi); + ret = ice_vsi_realloc_stat_arrays(vsi); if (ret) goto unroll_vsi_alloc;@@ -3097,10 +3071,6 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags) mutex_lock(&vsi->xdp_state_lock); - ret = ice_vsi_realloc_stat_arrays(vsi); - if (ret) - goto unlock; - ice_vsi_decfg(vsi); ret = ice_vsi_cfg_def(vsi); if (ret)
[Severity: Low]
Does moving the ice_vsi_realloc_stat_arrays() call change what happens when
the stat array allocation fails during a rebuild?
Before this patch, ice_vsi_rebuild() did the realloc before
ice_vsi_decfg(), so a krealloc_array() failure returned -ENOMEM with the VSI
still fully configured:
ret = ice_vsi_realloc_stat_arrays(vsi);
if (ret)
goto unlock;
ice_vsi_decfg(vsi);
Now the only call site is inside ice_vsi_cfg_def(), which runs after
ice_vsi_decfg(), so an -ENOMEM there unwinds via unroll_vsi_alloc and is
propagated out of ice_vsi_rebuild() with the queues, vectors and rings
already released.
For the ethtool -L path that leaves ice_vsi_recfg_qs() in ice_main.c with
only this recovery:
rebuild_err:
dev_err(ice_pf_to_dev(pf), "Error during VSI rebuild: %d. Unload and reload the driver.\n",
err);
The same outcome was already reachable pre-patch through the other
allocations in ice_vsi_cfg_def() (ice_vsi_alloc_def(),
ice_vsi_alloc_q_vectors(), ice_vsi_alloc_rings(),
ice_vsi_alloc_ring_stats()), so this adds one more allocation to an
already-destructive region rather than a new failure mode.
The commit message describes the move only as:
Also drop one callsite to avoid some duplication.
Could it also mention that the previously non-destructive failure path for
this allocation is gone?
--
pw-bot: cr