[PATCH net v4] net/mlx5e: macsec: fix use-after-free of metadata_dst on RX SC delete
From: Doruk Tan Ozturk <hidden>
Date: 2026-06-27 22:31:03
Also in:
linux-rdma, lkml, stable
Subsystem:
mellanox ethernet driver (mlx5e), mellanox ethernet innova drivers, mellanox mlx5 core vpi driver, networking drivers, the rest · Maintainers:
Saeed Mahameed, Tariq Toukan, Mark Bloch, Leon Romanovsky, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
When an offloaded MACsec RX SC is deleted, macsec_del_rxsc_ctx() freed the per-SC metadata_dst with metadata_dst_free(), which kfree()s the object unconditionally and ignores the dst reference count. The RX datapath in mlx5e_macsec_offload_handle_rx_skb() looks up the SC under rcu_read_lock() via xa_load(), takes a reference with dst_hold() and attaches the dst to the skb with skb_dst_set(). A reader that already obtained the rx_sc pointer can race with the delete path and operate on freed memory. Fix the owner side by dropping the reference with dst_release() instead of freeing unconditionally, and convert the RX datapath to dst_hold_safe() so a reader racing the SC delete cannot attach a dst whose last reference was just dropped; only attach it when a reference was actually taken. mlx5e_macsec_add_rxsc() also published sc_xarray_element via xa_alloc() before rx_sc->md_dst was allocated and initialised, so a datapath reader that looked the SC up by fs_id could observe rx_sc with md_dst still NULL or, on weakly-ordered architectures, a non-NULL md_dst pointer whose contents were not yet visible. NULL-check the xa_load() result and md_dst on the datapath, and reorder add_rxsc() so the xa_alloc() publish happens only after md_dst is fully initialised; the xarray RCU publish then pairs with the rcu_read_lock()/xa_load() in the datapath. Note: macsec_del_rxsc_ctx() also kfree()s rx_sc->sc_xarray_element without an RCU grace period while the same datapath reads it under rcu_read_lock(); that is a separate pre-existing issue left to a follow-up patch. Found by 0sec automated security-research tooling (https://0sec.ai). Fixes: b7c9400cbc48 ("net/mlx5e: Implement MACsec Rx data path using MACsec skb_metadata_dst") Cc: stable@vger.kernel.org Signed-off-by: Doruk Tan Ozturk <redacted> --- v4: - Reorder mlx5e_macsec_add_rxsc() so xa_alloc() publishes the SC only after rx_sc->md_dst is allocated and initialised; a datapath reader could otherwise observe a non-NULL md_dst with uninitialised contents (raised by the automated review forwarded by Simon Horman). Error paths adjusted (no xa_erase before the publish). v3: NULL-check the xa_load() result and rx_sc->md_dst on the datapath. v2: convert the datapath dst_hold() to dst_hold_safe(). v1: https://lore.kernel.org/netdev/20260615140534.52691-1-doruk@0sec.ai/ (local) .../mellanox/mlx5/core/en_accel/macsec.c | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 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 71b3a059c..daff53ba7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c@@ -714,34 +714,43 @@ static int mlx5e_macsec_add_rxsc(struct macsec_context *ctx) } sc_xarray_element->rx_sc = rx_sc; - err = xa_alloc(&macsec->sc_xarray, &sc_xarray_element->fs_id, sc_xarray_element, - XA_LIMIT(1, MLX5_MACEC_RX_FS_ID_MAX), GFP_KERNEL); - if (err) { - if (err == -EBUSY) - netdev_err(ctx->netdev, - "MACsec offload: unable to create entry for RX SC (%d Rx SCs already allocated)\n", - MLX5_MACEC_RX_FS_ID_MAX); - goto destroy_sc_xarray_elemenet; - } rx_sc->md_dst = metadata_dst_alloc(0, METADATA_MACSEC, GFP_KERNEL); if (!rx_sc->md_dst) { err = -ENOMEM; - goto erase_xa_alloc; + goto destroy_sc_xarray_elemenet; } rx_sc->sci = ctx_rx_sc->sci; rx_sc->active = ctx_rx_sc->active; - list_add_rcu(&rx_sc->rx_sc_list_element, rx_sc_list); - rx_sc->sc_xarray_element = sc_xarray_element; rx_sc->md_dst->u.macsec_info.sci = rx_sc->sci; + + /* + * Publish the fully-initialised SC last: xa_alloc() makes + * sc_xarray_element->rx_sc (and rx_sc->md_dst) reachable from the RX + * datapath via xa_load(). Doing it only after md_dst is allocated and + * initialised pairs with the rcu_read_lock()/xa_load() in + * mlx5e_macsec_offload_handle_rx_skb(), so a reader can never observe + * a non-NULL md_dst with uninitialised contents. + */ + err = xa_alloc(&macsec->sc_xarray, &sc_xarray_element->fs_id, sc_xarray_element, + XA_LIMIT(1, MLX5_MACEC_RX_FS_ID_MAX), GFP_KERNEL); + if (err) { + if (err == -EBUSY) + netdev_err(ctx->netdev, + "MACsec offload: unable to create entry for RX SC (%d Rx SCs already allocated)\n", + MLX5_MACEC_RX_FS_ID_MAX); + goto destroy_md_dst; + } + + list_add_rcu(&rx_sc->rx_sc_list_element, rx_sc_list); mutex_unlock(&macsec->lock); return 0; -erase_xa_alloc: - xa_erase(&macsec->sc_xarray, sc_xarray_element->fs_id); +destroy_md_dst: + dst_release(&rx_sc->md_dst->dst); destroy_sc_xarray_elemenet: kfree(sc_xarray_element); destroy_rx_sc:
@@ -829,7 +838,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); - metadata_dst_free(rx_sc->md_dst); + dst_release(&rx_sc->md_dst->dst); kfree(rx_sc->sc_xarray_element); kfree_rcu_mightsleep(rx_sc); }
@@ -1695,10 +1704,10 @@ void mlx5e_macsec_offload_handle_rx_skb(struct net_device *netdev, rcu_read_lock(); sc_xarray_element = xa_load(&macsec->sc_xarray, fs_id); - rx_sc = sc_xarray_element->rx_sc; - if (rx_sc) { - dst_hold(&rx_sc->md_dst->dst); - skb_dst_set(skb, &rx_sc->md_dst->dst); + rx_sc = sc_xarray_element ? sc_xarray_element->rx_sc : NULL; + if (rx_sc && rx_sc->md_dst) { + if (dst_hold_safe(&rx_sc->md_dst->dst)) + skb_dst_set(skb, &rx_sc->md_dst->dst); } rcu_read_unlock();
--
2.43.0