Re: [PATCH net V3 1/3] net/mlx5e: Fix HV VHCA stats zero-sized buffer allocation

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev

Simon says: I think this issue can be addressed by reversing the order
that patches 1/3 and 2/3 appear in this patch-set.

---
net/mlx5e: Fix HV VHCA stats zero-sized buffer allocation

This commit fixes a zero-sized buffer allocation by using max_nch instead of
stats_nch when allocating the buffer for HV VHCA stats.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
> index 195863b2c013e..06cbd49d4e98e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
> @@ -122,7 +128,7 @@ static void mlx5e_hv_vhca_stats_cleanup(struct mlx5_hv_vhca_agent *agent)
>  
>  void mlx5e_hv_vhca_stats_create(struct mlx5e_priv *priv)
>  {
> -	int buf_len = mlx5e_hv_vhca_stats_buf_size(priv);
> +	int buf_len = mlx5e_hv_vhca_stats_buf_max_size(priv);
>  	struct mlx5_hv_vhca_agent *agent;
>  
>  	priv->stats_agent.buf = kvzalloc(buf_len, GFP_KERNEL);

[Severity: High]
Does this change introduce a potential double-free regression on memory
allocation failure?

When mlx5e_hv_vhca_stats_destroy() cleans up the stats agent, it frees the
agent but does not set priv->stats_agent.agent to NULL.

Before this patch, kvzalloc() was called with 0 and returned ZERO_SIZE_PTR,
so it never returned NULL. It always proceeded to call
mlx5_hv_vhca_agent_create() which overwrote priv->stats_agent.agent.

Now that the buffer size is non-zero, if kvzalloc() fails it will return
NULL and take the early return path in mlx5e_hv_vhca_stats_create():

	priv->stats_agent.buf = kvzalloc(buf_len, GFP_KERNEL);
	if (!priv->stats_agent.buf)
		return;

This early return bypasses overwriting the dangling priv->stats_agent.agent.
If a subsequent device detach occurs, mlx5e_hv_vhca_stats_destroy() could
dereference and free this stale pointer again.

I noticed this was later addressed upstream in commit e600849cc1e0
("net/mlx5e: Fix HV VHCA stats agent registration race").