Re: [PATCH net] net/mlx5: FW tracer, clamp firmware-reported num_string_db

[Leon Romanovsky]

On Fri, Jul 17, 2026 at 10:25:43AM +0300, Tariq Toukan wrote:
> From: Akiva Goldberger <agoldberger@xxxxxxxxxx>
> 
> mlx5_query_mtrc_caps() reads num_string_db from the MTRC capabilities
> register and uses it directly as a loop bound to populate the fixed-size
> base_address_out[STRINGS_DB_SECTIONS_NUM] and
> size_out[STRINGS_DB_SECTIONS_NUM] arrays in the tracer's str_db
> structure (STRINGS_DB_SECTIONS_NUM == 8).
> 
> The field is 4 bits wide, so firmware can report up to 15. A value
> greater than STRINGS_DB_SECTIONS_NUM makes the loop write past the end
> of those arrays, corrupting adjacent fields of the fw_tracer structure
> on the kernel heap. Clamp the firmware-reported value before it is used.
> 
> Fixes: f53aaa31cce7 ("net/mlx5: FW tracer, implement tracer logic")
> Signed-off-by: Akiva Goldberger <agoldberger@xxxxxxxxxx>
> Reviewed-by: Shay Drori <shayd@xxxxxxxxxx>
> Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
> index adcc73e2a5b3..404736c46adf 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
> @@ -64,6 +64,13 @@ static int mlx5_query_mtrc_caps(struct mlx5_fw_tracer *tracer)
>  	tracer->str_db.num_string_trace =
>  			MLX5_GET(mtrc_cap, out, num_string_trace);
>  	tracer->str_db.num_string_db = MLX5_GET(mtrc_cap, out, num_string_db);
> +	if (tracer->str_db.num_string_db > STRINGS_DB_SECTIONS_NUM) {
> +		mlx5_core_warn(dev,
> +			       "FWTracer: Firmware reports num_string_db (%u) > (%u), clamping\n",
> +			       tracer->str_db.num_string_db,
> +			       STRINGS_DB_SECTIONS_NUM);
> +		tracer->str_db.num_string_db = STRINGS_DB_SECTIONS_NUM;
> +	}

First, these lines are:

"tracer->str_db.num_string_db = min(tracer->str_db.num_string_db,
				    STRINGS_DB_SECTIONS_NUM);"

Second, this is a very naive approach to "securing" the system.
Everything originates from the firmware: registers, DMA, and data.
You cannot single out one field and claim the system is now "secure".

I am aware of another large vendor that added similar "clamping"
throughout their driver. That does not make the implementation correct
or particularly useful.

Thanks

>  	tracer->owner = !!MLX5_GET(mtrc_cap, out, trace_owner);
>  	tracer->str_db.loaded = false;
>  
> 
> base-commit: 3f1f755366687d051174739fb99f7d560202f60b
> -- 
> 2.44.0
> 
>