Re: [PATCH net 2/2] net/mlx5: HWS, fix modify header pool creation race

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/mlx5: 32-bit truncation of 64-bit modify actions in pool match
--

commit 214c1c5671cbf4fde727381eec5a1bbac14499d3
Author: Alexandre Cassen <acassen@corp.free.fr>

net/mlx5: HWS, fix modify header pool creation race

This commit adds a mutex to serialize modify header pool creation in
mlx5_cmd_hws_modify_header_alloc() to prevent concurrent callers from
failing with -EBUSY. It also removes the error path destruction of the newly
created pool so that other concurrent users can still access it.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c
> index 66edb42268cbd..b387016bb5aad 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c

[ ... ]

> @@ -1474,6 +1476,7 @@ static int mlx5_cmd_hws_modify_header_alloc(struct mlx5_flow_root_namespace *ns,
>  	pattern.sz = MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto) * num_actions;
>  	pattern.data = modify_actions;
>  
> +	mutex_lock(&hws_pool->mh_pools_lock);
>  	known_pattern = false;
>  	xa_for_each(&hws_pool->mh_pools, i, pool) {
>  		if (mlx5_fs_hws_mh_pool_match(pool, &pattern)) {

[Severity: High]
Does mlx5_fs_hws_mh_pool_match() incorrectly truncate 64-bit modify actions to
32 bits during comparison?

Looking at drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c:

mlx5_fs_hws_mh_pool_match() {
	...
	for (i = 0; i < num_actions; i++) {
		if ((__force __be32)pattern->data[i] !=
		    (__force __be32)pool_pattern->data[i])
			return false;
	}
	...
}

The pattern->data array elements are 64-bit PRM elements (__be64). Casting
them to __be32 compares only the lower 32 bits, completely ignoring the action
type and target fields in the upper 32 bits.

Could this cause two distinct flow modify actions that share the same lower
32 bits to evaluate as equal, resulting in them incorrectly sharing the same
modify header pool and executing the wrong actions on packets?

>  			known_pattern = true;
>  			break;
>  		}
>  		cnt++;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008204230.3222198-1-acassen@corp.free.fr?part=2