mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] IB/mlx4: Fix stale CM id_map entries when RTU is never received
@ 2026-07-15  8:07 Praveen Kumar Kannoju
  2026-07-16  8:29 ` Leon Romanovsky
  0 siblings, 1 reply; 3+ messages in thread
From: Praveen Kumar Kannoju @ 2026-07-15  8:07 UTC (permalink / raw)
  To: yishaih, jgg, leon, linux-rdma, linux-kernel; +Cc: Praveen Kumar Kannoju

mlx4_ib_multiplex_cm_handler() allocates an id_map_entry for CM
transactions, but the entry is normally released only on DREQ or REJ
flows.

In the duplicate REP handling scenario, cm_dup_rep_handler() may be
invoked when the remote side receives a REP for which no matching
cm_id_priv exists. In such cases the CM handshake never reaches RTU, and
the sender side may never receive either DREQ or REJ cleanup events.

As a result, the allocated id_map_entry remains indefinitely, resulting in
a stale mapping leak.

Fix this by arming an RTU-abandon cleanup timeout when the id_map_entry is
allocated. The timeout uses the mlx4 CM workqueue and the existing
schedule_delayed() path, so later DREQ/REJ cleanup can shorten the pending
timeout with mod_delayed_work().

Track whether a pending cleanup timeout is still waiting for RTU. RTU
cancels only that initial timeout; if DREQ/REJ has already converted it to
normal teardown cleanup, a late or duplicate RTU does not cancel the
teardown timer. If the RTU timeout callback has already started, leave the
entry on the timeout path and make the RTU packet lose that race.

Hold id_map_lock while looking up the entry, canceling the RTU timeout,
scheduling teardown cleanup, and copying the id values needed by the CM
handlers. The delayed-work callback rechecks scheduled_delete under the
same lock before removing and freeing the entry, avoiding use-after-free
when RTU races with timeout execution.

Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
---
v1: https://lore.kernel.org/linux-rdma/20260507154755.452008-1-praveen.kannoju@oracle.com/T/#u
v2: https://lore.kernel.org/linux-rdma/BL0PR10MB282074FA0D571F5072DFCB4B8CF12@BL0PR10MB2820.namprd10.prod.outlook.com/T/#t
v3: https://lore.kernel.org/linux-rdma/20260713111142.1206710-1-praveen.kannoju@oracle.com/T/#u

Changes in v4:
- Rebase onto latest rdma-next.
- Preserve rdma-next CM_REJ_ATTR_ID handling while moving cleanup
  scheduling under id_map_lock.

Changes in v3:
- Replace "Lock should be taken before called" comments with
  lockdep_assert_held(&sriov->id_map_lock).

Changes in v2:
- Queue the RTU-abandon timeout on the mlx4 CM workqueue through
  schedule_delayed() and use mod_delayed_work() so DREQ/REJ cleanup can
  shorten a pending RTU timeout.
- Track RTU-abandon cleanup separately from normal DREQ/REJ cleanup so a
  late or duplicate RTU does not cancel a teardown timer.
- Hold id_map_lock while looking up id_map entries, canceling or updating
  delayed work, and copying CM IDs needed by the handlers.
- Make RTU lose the race when the timeout callback has already started.

 drivers/infiniband/hw/mlx4/cm.c | 111 +++++++++++++++++++++++++-------
 1 file changed, 87 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index 202fd5365e35..1e5e525fea73 100644
--- a/drivers/infiniband/hw/mlx4/cm.c
+++ b/drivers/infiniband/hw/mlx4/cm.c
@@ -40,6 +40,7 @@
 #include "mlx4_ib.h"
 
 #define CM_CLEANUP_CACHE_TIMEOUT  (30 * HZ)
+#define CM_RTU_TIMEOUT		  (60 * HZ)
 
 struct id_map_entry {
 	struct rb_node node;
@@ -48,6 +49,7 @@ struct id_map_entry {
 	u32 pv_cm_id;
 	int slave_id;
 	int scheduled_delete;
+	bool rtu_timeout;
 	struct mlx4_ib_dev *dev;
 
 	struct list_head list;
@@ -184,6 +186,10 @@ static void id_map_ent_timeout(struct work_struct *work)
 	struct rb_root *sl_id_map = &sriov->sl_id_map;
 
 	spin_lock(&sriov->id_map_lock);
+	if (!ent->scheduled_delete) {
+		spin_unlock(&sriov->id_map_lock);
+		return;
+	}
 	if (!xa_erase(&sriov->pv_id_table, ent->pv_cm_id))
 		goto out;
 	found_ent = id_map_find_by_sl_id(&dev->ib_dev, ent->slave_id, ent->sl_cm_id);
@@ -228,8 +234,12 @@ static void sl_id_map_add(struct ib_device *ibdev, struct id_map_entry *new)
 	rb_insert_color(&new->node, sl_id_map);
 }
 
+static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id,
+			     unsigned long timeout, bool rtu_timeout);
+
 static struct id_map_entry *
-id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
+id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id,
+	     u32 *pv_cm_id)
 {
 	int ret;
 	struct id_map_entry *ent;
@@ -242,6 +252,7 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
 	ent->sl_cm_id = sl_cm_id;
 	ent->slave_id = slave_id;
 	ent->scheduled_delete = 0;
+	ent->rtu_timeout = false;
 	ent->dev = to_mdev(ibdev);
 	INIT_DELAYED_WORK(&ent->timeout, id_map_ent_timeout);
 
@@ -251,6 +262,8 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
 		spin_lock(&sriov->id_map_lock);
 		sl_id_map_add(ibdev, ent);
 		list_add_tail(&ent->list, &sriov->cm_list);
+		*pv_cm_id = ent->pv_cm_id;
+		schedule_delayed(ibdev, ent, CM_RTU_TIMEOUT, true);
 		spin_unlock(&sriov->id_map_lock);
 		return ent;
 	}
@@ -267,42 +280,41 @@ id_map_get(struct ib_device *ibdev, int *pv_cm_id, int slave_id, int sl_cm_id)
 	struct id_map_entry *ent;
 	struct mlx4_ib_sriov *sriov = &to_mdev(ibdev)->sriov;
 
-	spin_lock(&sriov->id_map_lock);
+	lockdep_assert_held(&sriov->id_map_lock);
 	if (*pv_cm_id == -1) {
 		ent = id_map_find_by_sl_id(ibdev, slave_id, sl_cm_id);
 		if (ent)
 			*pv_cm_id = (int) ent->pv_cm_id;
 	} else
 		ent = xa_load(&sriov->pv_id_table, *pv_cm_id);
-	spin_unlock(&sriov->id_map_lock);
 
 	return ent;
 }
 
-static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id)
+static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id,
+			     unsigned long timeout, bool rtu_timeout)
 {
 	struct mlx4_ib_sriov *sriov = &to_mdev(ibdev)->sriov;
 	unsigned long flags;
 
-	spin_lock(&sriov->id_map_lock);
+	lockdep_assert_held(&sriov->id_map_lock);
 	spin_lock_irqsave(&sriov->going_down_lock, flags);
 	/*make sure that there is no schedule inside the scheduled work.*/
-	if (!sriov->is_going_down && !id->scheduled_delete) {
+	if (!sriov->is_going_down || id->scheduled_delete) {
 		id->scheduled_delete = 1;
-		queue_delayed_work(cm_wq, &id->timeout, CM_CLEANUP_CACHE_TIMEOUT);
-	} else if (id->scheduled_delete) {
-		/* Adjust timeout if already scheduled */
-		mod_delayed_work(cm_wq, &id->timeout, CM_CLEANUP_CACHE_TIMEOUT);
+		id->rtu_timeout = rtu_timeout;
+		mod_delayed_work(cm_wq, &id->timeout, timeout);
 	}
 	spin_unlock_irqrestore(&sriov->going_down_lock, flags);
-	spin_unlock(&sriov->id_map_lock);
 }
 
 #define REJ_REASON(m) be16_to_cpu(((struct cm_generic_msg *)(m))->rej_reason)
 int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_id,
 		struct ib_mad *mad)
 {
+	struct mlx4_ib_sriov *sriov = &to_mdev(ibdev)->sriov;
 	struct id_map_entry *id;
+	u32 pv_cm_id_to_set = 0;
 	u32 sl_cm_id;
 	int pv_cm_id = -1;
 
@@ -312,12 +324,22 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_id
 	    mad->mad_hdr.attr_id == CM_SIDR_REQ_ATTR_ID ||
 	    (mad->mad_hdr.attr_id == CM_REJ_ATTR_ID && REJ_REASON(mad) == IB_CM_REJ_TIMEOUT)) {
 		sl_cm_id = get_local_comm_id(mad);
+		spin_lock(&sriov->id_map_lock);
 		id = id_map_get(ibdev, &pv_cm_id, slave_id, sl_cm_id);
+		if (id) {
+			pv_cm_id_to_set = id->pv_cm_id;
+			if (mad->mad_hdr.attr_id == CM_REJ_ATTR_ID)
+				schedule_delayed(ibdev, id,
+						 CM_CLEANUP_CACHE_TIMEOUT,
+						 false);
+		}
+		spin_unlock(&sriov->id_map_lock);
 		if (id)
 			goto cont;
 		if (mad->mad_hdr.attr_id == CM_REJ_ATTR_ID)
 			return 0;
-		id = id_map_alloc(ibdev, slave_id, sl_cm_id);
+		id = id_map_alloc(ibdev, slave_id, sl_cm_id,
+				  &pv_cm_id_to_set);
 		if (IS_ERR(id)) {
 			mlx4_ib_warn(ibdev, "%s: id{slave: %d, sl_cm_id: 0x%x} Failed to id_map_alloc\n",
 				__func__, slave_id, sl_cm_id);
@@ -325,14 +347,39 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_id
 		}
 	} else if (mad->mad_hdr.attr_id == CM_REJ_ATTR_ID) {
 		sl_cm_id = get_local_comm_id(mad);
+		spin_lock(&sriov->id_map_lock);
 		id = id_map_get(ibdev, &pv_cm_id, slave_id, sl_cm_id);
+		if (id) {
+			pv_cm_id_to_set = id->pv_cm_id;
+			schedule_delayed(ibdev, id, CM_CLEANUP_CACHE_TIMEOUT,
+					 false);
+		}
+		spin_unlock(&sriov->id_map_lock);
 		if (!id)
 			return 0;
 	} else if (mad->mad_hdr.attr_id == CM_SIDR_REP_ATTR_ID) {
 		return 0;
 	} else {
 		sl_cm_id = get_local_comm_id(mad);
+		spin_lock(&sriov->id_map_lock);
 		id = id_map_get(ibdev, &pv_cm_id, slave_id, sl_cm_id);
+		if (id) {
+			if (mad->mad_hdr.attr_id == CM_RTU_ATTR_ID &&
+			    id->rtu_timeout) {
+				id->rtu_timeout = false;
+				if (cancel_delayed_work(&id->timeout))
+					id->scheduled_delete = 0;
+				else
+					id = NULL;
+			}
+			if (id)
+				pv_cm_id_to_set = id->pv_cm_id;
+			if (id && mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID)
+				schedule_delayed(ibdev, id,
+						 CM_CLEANUP_CACHE_TIMEOUT,
+						 false);
+		}
+		spin_unlock(&sriov->id_map_lock);
 	}
 
 	if (!id) {
@@ -342,11 +389,7 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_id
 	}
 
 cont:
-	set_local_comm_id(mad, id->pv_cm_id);
-
-	if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID ||
-	    mad->mad_hdr.attr_id == CM_REJ_ATTR_ID)
-		schedule_delayed(ibdev, id);
+	set_local_comm_id(mad, pv_cm_id_to_set);
 	return 0;
 }
 
@@ -436,7 +479,10 @@ int mlx4_ib_demux_cm_handler(struct ib_device *ibdev, int port, int *slave,
 	struct mlx4_ib_sriov *sriov = &to_mdev(ibdev)->sriov;
 	u32 rem_pv_cm_id = get_local_comm_id(mad);
 	u32 pv_cm_id;
+	u32 sl_cm_id = 0;
 	struct id_map_entry *id;
+	int pv_cm_id_int;
+	int slave_id = 0;
 	int sts;
 
 	if (mad->mad_hdr.attr_id == CM_REQ_ATTR_ID ||
@@ -464,7 +510,28 @@ int mlx4_ib_demux_cm_handler(struct ib_device *ibdev, int port, int *slave,
 	}
 
 	pv_cm_id = get_remote_comm_id(mad);
-	id = id_map_get(ibdev, (int *)&pv_cm_id, -1, -1);
+	pv_cm_id_int = pv_cm_id;
+	spin_lock(&sriov->id_map_lock);
+	id = id_map_get(ibdev, &pv_cm_id_int, -1, -1);
+	if (id) {
+		if (mad->mad_hdr.attr_id == CM_RTU_ATTR_ID &&
+		    id->rtu_timeout) {
+			id->rtu_timeout = false;
+			if (cancel_delayed_work(&id->timeout))
+				id->scheduled_delete = 0;
+			else
+				id = NULL;
+		}
+		if (id && slave)
+			slave_id = id->slave_id;
+		if (id)
+			sl_cm_id = id->sl_cm_id;
+		if (id && (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID ||
+			   mad->mad_hdr.attr_id == CM_REJ_ATTR_ID))
+			schedule_delayed(ibdev, id,
+					 CM_CLEANUP_CACHE_TIMEOUT, false);
+	}
+	spin_unlock(&sriov->id_map_lock);
 
 	if (!id) {
 		if (mad->mad_hdr.attr_id == CM_REJ_ATTR_ID &&
@@ -479,12 +546,8 @@ int mlx4_ib_demux_cm_handler(struct ib_device *ibdev, int port, int *slave,
 	}
 
 	if (slave)
-		*slave = id->slave_id;
-	set_remote_comm_id(mad, id->sl_cm_id);
-
-	if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID ||
-	    mad->mad_hdr.attr_id == CM_REJ_ATTR_ID)
-		schedule_delayed(ibdev, id);
+		*slave = slave_id;
+	set_remote_comm_id(mad, sl_cm_id);
 
 	return 0;
 }
-- 
2.43.7

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] IB/mlx4: Fix stale CM id_map entries when RTU is never received
  2026-07-15  8:07 [PATCH v4] IB/mlx4: Fix stale CM id_map entries when RTU is never received Praveen Kumar Kannoju
@ 2026-07-16  8:29 ` Leon Romanovsky
  2026-07-16  8:33   ` Praveen Kannoju
  0 siblings, 1 reply; 3+ messages in thread
From: Leon Romanovsky @ 2026-07-16  8:29 UTC (permalink / raw)
  To: yishaih, jgg, linux-rdma, linux-kernel, Praveen Kumar Kannoju


On Wed, 15 Jul 2026 08:07:38 +0000, Praveen Kumar Kannoju wrote:
> mlx4_ib_multiplex_cm_handler() allocates an id_map_entry for CM
> transactions, but the entry is normally released only on DREQ or REJ
> flows.
> 
> In the duplicate REP handling scenario, cm_dup_rep_handler() may be
> invoked when the remote side receives a REP for which no matching
> cm_id_priv exists. In such cases the CM handshake never reaches RTU, and
> the sender side may never receive either DREQ or REJ cleanup events.
> 
> [...]

Applied, thanks!

[1/1] IB/mlx4: Fix stale CM id_map entries when RTU is never received
      https://git.kernel.org/rdma/rdma/c/548897f0f78837

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH v4] IB/mlx4: Fix stale CM id_map entries when RTU is never received
  2026-07-16  8:29 ` Leon Romanovsky
