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

[Leon Romanovsky <leon@kernel.org>]

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