Re: [PATCH net 3/4] net/mlx5e: Bounds-check stats_nch in mlx5e_get_queue_stats_rx()

[Jacob Keller]

On 6/4/2026 6:50 AM, Tariq Toukan wrote:
> From: Feng Liu <feliu@xxxxxxxxxx>
> 
> mlx5e_get_queue_stats_rx() is invoked by the netdev stats core with
> an RX queue index 'i' from real_num_rx_queues. Today it only guards
> against priv->stats_nch == 0 and then dereferences
> priv->channel_stats[i] unconditionally.
> 
> During interface bring-up channel_stats[] is populated incrementally
> by mlx5e_channel_stats_alloc(), so a concurrent QSTATS netlink dump
> can call into the helper with i >= stats_nch. The non-zero check
> passes, channel_stats[i] is NULL, and the dereference panics.
> 
> Replace the non-zero check with an upper-bound check against
> stats_nch, which subsumes the zero check and prevents the
> out-of-bounds dereference.
> 
> Fixes: 7b66ae536a78 ("net/mlx5e: Add per queue netdev-genl stats")
> Signed-off-by: Feng Liu <feliu@xxxxxxxxxx>
> Reviewed-by: Eran Ben Elisha <eranbe@xxxxxxxxxx>
> Reviewed-by: Cosmin Ratiu <cratiu@xxxxxxxxxx>
> Reviewed-by: Nimrod Oren <noren@xxxxxxxxxx>
> Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 8f2b3abe0092..42a658402592 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -5489,7 +5489,7 @@ static void mlx5e_get_queue_stats_rx(struct net_device *dev, int i,
>  	struct mlx5e_rq_stats *xskrq_stats;
>  	struct mlx5e_rq_stats *rq_stats;
>  
> -	if (mlx5e_is_uplink_rep(priv) || !priv->stats_nch)
> +	if (mlx5e_is_uplink_rep(priv) || i >= priv->stats_nch)

i is a signed integer, but from the way its used its clear that it
shouldn't be negative (indeed, direct indexing with a negative in
channel_stats would be strange..). The API could be clarified to use an
unsigned int, but in the assumption it always ranges from [0, MAX] then
this check makes sense. If stats_nch is zero, the check is trivially
true, and otherwise this now also prevents access to stats which weren't
yet initialized. Ok.

Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>

>  		return;
>  
>  	channel_stats = priv->channel_stats[i];