Re: [PATCH net v2] net: mlx5e: Fix use-after-free of the MACsec xarray entry on RX SC deletion

[Tariq Toukan <tariqt@nvidia.com>]

On 10/10/2026 6:21, Binbin Deng wrote:
> In macsec_del_rxsc_ctx(), the RX secure channel is torn down with:
> 
> 	list_del_rcu(&rx_sc->rx_sc_list_element);
> 	xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
> 	dst_release(&rx_sc->md_dst->dst);
> 	kfree(rx_sc->sc_xarray_element);
> 	kfree_rcu_mightsleep(rx_sc);
> 
> The comment above the sequence claims that "xa_erase which uses rcu to
> sync" hides the RX SC from the data path, but this is not sufficient:
> the RCU protection of xarray covers only its internal nodes (making
> xa_load() safe against concurrent erase); it does not extend to the
> object pointed to by the entry value.  A reader that has already
> obtained the pointer from xa_load() is not covered by any grace period,
> and the bare kfree() releases the entry object immediately.
> 
> The reader is mlx5e_macsec_offload_handle_rx_skb() in the NAPI RX
> path, which runs under rcu_read_lock() only:
> 
> 	sc_xarray_element = xa_load(&macsec->sc_xarray, fs_id);
> 	rx_sc = sc_xarray_element ? sc_xarray_element->rx_sc : NULL;
> 
> An RCU read-side critical section protects only against deferred frees
> (call_rcu()/kfree_rcu()); it provides no protection against the
> immediate kfree() above.  If the NAPI reader loads the entry pointer
> and is then delayed while another CPU executes the deletion path, the
> subsequent read of sc_xarray_element->rx_sc dereferences freed memory.
> 
> The path is reachable with CAP_NET_ADMIN while an RX SC has in-flight
> traffic: CQEs carrying the fs_id metadata can still arrive after the
> offload rule has been removed, so deleting an RX SC concurrently with
> reception can race the reader.
> 
> Note that the same function already releases rx_sc itself via
> kfree_rcu_mightsleep(): the author was aware that this teardown needs a
> deferred free, but the intermediate entry object was missed.
> 
> Fix this by adding an rcu_head to the entry object and releasing it
> with kfree_rcu().
> 
> Found during a static-analysis-assisted audit: candidate sites were
> surfaced by a Coccinelle rule, and the reader/writer paths were
> confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
> not observed at runtime; it is identified by code inspection only.
> 
> Fixes: b7c9400cbc48 ("net/mlx5e: Implement MACsec Rx data path using MACsec skb_metadata_dst")
> Signed-off-by: Binbin Deng <18983559317@163.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> index daff53ba7d09..d4caa7affae6 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> @@ -75,6 +75,7 @@ struct mlx5e_macsec_rx_sc;
>   struct mlx5e_macsec_rx_sc_xarray_element {
>   	u32 fs_id;
>   	struct mlx5e_macsec_rx_sc *rx_sc;
> +	struct rcu_head rcu_head;
>   };
>   
>   struct mlx5e_macsec_rx_sc {
> @@ -839,7 +840,7 @@ static void macsec_del_rxsc_ctx(struct mlx5e_macsec *macsec, struct mlx5e_macsec
>   	list_del_rcu(&rx_sc->rx_sc_list_element);
>   	xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
>   	dst_release(&rx_sc->md_dst->dst);
> -	kfree(rx_sc->sc_xarray_element);
> +	kfree_rcu(rx_sc->sc_xarray_element, rcu_head);
>   	kfree_rcu_mightsleep(rx_sc);
>   }
>   

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Thanks.