Re: [PATCH net 1/2] mlxsw: spectrum_flower: Fix port range register leak in tmplt_create()

[Jacob Keller <jacob.e.keller@intel.com>]

On 10/6/2026 7:21 AM, Petr Machata wrote:
> mlxsw_sp_flower_tmplt_create() parses a flow_cls_offload template
> into a stack-local struct mlxsw_sp_acl_rule_info purely to compute
> rulei.values.elusage. Parsing can acquire port range registers via
> mlxsw_sp_flower_parse_ports_range(), but since this rulei never goes
> through mlxsw_sp_acl_rulei_destroy(), those registers were never
> released, including through chain template deletion.
> 
> Factor out of mlxsw_sp_acl_rulei_destroy() the code to actually
> release the necessary resources and call from
> mlxsw_sp_flower_tmplt_create() to plug the leak.
> 

Since mlxsw_sp_acl_rulei_destroy() now just calls free + a kfree, it
would also be reasonable to just convert to heap allocated structure
instead of keeping it on the stack since you have to call a function
either way.

No strong objection to this implementation though, and it leaves more of
the other code unchanged. The stack use has already been accepted anyways.

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

> The issue was found during a review of Wentao Liang's patch referenced
> below.
> 
> Fixes: fe22f7410527 ("mlxsw: spectrum_flower: Add ability to match on port ranges")
> Reported-by: Wentao Liang <vulab@iscas.ac.cn>
> Closes: https://lore.kernel.org/netdev/20260917113236.2149095-1-vulab@iscas.ac.cn/
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h        |  2 ++
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c    | 10 ++++++++--
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c |  9 +++++++--
>  3 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index b03ff9e044f9..10af08ce72ef 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -989,6 +989,8 @@ void mlxsw_sp_acl_ruleset_prio_get(struct mlxsw_sp_acl_ruleset *ruleset,
>  struct mlxsw_sp_acl_rule_info *
>  mlxsw_sp_acl_rulei_create(struct mlxsw_sp_acl *acl,
>  			  struct mlxsw_afa_block *afa_block);
> +void mlxsw_sp_acl_rulei_free(struct mlxsw_sp *mlxsw_sp,
> +			     const struct mlxsw_sp_acl_rule_info *rulei);
>  void mlxsw_sp_acl_rulei_destroy(struct mlxsw_sp *mlxsw_sp,
>  				struct mlxsw_sp_acl_rule_info *rulei);
>  int mlxsw_sp_acl_rulei_commit(struct mlxsw_sp_acl_rule_info *rulei);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
> index cb232accb296..f7be4f3a1518 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
> @@ -340,8 +340,8 @@ mlxsw_sp_acl_rulei_create(struct mlxsw_sp_acl *acl,
>  	return ERR_PTR(err);
>  }
>  
> -void mlxsw_sp_acl_rulei_destroy(struct mlxsw_sp *mlxsw_sp,
> -				struct mlxsw_sp_acl_rule_info *rulei)
> +void mlxsw_sp_acl_rulei_free(struct mlxsw_sp *mlxsw_sp,
> +			     const struct mlxsw_sp_acl_rule_info *rulei)
>  {
>  	if (rulei->action_created)
>  		mlxsw_afa_block_destroy(rulei->act_block);
> @@ -351,6 +351,12 @@ void mlxsw_sp_acl_rulei_destroy(struct mlxsw_sp *mlxsw_sp,
>  	if (rulei->dst_port_range_reg_valid)
>  		mlxsw_sp_port_range_reg_put(mlxsw_sp,
>  					    rulei->dst_port_range_reg_index);
> +}
> +
> +void mlxsw_sp_acl_rulei_destroy(struct mlxsw_sp *mlxsw_sp,
> +				struct mlxsw_sp_acl_rule_info *rulei)
> +{
> +	mlxsw_sp_acl_rulei_free(mlxsw_sp, rulei);
>  	kfree(rulei);
>  }
>  
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
> index 353fd9ca89a6..5e4632cb792d 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
> @@ -862,14 +862,19 @@ int mlxsw_sp_flower_tmplt_create(struct mlxsw_sp *mlxsw_sp,
>  	memset(&rulei, 0, sizeof(rulei));
>  	err = mlxsw_sp_flower_parse(mlxsw_sp, block, &rulei, f);
>  	if (err)
> -		return err;
> +		goto out;
> +
>  	ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
>  					   f->common.chain_index,
>  					   MLXSW_SP_ACL_PROFILE_FLOWER,
>  					   &rulei.values.elusage);
> +	err = PTR_ERR_OR_ZERO(ruleset);
> +
> +out:
> +	mlxsw_sp_acl_rulei_free(mlxsw_sp, &rulei);
>  
>  	/* keep the reference to the ruleset */
> -	return PTR_ERR_OR_ZERO(ruleset);
> +	return err;
>  }
>  
>  void mlxsw_sp_flower_tmplt_destroy(struct mlxsw_sp *mlxsw_sp,