> -----Original Message-----
> From: Kitszel, Przemyslaw <[email protected]>
> Sent: Friday, September 25, 2026 3:16 PM
> To: [email protected]; Jakub Kicinski <[email protected]>
> Cc: Nguyen, Anthony L <[email protected]>; Loktionov,
> Aleksandr <[email protected]>; Schmidt, Michal
> <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Kitszel, Przemyslaw
> <[email protected]>
> 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 <[email protected]>
> Closes: https://redhat.atlassian.net/browse/RHEL-164321
> Signed-off-by: Przemek Kitszel <[email protected]>
> ---
> 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 <[email protected]>
