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+AEA-oracle.com/T/+ACM-u

Request you to review it.

-
Praveen.


Oracle Confidential
+AD4- -----Original Message-----
+AD4- From: Leon Romanovsky +ADw-leon+AEA-kernel.org+AD4-
+AD4- Sent: Monday, July 13, 2026 2:06 PM
+AD4- To: Praveen Kannoju +ADw-praveen.kannoju+AEA-oracle.com+AD4-
+AD4- Cc: yishaih+AEA-nvidia.com+ADs- jgg+AEA-ziepe.ca+ADs- linux-rdma+AEA-vger.kernel.org+ADs- linux-
+AD4- kernel+AEA-vger.kernel.org
+AD4- Subject: Re: +AFs-PATCH v2+AF0- IB/mlx4: Fix stale CM id+AF8-map entries when RTU is
+AD4- never received
+AD4-
+AD4- On Mon, Jul 06, 2026 at 12:02:55PM +-0000, Praveen Kumar Kannoju wrote:
+AD4- +AD4- mlx4+AF8-ib+AF8-multiplex+AF8-cm+AF8-handler() allocates an id+AF8-map+AF8-entry for CM
+AD4- +AD4- transactions, but the entry is normally released only on DREQ or REJ
+AD4- +AD4- flows.
+AD4- +AD4-
+AD4- +AD4- In the duplicate REP handling scenario, cm+AF8-dup+AF8-rep+AF8-handler() may be
+AD4- +AD4- invoked when the remote side receives a REP for which no matching
+AD4- +AD4- cm+AF8-id+AF8-priv exists. In such cases the CM handshake never reaches RTU,
+AD4- +AD4- and the sender side may never receive either DREQ or REJ cleanup events.
+AD4- +AD4-
+AD4- +AD4- As a result, the allocated id+AF8-map+AF8-entry remains indefinitely,
+AD4- +AD4- resulting in a stale mapping leak.
+AD4- +AD4-
+AD4- +AD4- Fix this by arming an RTU-abandon cleanup timeout when the
+AD4- +AD4- id+AF8-map+AF8-entry is allocated. The timeout uses the mlx4 CM workqueue and
+AD4- +AD4- the existing
+AD4- +AD4- schedule+AF8-delayed() path, so later DREQ/REJ cleanup can shorten the
+AD4- +AD4- pending timeout with mod+AF8-delayed+AF8-work().
+AD4- +AD4-
+AD4- +AD4- Track whether a pending cleanup timeout is still waiting for RTU. RTU
+AD4- +AD4- cancels only that initial timeout+ADs- if DREQ/REJ has already converted
+AD4- +AD4- it to normal teardown cleanup, a late or duplicate RTU does not cancel
+AD4- +AD4- the teardown timer. If the RTU timeout callback has already started,
+AD4- +AD4- leave the entry on the timeout path and make the RTU packet lose that race.
+AD4- +AD4-
+AD4- +AD4- Hold id+AF8-map+AF8-lock while looking up the entry, canceling the RTU
+AD4- +AD4- timeout, scheduling teardown cleanup, and copying the id values needed
+AD4- +AD4- by the CM handlers. The delayed-work callback rechecks
+AD4- +AD4- scheduled+AF8-delete under the same lock before removing and freeing the
+AD4- +AD4- entry, avoiding use-after-free when RTU races with timeout execution.
+AD4- +AD4-
+AD4- +AD4- Signed-off-by: Praveen Kumar Kannoju +ADw-praveen.kannoju+AEA-oracle.com+AD4-
+AD4- +AD4- ---
+AD4- +AD4- v1:
+AD4- +AD4- https://lore.kernel.org/linux-rdma/20260507154755.452008-1-
+AD4- praveen.kan
+AD4- +AD4- noju+AEA-oracle.com/T/+ACM-u
+AD4- +AD4-
+AD4- +AD4- Changes in v2:
+AD4- +AD4- - Queue the RTU-abandon timeout on the mlx4 CM workqueue through
+AD4- +AD4-   schedule+AF8-delayed() and use mod+AF8-delayed+AF8-work() so DREQ/REJ cleanup
+AD4- can
+AD4- +AD4-   shorten a pending RTU timeout.
+AD4- +AD4- - Track RTU-abandon cleanup separately from normal DREQ/REJ cleanup so
+AD4- a
+AD4- +AD4-   late or duplicate RTU does not cancel a teardown timer.
+AD4- +AD4- - Hold id+AF8-map+AF8-lock while looking up id+AF8-map entries, canceling or updating
+AD4- +AD4-   delayed work, and copying CM IDs needed by the handlers.
+AD4- +AD4- - Make RTU lose the race when the timeout callback has already started.
+AD4- +AD4-
+AD4- +AD4-  drivers/infiniband/hw/mlx4/cm.c +AHw- 101
+AD4- +AD4- +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-----------
+AD4- +AD4-  1 file changed, 76 insertions(+-), 25 deletions(-)
+AD4- +AD4-
+AD4- +AD4- diff --git a/drivers/infiniband/hw/mlx4/cm.c
+AD4- +AD4- b/drivers/infiniband/hw/mlx4/cm.c index 63a868a..f7905df 100644
+AD4- +AD4- --- a/drivers/infiniband/hw/mlx4/cm.c
+AD4- +AD4- +-+-+- b/drivers/infiniband/hw/mlx4/cm.c
+AD4- +AD4- +AEAAQA- -40,6 +-40,7 +AEAAQA-
+AD4- +AD4-  +ACM-include +ACI-mlx4+AF8-ib.h+ACI-
+AD4- +AD4-
+AD4- +AD4-  +ACM-define CM+AF8-CLEANUP+AF8-CACHE+AF8-TIMEOUT  (30 +ACo- HZ)
+AD4- +AD4- +-+ACM-define CM+AF8-RTU+AF8-TIMEOUT               (60 +ACo- HZ)
+AD4- +AD4-
+AD4- +AD4-  struct id+AF8-map+AF8-entry +AHs-
+AD4- +AD4-     struct rb+AF8-node node+ADs-
+AD4- +AD4- +AEAAQA- -48,6 +-49,7 +AEAAQA- struct id+AF8-map+AF8-entry +AHs-
+AD4- +AD4-     u32 pv+AF8-cm+AF8-id+ADs-
+AD4- +AD4-     int slave+AF8-id+ADs-
+AD4- +AD4-     int scheduled+AF8-delete+ADs-
+AD4- +AD4- +-   bool rtu+AF8-timeout+ADs-
+AD4- +AD4-     struct mlx4+AF8-ib+AF8-dev +ACo-dev+ADs-
+AD4- +AD4-
+AD4- +AD4-     struct list+AF8-head list+ADs-
+AD4- +AD4- +AEAAQA- -184,6 +-186,10 +AEAAQA- static void id+AF8-map+AF8-ent+AF8-timeout(struct
+AD4- work+AF8-struct +ACo-work)
+AD4- +AD4-     struct rb+AF8-root +ACo-sl+AF8-id+AF8-map +AD0- +ACY-sriov-+AD4-sl+AF8-id+AF8-map+ADs-
+AD4- +AD4-
+AD4- +AD4-     spin+AF8-lock(+ACY-sriov-+AD4-id+AF8-map+AF8-lock)+ADs-
+AD4- +AD4- +-   if (+ACE-ent-+AD4-scheduled+AF8-delete) +AHs-
+AD4- +AD4- +-           spin+AF8-unlock(+ACY-sriov-+AD4-id+AF8-map+AF8-lock)+ADs-
+AD4- +AD4- +-           return+ADs-
+AD4- +AD4- +-   +AH0-
+AD4- +AD4-     if (+ACE-xa+AF8-erase(+ACY-sriov-+AD4-pv+AF8-id+AF8-table, ent-+AD4-pv+AF8-cm+AF8-id))
+AD4- +AD4-             goto out+ADs-
+AD4- +AD4-     found+AF8-ent +AD0- id+AF8-map+AF8-find+AF8-by+AF8-sl+AF8-id(+ACY-dev-+AD4-ib+AF8-dev, ent-+AD4-slave+AF8-id,
+AD4- +AD4- ent-+AD4-sl+AF8-cm+AF8-id)+ADs- +AEAAQA- -228,8 +-234,12 +AEAAQA- static void sl+AF8-id+AF8-map+AF8-add(struct
+AD4- ib+AF8-device +ACo-ibdev, struct id+AF8-map+AF8-entry +ACo-new)
+AD4- +AD4-     rb+AF8-insert+AF8-color(+ACY-new-+AD4-node, sl+AF8-id+AF8-map)+ADs-  +AH0-
+AD4- +AD4-
+AD4- +AD4- +-static void schedule+AF8-delayed(struct ib+AF8-device +ACo-ibdev, struct id+AF8-map+AF8-entry
+AD4- +ACo-id,
+AD4- +AD4- +-                        unsigned long timeout, bool rtu+AF8-timeout)+ADs-
+AD4- +AD4- +-
+AD4- +AD4-  static struct id+AF8-map+AF8-entry +ACo-
+AD4- +AD4- -id+AF8-map+AF8-alloc(struct ib+AF8-device +ACo-ibdev, int slave+AF8-id, u32 sl+AF8-cm+AF8-id)
+AD4- +AD4- +-id+AF8-map+AF8-alloc(struct ib+AF8-device +ACo-ibdev, int slave+AF8-id, u32 sl+AF8-cm+AF8-id,
+AD4- +AD4- +-        u32 +ACo-pv+AF8-cm+AF8-id)
+AD4- +AD4-  +AHs-
+AD4- +AD4-     int ret+ADs-
+AD4- +AD4-     struct id+AF8-map+AF8-entry +ACo-ent+ADs-
+AD4- +AD4- +AEAAQA- -242,6 +-252,7 +AEAAQA- id+AF8-map+AF8-alloc(struct ib+AF8-device +ACo-ibdev, int slave+AF8-id,
+AD4- u32 sl+AF8-cm+AF8-id)
+AD4- +AD4-     ent-+AD4-sl+AF8-cm+AF8-id +AD0- sl+AF8-cm+AF8-id+ADs-
+AD4- +AD4-     ent-+AD4-slave+AF8-id +AD0- slave+AF8-id+ADs-
+AD4- +AD4-     ent-+AD4-scheduled+AF8-delete +AD0- 0+ADs-
+AD4- +AD4- +-   ent-+AD4-rtu+AF8-timeout +AD0- false+ADs-
+AD4- +AD4-     ent-+AD4-dev +AD0- to+AF8-mdev(ibdev)+ADs-
+AD4- +AD4-     INIT+AF8-DELAYED+AF8-WORK(+ACY-ent-+AD4-timeout, id+AF8-map+AF8-ent+AF8-timeout)+ADs-
+AD4- +AD4-
+AD4- +AD4- +AEAAQA- -251,6 +-262,8 +AEAAQA- id+AF8-map+AF8-alloc(struct ib+AF8-device +ACo-ibdev, int slave+AF8-id,
+AD4- u32 sl+AF8-cm+AF8-id)
+AD4- +AD4-             spin+AF8-lock(+ACY-sriov-+AD4-id+AF8-map+AF8-lock)+ADs-
+AD4- +AD4-             sl+AF8-id+AF8-map+AF8-add(ibdev, ent)+ADs-
+AD4- +AD4-             list+AF8-add+AF8-tail(+ACY-ent-+AD4-list, +ACY-sriov-+AD4-cm+AF8-list)+ADs-
+AD4- +AD4- +-           +ACo-pv+AF8-cm+AF8-id +AD0- ent-+AD4-pv+AF8-cm+AF8-id+ADs-
+AD4- +AD4- +-           schedule+AF8-delayed(ibdev, ent, CM+AF8-RTU+AF8-TIMEOUT, true)+ADs-
+AD4- +AD4-             spin+AF8-unlock(+ACY-sriov-+AD4-id+AF8-map+AF8-lock)+ADs-
+AD4- +AD4-             return ent+ADs-
+AD4- +AD4-     +AH0-
+AD4- +AD4- +AEAAQA- -261,48 +-274,46 +AEAAQA- id+AF8-map+AF8-alloc(struct ib+AF8-device +ACo-ibdev, int
+AD4- slave+AF8-id, u32 sl+AF8-cm+AF8-id)
+AD4- +AD4-     return ERR+AF8-PTR(-ENOMEM)+ADs-
+AD4- +AD4-  +AH0-
+AD4- +AD4-
+AD4- +AD4- +-/+ACo- Lock should be taken before called +ACo-/
+AD4-
+AD4- /+ACo- Lock should be taken before called +ACo-/ sentence needs to be replaced by
+AD4- lockdep+AF8-held(+ACY-sriov-+AD4-id+AF8-map+AF8-lock)+ADs-
+AD4-
+AD4- Thanks