Re: [PATCH] IB/mlx4: delete allocated id_map_entry while sending REJ

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, May 06, 2026 at 09:08:24AM +0000, Praveen Kumar Kannoju wrote:
> During scenarios where a REJ is sent after a REQ or REP, the allocated
> is_map_entry remains in memory, resulting in a memory leak. Scheduling the
> entry for deletion during REJ handling, if it is not NULL, resolves the
> issue.

Well, the leak seems quite likely, but I'm not sure about this fix.

This code looks quite odd and it seems to have other races as well, so
IDK..

> Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
> ---
>  drivers/infiniband/hw/mlx4/cm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
> index 63a868a3822f..21f2f401ed61 100644
> --- a/drivers/infiniband/hw/mlx4/cm.c
> +++ b/drivers/infiniband/hw/mlx4/cm.c
> @@ -321,10 +321,9 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_id
>  				__func__, slave_id, sl_cm_id);
>  			return PTR_ERR(id);
>  		}
> -	} else if (mad->mad_hdr.attr_id == CM_REJ_ATTR_ID ||
> -		   mad->mad_hdr.attr_id == CM_SIDR_REP_ATTR_ID) {
> +	} else if (mad->mad_hdr.attr_id == CM_SIDR_REP_ATTR_ID)
>  		return 0;
> -	} else {
> +	else {
>  		sl_cm_id = get_local_comm_id(mad);
>  		id = id_map_get(ibdev, &pv_cm_id, slave_id, sl_cm_id);
>  	}

What is this change for?

It does look like ignoring the rej isn't right, but then also why does
this rej just search and free but the rej in the prior stanza is
allocating too?

> @@ -338,7 +337,8 @@ 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)
> +	if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID ||
> +	    mad->mad_hdr.attr_id == CM_REJ_ATTR_ID)
>  		schedule_delayed(ibdev, id);
>  	return 0;
>  }

SIDR seems troubled as well.

AI pointed out the use of id like this is racey too.

But broadly this seems like it might be the right direction, but the
commit message should explain what this logic is alot better

Jason