[PATCH net] net/mlx5e: macsec: free SC lookup entries after an RCU grace period

[PATCH net] net/mlx5e: macsec: free SC lookup entries after an RCU grace period

[Joas Antonio dos Santos]

macsec_del_rxsc_ctx() removes rx_sc->sc_xarray_element from
macsec->sc_xarray with xa_erase() and frees it with kfree() right away.
mlx5e_macsec_offload_handle_rx_skb() looks the element up with
xa_load() under rcu_read_lock() and then dereferences ->rx_sc, so an RX
completion that loaded the element just before xa_erase() reads freed
memory. rx_sc itself is already freed with kfree_rcu_mightsleep(); only
the element pointing to it was missed.

macsec_fs_id_del() has the same problem on the TX side: it removes
fs_id_found from the SCI rhashtable and frees it with kfree(), while
mlx5e_macsec_handle_tx_skb() and mlx5e_macsec_tx_build_eseg() read it
through mlx5_macsec_fs_get_fs_id_from_hashtable(), which uses
rhashtable_lookup() under rcu_read_lock().

Free both with kvfree_rcu_mightsleep(). Both run in process context
from the macsec offload ops, outside any spinlock, so sleeping is fine.

The sc_xarray_element issue was pointed out during review of
"net/mlx5e: macsec: fix use-after-free of metadata_dst on RX SC delete"
and left for a follow-up; this is that follow-up.

Fixes: b7c9400cbc48 ("net/mlx5e: Implement MACsec Rx data path using MACsec skb_metadata_dst")
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
---
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 real device and I have no reproducer against the driver
itself. The free-while-reader-holds-RCU pattern was reproduced with a
standalone test module under KASAN (slab-use-after-free on kmalloc-16),
and the same module showed no reports after switching to
kvfree_rcu_mightsleep().

 drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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 38a3415ac..fb9126f6b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
@@ -839,7 +839,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);
+	kvfree_rcu_mightsleep(rx_sc->sc_xarray_element);
 	kfree_rcu_mightsleep(rx_sc);
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
index efc516749..910fafe40 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
@@ -684,7 +684,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);
+	kvfree_rcu_mightsleep(fs_id_found);
 
 	if (macsec_fs_is_macsec_device_empty(macsec_device)) {
 		list_del(&macsec_device->macsec_devices_list_entry);

Re: [PATCH net] net/mlx5e: macsec: free SC lookup entries after an RCU grace period

[Simon Horman]

On Sat, Oct 24, 2026 at 01:21:28PM -0300, Joas Antonio dos Santos wrote:
> macsec_del_rxsc_ctx() removes rx_sc->sc_xarray_element from
> macsec->sc_xarray with xa_erase() and frees it with kfree() right away.
> mlx5e_macsec_offload_handle_rx_skb() looks the element up with
> xa_load() under rcu_read_lock() and then dereferences ->rx_sc, so an RX
> completion that loaded the element just before xa_erase() reads freed
> memory. rx_sc itself is already freed with kfree_rcu_mightsleep(); only
> the element pointing to it was missed.
> 
> macsec_fs_id_del() has the same problem on the TX side: it removes
> fs_id_found from the SCI rhashtable and frees it with kfree(), while
> mlx5e_macsec_handle_tx_skb() and mlx5e_macsec_tx_build_eseg() read it
> through mlx5_macsec_fs_get_fs_id_from_hashtable(), which uses
> rhashtable_lookup() under rcu_read_lock().
> 
> Free both with kvfree_rcu_mightsleep(). Both run in process context
> from the macsec offload ops, outside any spinlock, so sleeping is fine.
> 
> The sc_xarray_element issue was pointed out during review of
> "net/mlx5e: macsec: fix use-after-free of metadata_dst on RX SC delete"
> and left for a follow-up; this is that follow-up.
> 
> Fixes: b7c9400cbc48 ("net/mlx5e: Implement MACsec Rx data path using MACsec skb_metadata_dst")
> Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
> Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
> ---
> 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 real device and I have no reproducer against the driver
> itself. The free-while-reader-holds-RCU pattern was reproduced with a
> standalone test module under KASAN (slab-use-after-free on kmalloc-16),
> and the same module showed no reports after switching to
> kvfree_rcu_mightsleep().
> 
>  drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 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 38a3415ac..fb9126f6b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> @@ -839,7 +839,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);
> +	kvfree_rcu_mightsleep(rx_sc->sc_xarray_element);
>  	kfree_rcu_mightsleep(rx_sc);
>  }
>  

Hi Joas,

This patch does not apply because the problem in macsec_del_rxsc_ctx() has
already been addressed by ad17ad50b8dc ("net: mlx5e: Fix use-after-free of
the MACsec xarray entry on RX SC deletion") using kfree_rcu.

I would suggest taking that approach in macsec_fs_id_del() too
unless there is a reason to use mightsleep.

Quoting the documentation of kfree_rcu_mightsleep():

/**
 * kfree_rcu_mightsleep() - kfree an object after a grace period.
 * @ptr: pointer to kfree for single-argument invocations.
 *
 * When it comes to head-less variant, only one argument
 * is passed and that is just a pointer which has to be
 * freed after a grace period. Therefore the semantic is
 *
 *     kfree_rcu_mightsleep(ptr);
 *
 * where @ptr is the pointer to be freed by kvfree().
 *
 * Please note, head-less way of freeing is permitted to
 * use from a context that has to follow might_sleep()
 * annotation. Otherwise, please switch and embed the
 * rcu_head structure within the type of @ptr.
 */

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> index efc516749..910fafe40 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> @@ -684,7 +684,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);
> +	kvfree_rcu_mightsleep(fs_id_found);
>  
>  	if (macsec_fs_is_macsec_device_empty(macsec_device)) {
>  		list_del(&macsec_device->macsec_devices_list_entry);

-- 
pw-bot: changes-requested

Re: [PATCH net] net/mlx5e: macsec: free SC lookup entries after an RCU grace period

[Joas Antonio dos Santos]

On Tue, Oct 27, 2026 at 02:26:01PM +0000, Simon Horman wrote:
> This patch does not apply because the problem in macsec_del_rxsc_ctx() has
> already been addressed by ad17ad50b8dc ("net: mlx5e: Fix use-after-free of
> the MACsec xarray entry on RX SC deletion") using kfree_rcu.
>
> I would suggest taking that approach in macsec_fs_id_del() too
> unless there is a reason to use mightsleep.

Thanks Simon, I missed that ad17ad50b8dc had already landed. There is no
reason to use the mightsleep variant, so v2 drops the macsec.c hunk and
embeds an rcu_head in struct mlx5_fs_id so it can be freed with
kfree_rcu():

https://lore.kernel.org/all/179313582625.87630.11382992904624255106@gmail.com/

Joas