[PATCH] IB/mlx4: Avoid requeueing executing CM timeout work

[PATCH] IB/mlx4: Avoid requeueing executing CM timeout work

[Praveen Kumar Kannoju]

id->scheduled_delete is set by schedule_delayed() itself.  It is used
in two cases after the RTU timeout support was added:

* A new id_map_entry is created, and a 60-second RTU timeout is
  scheduled.
* DREQ or REJ is received for an existing entry, and the normal
  30-second teardown timeout is scheduled or adjusted.

The following race is possible when the RTU timeout expires while a
DREQ or REJ is handled:

 CPU 0: timeout work                         CPU 1: DREQ/REJ handler
 --------------------                        -----------------------
 RTU timer expires
 timeout callback starts
 waits for id_map_lock                       takes id_map_lock

                                             calls schedule_delayed()
                                             calls mod_delayed_work()

                                             drops id_map_lock

 takes id_map_lock
 removes mapping
 frees id_map_entry

                                             requeued timeout runs
                                             accesses freed id_map_entry

mod_delayed_work() may requeue the work after the timeout callback has
started.  The original timeout callback then frees the id_map_entry,
leaving the requeued work with a dangling delayed_work pointer.

When a timeout is already scheduled, first cancel its pending work.  A
successful cancellation means the callback has not started and the
timeout can safely be scheduled again.  If cancellation fails, the
callback is executing and will free the entry after id_map_lock is
released; do not requeue it.

cancel_delayed_work() is deliberately used instead of
cancel_delayed_work_sync(), because the timeout callback may be waiting
for id_map_lock, which is held by the caller.

Fixes: fa09adf4231f ("IB/mlx4: Fix stale CM id_map entries when RTU is never received")
Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
---
 drivers/infiniband/hw/mlx4/cm.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index 1e5e525fea73..810f95e55fda 100644
--- a/drivers/infiniband/hw/mlx4/cm.c
+++ b/drivers/infiniband/hw/mlx4/cm.c
@@ -301,10 +301,20 @@ static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id,
 	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) {
+		/*
+		 * Do not requeue timeout work that is already executing. The
+		 * timeout handler may be waiting for id_map_lock and will free
+		 * this entry after the caller releases it.
+		 */
+		if (id->scheduled_delete &&
+		    !cancel_delayed_work(&id->timeout))
+			goto out;
+
 		id->scheduled_delete = 1;
 		id->rtu_timeout = rtu_timeout;
 		mod_delayed_work(cm_wq, &id->timeout, timeout);
 	}
+out:
 	spin_unlock_irqrestore(&sriov->going_down_lock, flags);
 }
 
-- 
2.43.7

Re: [PATCH] IB/mlx4: Avoid requeueing executing CM timeout work

[sashiko-bot]

> id->scheduled_delete is set by schedule_delayed() itself.  It is used
> in two cases after the RTU timeout support was added:
> 
> * A new id_map_entry is created, and a 60-second RTU timeout is
>   scheduled.
> * DREQ or REJ is received for an existing entry, and the normal
>   30-second teardown timeout is scheduled or adjusted.
> 
> The following race is possible when the RTU timeout expires while a
> DREQ or REJ is handled:
> 
>  CPU 0: timeout work                         CPU 1: DREQ/REJ handler
>  --------------------                        -----------------------
>  RTU timer expires
>  timeout callback starts
> [ ... ]
> cancel_delayed_work_sync(), because the timeout callback may be waiting
> for id_map_lock, which is held by the caller.
> 
> Fixes: fa09adf4231f ("IB/mlx4: Fix stale CM id_map entries when RTU is never received")
> Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014121344.100048-1-praveen.kannoju@oracle.com?part=1