[PATCH] RDMA/cma: add rdma_destroy_id_wq() and use it from RDS

[PATCH] RDMA/cma: add rdma_destroy_id_wq() and use it from RDS

[Aibing Chen]

syzbot reported a possible circular locking dependency between
rds_trans_sem and the RDMA CM handler mutex:

  CPU0 (bind path)                    CPU1 (CM callback path)
  ----                                ----
  down_read(&rds_trans_sem)           mutex_lock(&handler_mutex)
  rds_ib_laddr_check()                cma_ib_req_handler()
    rdma_destroy_id()                   rds_ib_cm_handle_connect()
      mutex_lock(&handler_mutex)          rds_conn_create()
                                            rds_trans_get_preferred()
                                              down_read(&rds_trans_sem)

rds_ib_laddr_check_cm() destroys the CM id from inside
rds_trans_get_preferred()'s read-side critical section, while CM
callbacks create RDS connections (taking the same read lock) from
under handler_mutex.

Break the cycle by destroying the CM id asynchronously: add
rdma_destroy_id_wq(), which queues the destruction on cma_wq, and
switch rds_ib_laddr_check_cm() to it.  The destroy work is embedded
in struct rdma_id_private, which keeps the id alive until the work
runs; rds does not touch cm_id after the destroy call.

This formalizes the approach proposed and syzbot-tested by Hillf
Danton; Eduard Zateev additionally verified the fix on a real QEMU
system (KASAN + lockdep + RDMA_RXE, active and passive RDS
connections established without any lockdep output).

Reported-by: syzbot+ccdb5058c3df2d94811c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ccdb5058c3df2d94811c
Suggested-by: Hillf Danton <hdanton@sina.com>
Tested-by: syzbot+ccdb5058c3df2d94811c@syzkaller.appspotmail.com
Tested-by: Eduard Zateev <hackerowskiy@gmail.com>
Signed-off-by: Aibing Chen <chenaibing@cmss.chinamobile.com>
---
 drivers/infiniband/core/cma.c      | 19 +++++++++++++++++++
 drivers/infiniband/core/cma_priv.h |  1 +
 include/rdma/rdma_cm.h             |  1 +
 net/rds/ib.c                       |  2 +-
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 337a49d1a..d10aaf51e 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2126,6 +2126,25 @@ void rdma_destroy_id(struct rdma_cm_id *id)
 }
 EXPORT_SYMBOL(rdma_destroy_id);
 
+static void destroy_id_work(struct work_struct *work)
+{
+	struct rdma_id_private *id_priv =
+		container_of(work, struct rdma_id_private, destroy_work);
+
+	mutex_lock(&id_priv->handler_mutex);
+	destroy_id_handler_unlock(id_priv);
+}
+
+void rdma_destroy_id_wq(struct rdma_cm_id *id)
+{
+	struct rdma_id_private *id_priv =
+		container_of(id, struct rdma_id_private, id);
+
+	INIT_WORK(&id_priv->destroy_work, destroy_id_work);
+	queue_work(cma_wq, &id_priv->destroy_work);
+}
+EXPORT_SYMBOL(rdma_destroy_id_wq);
+
 static int cma_rep_recv(struct rdma_id_private *id_priv)
 {
 	int ret;
diff --git a/drivers/infiniband/core/cma_priv.h b/drivers/infiniband/core/cma_priv.h
index 04332eb82..95ff8bbd4 100644
--- a/drivers/infiniband/core/cma_priv.h
+++ b/drivers/infiniband/core/cma_priv.h
@@ -110,6 +110,7 @@ struct rdma_id_private {
 	 */
 	struct rdma_restrack_entry     res;
 	struct rdma_ucm_ece ece;
+	struct work_struct destroy_work;
 };
 
 #if IS_ENABLED(CONFIG_INFINIBAND_ADDR_TRANS_CONFIGFS)
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index d639ff889..ac1c1d917 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -167,6 +167,7 @@ struct rdma_cm_id *rdma_create_user_id(rdma_cm_event_handler event_handler,
   * asynchronous operations associated with the id.
   */
 void rdma_destroy_id(struct rdma_cm_id *id);
+void rdma_destroy_id_wq(struct rdma_cm_id *id);
 
 /**
  * rdma_restrict_node_type - Restrict an RDMA identifier to specific
diff --git a/net/rds/ib.c b/net/rds/ib.c
index 4ea9838d0..1290747cd 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -484,7 +484,7 @@ static int rds_ib_laddr_check_cm(struct net *net, const struct in6_addr *addr,
 		 cm_id->device ? cm_id->device->node_type : -1);
 
 out:
-	rdma_destroy_id(cm_id);
+	rdma_destroy_id_wq(cm_id);
 
 	return ret;
 }
-- 
2.50.1.windows.1