Re: [PATCH net v2] net/mlx5: macsec: free struct mlx5_fs_id after an RCU grace period

[Cosmin Ratiu]

On Tue, 2026-10-27 at 18:17 -0300, Joas Antonio dos Santos wrote:
> macsec_fs_id_del() removes a struct mlx5_fs_id from its rhashtable
> with
> rhashtable_remove_fast() and then frees it with kfree() right away.
> 
> The TX (SCI) table is read locklessly from the transmit path:
> mlx5e_macsec_handle_tx_skb() and mlx5e_macsec_tx_build_eseg() call
> mlx5_macsec_fs_get_fs_id_from_hashtable(), which does
> rhashtable_lookup() under rcu_read_lock() and then reads
> mlx5_fs_id->id. A transmit that looked the entry up just before it
> was
> removed reads freed memory.
> 
> Before commit d4ece08f4b1d ("net/mlx5: Maintain fs_id xarray per
> MACsec
> device inside macsec steering") the SCI table held struct
> mlx5e_macsec_sa directly, and mlx5e_macsec_del_txsa() freed it with
> kfree_rcu_mightsleep(). That commit moved the table into macsec_fs.c
> with the new struct mlx5_fs_id, but frees it with a plain kfree().
> 
> Add an rcu_head to struct mlx5_fs_id and free it with kfree_rcu().
> 
> The same issue was flagged by the Sashiko AI review as a pre-existing
> problem while reviewing the HW-GRO for PSP series.
> 
> Fixes: d4ece08f4b1d ("net/mlx5: Maintain fs_id xarray per MACsec
> device inside macsec steering")
> Link:
> https://lore.kernel.org/all/20260909065542.4B80D1F00ACA@xxxxxxxxxxxxxxx/
> Signed-off-by: Joas Antonio dos Santos <joasantonio108@xxxxxxxxx>
> Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
> ---
> v2:
> - Drop the macsec.c hunk: the sc_xarray_element free was already
> fixed
>   by ad17ad50b8dc ("net: mlx5e: Fix use-after-free of the MACsec
> xarray
>   entry on RX SC deletion"). (Simon)
> - Embed an rcu_head and use kfree_rcu() instead of
>   kvfree_rcu_mightsleep(). (Simon)
> - Fixes: now points at the commit that introduced the kfree().
> - v1:
> https://lore.kernel.org/all/179285888829.90275.17386097105525068016@xxxxxxxxx/
> 
> Testing: compile-tested only (arm64, W=1, CONFIG_MLX5_MACSEC=y, no
> new
> warnings). I do not have
> MACsec-capable ConnectX hardware, so this was not run on a device.
> 
>  drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> @@ -94,6 +94,7 @@ struct mlx5_fs_id {
>  	refcount_t refcnt;
>  	sci_t sci;
>  	struct rhash_head hash;
> +	struct rcu_head rcu;
>  };
> 
>  struct mlx5_macsec_device {
> @@ -684,7 +685,7 @@ static void macsec_fs_id_del(struct list_head
> *macsec_devices_list, u32 fs_id,
> 
>  	xa_erase(fs_id_xa, fs_id);
> 
> -	kfree(fs_id_found);
> +	kfree_rcu(fs_id_found, rcu);
> 
>  	if (macsec_fs_is_macsec_device_empty(macsec_device)) {
>  		list_del(&macsec_device->macsec_devices_list_entry);
> --
> 2.39.5

Thank you for the fix, it looks good.

Reviewed-by: Cosmin Ratiu <cratiu@xxxxxxxxxx>