[PATCH net V2] net/mlx5e: rep, Fix races between devcom PAIR event and uplink close

[Tariq Toukan <tariqt@nvidia.com>]

From: Shay Drory <shayd@nvidia.com>

mlx5e_sqs2vport_stop() frees each rep_sq and its per-peer send-to-vport
rules (rep_sq->sq_peer) while holding only the uplink's priv->state_lock
(it runs from the ndo_stop -> mlx5e_deactivate_priv_channels ->
mlx5e_rep_deactivate_channels path).

The same rep_sq->sq_peer entries are inserted and erased by the devcom
ESW_OFFLOADS PAIR/UNPAIR handler (mlx5e_vport_rep_event_pair/unpair),
which runs under the devcom component write semaphore and takes no
state_lock. The two lock sets are disjoint, so closing the uplink of one
eswitch can run concurrently with a pairing/unpairing event driven by
the peer eswitch, both mutating the same rep's vport_sqs_list / sq_peer.
This is a use-after-free: one side frees rep_sq / sq_peer while the
other walks the list or inserts/erases the xarray.

The add side (mlx5e_sqs2vport_start) is not safe either. It takes the
devcom read semaphore only via mlx5_devcom_for_each_peer_begin(), which
is gated on comp->ready. The PAIR handler does all of its rep work
before setting ready, so a first-time PAIR runs with ready == false and
start can list_add() into vport_sqs_list while the handler walks it.

Add mlx5_devcom_comp_read_lock/unlock(), which take the component
semaphore for read regardless of ready, and hold it across both
mlx5e_sqs2vport_start() and mlx5e_sqs2vport_stop(). Start still adds
peer rules only when the component is ready, now checked under the
lock.

Lock ordering is preserved: start already took state_lock -> comp->sem
(read); stop now does the same. The event handler takes comp->sem and
never state_lock, so there is no inversion.

Fixes: 898b07861565 ("net/mlx5: Add send to vport rules on paired device")
Signed-off-by: Shay Drory <shayd@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  | 21 ++++++++++---------
 .../ethernet/mellanox/mlx5/core/lib/devcom.c  | 14 +++++++++++++
 .../ethernet/mellanox/mlx5/core/lib/devcom.h  |  2 ++
 3 files changed, 27 insertions(+), 10 deletions(-)

V2:
- Add mlx5_devcom_comp_read_lock/unlock() and use them instead of the
  ready-gated mlx5_devcom_for_each_peer_begin/end(), so the PAIR handler
  is excluded even while comp->ready is false.
- Apply the same locking to mlx5e_sqs2vport_start(), whose list_add()
  could race a first-time PAIR.
- Read esw->devcom once so lock and unlock use the same pointer.
- Update the Fixes tag to 898b07861565.

answer to Sashiko comment:
The esw->devcom lifetime race with mlx5_esw_offloads_devcom_cleanup() is
pre-existing and not specific to the close path: mlx5e_sqs2vport_start()
and other users (en_tc, esw bridge, shared_fdb) dereference esw->devcom
the same way without serializing against devcom cleanup. It needs its own
fix and is out of scope for this patch.

