[PATCH v2] IB/mlx4: Fix stale CM id_map entries when RTU is never received

[PATCH v2] IB/mlx4: Fix stale CM id_map entries when RTU is never received

[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

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 | 101 ++++++++++++++++++++++++++++++----------
 1 file changed, 76 insertions(+), 25 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index 63a868a..f7905df 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;
 	}
@@ -261,48 +274,46 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
 	return ERR_PTR(-ENOMEM);
 }
 
+/* Lock should be taken before called */
 static struct id_map_entry *
 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);
 	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);
+		ent = xa_load(&to_mdev(ibdev)->sriov.pv_id_table, *pv_cm_id);
 
 	return ent;
 }
 
-static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id)
+/* Lock should be taken before called */
+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);
 	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,10 +323,15 @@ 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;
+		spin_unlock(&sriov->id_map_lock);
 		if (id)
 			goto cont;
-		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);
@@ -326,7 +342,25 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_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) {
@@ -336,10 +370,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)
-		schedule_delayed(ibdev, id);
+	set_local_comm_id(mad, pv_cm_id_to_set);
 	return 0;
 }
 
@@ -429,7 +460,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 ||
@@ -457,7 +491,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 &&
@@ -472,12 +527,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;
 }

RE: [PATCH v2] IB/mlx4: Fix stale CM id_map entries when RTU is never received

[Praveen Kannoju]

Oracle Confidential

+ Anand, Manjunath.


Oracle Confidential
> -----Original Message-----
> From: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
> Sent: Monday, July 6, 2026 5:33 PM
> To: yishaih@nvidia.com; jgg@ziepe.ca; leon@kernel.org; linux-
> rdma@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Praveen Kannoju <praveen.kannoju@oracle.com>
> Subject: [PATCH v2] IB/mlx4: Fix stale CM id_map entries when RTU is never
> received
>
> 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
>
> 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 | 101
> ++++++++++++++++++++++++++++++----------
>  1 file changed, 76 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx4/cm.c
> b/drivers/infiniband/hw/mlx4/cm.c
> index 63a868a..f7905df 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;
>       }
> @@ -261,48 +274,46 @@ id_map_alloc(struct ib_device *ibdev, int slave_id,
> u32 sl_cm_id)
>       return ERR_PTR(-ENOMEM);
>  }
>
> +/* Lock should be taken before called */
>  static struct id_map_entry *
>  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);
>       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);
> +             ent = xa_load(&to_mdev(ibdev)->sriov.pv_id_table,
> *pv_cm_id);
>
>       return ent;
>  }
>
> -static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry
> *id)
> +/* Lock should be taken before called */
> +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);
>       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,10 +323,15 @@ 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;
> +             spin_unlock(&sriov->id_map_lock);
>               if (id)
>                       goto cont;
> -             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);
> @@ -326,7 +342,25 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device
> *ibdev, int port, int slave_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) {
> @@ -336,10 +370,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)
> -             schedule_delayed(ibdev, id);
> +     set_local_comm_id(mad, pv_cm_id_to_set);
>       return 0;
>  }
>
> @@ -429,7 +460,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 ||
> @@ -457,7 +491,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 &&
> @@ -472,12 +527,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;
>  }

Re: [PATCH v2] IB/mlx4: Fix stale CM id_map entries when RTU is never received

[Leon Romanovsky]

On Mon, Jul 06, 2026 at 12:02:55PM +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.
> 
> 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
> 
> 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 | 101 ++++++++++++++++++++++++++++++----------
>  1 file changed, 76 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
> index 63a868a..f7905df 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;
>  	}
> @@ -261,48 +274,46 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> +/* Lock should be taken before called */

/* Lock should be taken before called */ sentence needs to be replaced 
by lockdep_held(&sriov->id_map_lock);

Thanks

RE: [PATCH v2] IB/mlx4: Fix stale CM id_map entries when RTU is never received

[Praveen Kannoju]

Oracle Confidential

Thank you very much for the review, Leon.
Have addressed your comment and submitted the v3.
https://lore.kernel.org/linux-rdma/20260713111142.1206710-1-praveen.kannoju@oracle.com/T/#u

Request you to review it.

-
Praveen.


Oracle Confidential
> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Monday, July 13, 2026 2:06 PM
> To: Praveen Kannoju <praveen.kannoju@oracle.com>
> Cc: yishaih@nvidia.com; jgg@ziepe.ca; linux-rdma@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2] IB/mlx4: Fix stale CM id_map entries when RTU is
> never received
>
> On Mon, Jul 06, 2026 at 12:02:55PM +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.
> >
> > 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.kan
> > noju@oracle.com/T/#u
> >
> > 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 | 101
> > ++++++++++++++++++++++++++++++----------
> >  1 file changed, 76 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx4/cm.c
> > b/drivers/infiniband/hw/mlx4/cm.c index 63a868a..f7905df 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;
> >     }
> > @@ -261,48 +274,46 @@ id_map_alloc(struct ib_device *ibdev, int
> slave_id, u32 sl_cm_id)
> >     return ERR_PTR(-ENOMEM);
> >  }
> >
> > +/* Lock should be taken before called */
>
> /* Lock should be taken before called */ sentence needs to be replaced by
> lockdep_held(&sriov->id_map_lock);
>
> Thanks