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

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

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