V1:
https://lore.kernel.org/all/20261015085744.1109107-1-tariqt@nvidia.com/

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index a50a42581528..962c70b9b566 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -506,10 +506,11 @@ static int mlx5e_sqs2vport_start(struct mlx5_eswitch *esw,
 				 struct mlx5_eswitch_rep *rep,
 				 u32 *sqns_array, int sqns_num)
 {
+	struct mlx5_devcom_comp_dev *devcom = esw->devcom;
 	struct mlx5_flow_handle *flow_rule;
 	struct mlx5e_rep_priv *rpriv;
 	struct mlx5e_rep_sq *rep_sq;
-	bool devcom_locked = false;
+	bool peers_ready;
 	int err;
 	int i;
 
@@ -518,9 +519,8 @@ static int mlx5e_sqs2vport_start(struct mlx5_eswitch *esw,
 
 	rpriv = mlx5e_rep_to_rep_priv(rep);
 
-	if (mlx5_devcom_comp_is_ready(esw->devcom) &&
-	    mlx5_devcom_for_each_peer_begin(esw->devcom))
-		devcom_locked = true;
+	mlx5_devcom_comp_read_lock(devcom);
+	peers_ready = mlx5_devcom_comp_is_ready(devcom);
 
 	for (i = 0; i < sqns_num; i++) {
 		rep_sq = kzalloc_obj(*rep_sq);
@@ -541,7 +541,7 @@ static int mlx5e_sqs2vport_start(struct mlx5_eswitch *esw,
 		rep_sq->sqn = sqns_array[i];
 
 		xa_init(&rep_sq->sq_peer);
-		if (devcom_locked) {
+		if (peers_ready) {
 			err = mlx5e_sqs2vport_add_peers_rules(esw, rep, rep_sq, i);
 			if (err) {
 				mlx5_eswitch_del_send_to_vport_rule(rep_sq->send_to_vport_rule);
@@ -554,16 +554,13 @@ static int mlx5e_sqs2vport_start(struct mlx5_eswitch *esw,
 		list_add(&rep_sq->list, &rpriv->vport_sqs_list);
 	}
 
-	if (devcom_locked)
-		mlx5_devcom_for_each_peer_end(esw->devcom);
+	mlx5_devcom_comp_read_unlock(devcom);
 
 	return 0;
 
 out_err:
 	mlx5e_sqs2vport_stop(esw, rep);
-
-	if (devcom_locked)
-		mlx5_devcom_for_each_peer_end(esw->devcom);
+	mlx5_devcom_comp_read_unlock(devcom);
 
 	return err;
 }
@@ -630,8 +627,12 @@ mlx5e_remove_sqs_fwd_rules(struct mlx5e_priv *priv)
 	struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
 	struct mlx5_eswitch_rep *rep = rpriv->rep;
+	struct mlx5_devcom_comp_dev *devcom;
 
+	devcom = esw->devcom;
+	mlx5_devcom_comp_read_lock(devcom);
 	mlx5e_sqs2vport_stop(esw, rep);
+	mlx5_devcom_comp_read_unlock(devcom);
 }
 
 static int
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
index 75855481522b..aed9e2a74162 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
@@ -456,6 +456,20 @@ void mlx5_devcom_comp_unlock(struct mlx5_devcom_comp_dev *devcom)
 	up_write(&devcom->comp->sem);
 }
 
+void mlx5_devcom_comp_read_lock(struct mlx5_devcom_comp_dev *devcom)
+{
+	if (!devcom)
+		return;
+	down_read(&devcom->comp->sem);
+}
+
+void mlx5_devcom_comp_read_unlock(struct mlx5_devcom_comp_dev *devcom)
+{
+	if (!devcom)
+		return;
+	up_read(&devcom->comp->sem);
+}
+
 int mlx5_devcom_comp_trylock(struct mlx5_devcom_comp_dev *devcom)
 {
 	if (!devcom)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h
index 7a704fafdbd3..94b0fe20cc26 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h
@@ -79,6 +79,8 @@ void *mlx5_devcom_get_next_peer_data_rcu(struct mlx5_devcom_comp_dev *devcom,
 
 void mlx5_devcom_comp_lock(struct mlx5_devcom_comp_dev *devcom);
 void mlx5_devcom_comp_unlock(struct mlx5_devcom_comp_dev *devcom);
+void mlx5_devcom_comp_read_lock(struct mlx5_devcom_comp_dev *devcom);
+void mlx5_devcom_comp_read_unlock(struct mlx5_devcom_comp_dev *devcom);
 int mlx5_devcom_comp_trylock(struct mlx5_devcom_comp_dev *devcom);
 void mlx5_devcom_comp_assert_locked(struct mlx5_devcom_comp_dev *devcom);
 

base-commit: b71c101bbb9814d163c00234d43c41f0a17907d4
-- 
2.44.0