[Intel-wired-lan] [net-next, v2 2/2] ice: Accumulate ring statistics over reset

[Jacob Keller]

On 10/10/2022 12:22 PM, Benjamin Mikailenko wrote:
> Resets may occur with or without user interaction. For example, a TX hang
> or reconfiguration of parameters will result in a reset. During reset, the
> VSI is freed, freeing any statistics structures inside as well. This would
> create an issue for the user where a reset happens in the background,
> statistics set to zero, and the user checks ring statistics expecting them
> to be populated.
> 
> To ensure this doesn't happen, accumulate ring statistics over reset.
> 
> Define a new ring statistics structure, ice_ring_stats. The new structure
> lives in the VSI's parent, preserving ring statistics when VSI is freed.
> 
> 1. Define a new structure vsi_ring_stats in the PF scope
> 2. Allocate/free stats only during probe, unload, or change in ring size
> 3. Replace previous ring statistics functionality with new structure
> 
> Signed-off-by: Benjamin Mikailenko <benjamin.mikailenko at intel.com>
> ---

I found a potential use-after-free bug in ice_vsi_alloc with these changes:

>  /**
>   * ice_vsi_alloc - Allocates the next available struct VSI in the PF
>   * @pf: board private structure
> @@ -560,6 +606,11 @@ ice_vsi_alloc(struct ice_pf *pf, enum ice_vsi_type vsi_type,
>  
>  	if (vsi->type == ICE_VSI_CTRL && vf)
>  		vf->ctrl_vsi_idx = vsi->idx;
> +
> +	/* allocate memory for Tx/Rx ring stat pointers */
> +	if (ice_vsi_alloc_stat_arrays(vsi))
> +		goto err_rings;
> +
>  	goto unlock_pf;
>  

This is placed in ice_vsi_alloc after we insert the VSI into the PF
array and update pointers such as the next_vsi and ctrl_vsi_idx. Doing
so means that if this function fails we will leave stale data behind in
the PF resulting in a use-after-free when tearing down all the VSIs.

This can be avoided by moving this logic up higher to just before we
update the PF VSI array.

Thanks,
Jake