@ 2026-07-16  8:33   ` Praveen Kannoju
  0 siblings, 0 replies; 3+ messages in thread
From: Praveen Kannoju @ 2026-07-16  8:33 UTC (permalink / raw)
  To: Leon Romanovsky, yishaih, jgg, linux-rdma, linux-kernel
  Cc: Anand Khoje, Manjunath Patil

Oracle Confidential

Thank you very much, Leon.

-
Praveen.


Oracle Confidential
> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Thursday, July 16, 2026 2:00 PM
> To: yishaih@nvidia.com; jgg@ziepe.ca; linux-rdma@vger.kernel.org; linux-
> kernel@vger.kernel.org; Praveen Kannoju <praveen.kannoju@oracle.com>
> Subject: Re: [PATCH v4] IB/mlx4: Fix stale CM id_map entries when RTU is
> never received
>
>
> On Wed, 15 Jul 2026 08:07:38 +0000, Praveen Kumar Kannoju wrote:
> > mlx4_ib_multiplex_cm_handler() allocates an id_map_entry for CM
> > transactions, but the entry is normally released only on DREQ or REJ
> > flows.
> >
> > In the duplicate REP handling scenario, cm_dup_rep_handler() may be
> > invoked when the remote side receives a REP for which no matching
> > cm_id_priv exists. In such cases the CM handshake never reaches RTU,
> > and the sender side may never receive either DREQ or REJ cleanup events.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] IB/mlx4: Fix stale CM id_map entries when RTU is never received
>       https://git.kernel.org/rdma/rdma/c/548897f0f78837
>
> Best regards,
> --
> Leon Romanovsky <leon@kernel.org>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-16  8:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-15  8:07 [PATCH v4] IB/mlx4: Fix stale CM id_map entries when RTU is never received Praveen Kumar Kannoju
2026-07-16  8:29 ` Leon Romanovsky
2026-07-16  8:33   ` Praveen Kannoju

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox