diff mbox series

RDMA: Fix potential deadlock on &dev->rdi.pending_lock

Message ID 20230926102331.5095-1-dg573847474@gmail.com (mailing list archive)
State Rejected
Headers show
Series RDMA: Fix potential deadlock on &dev->rdi.pending_lock | expand

Commit Message

Chengfeng Ye Sept. 26, 2023, 10:23 a.m. UTC 
qib_7322intr() could introduce double locks on &dev->rdi.pending_lock
if it preempts other execution units requiring the same locks.

<Deadlock #1>
qib_notify_error_qp()
--> spin_lock(&dev->rdi.pending_lock)
<interrupt>
   --> qib_7322intr()
   --> qib_ib_piobufavail()
   --> spin_lock_irqsave(&dev->rdi.pending_lock)

<Deadlock #2>
qib_flush_qp_waiters()
--> spin_lock(&dev->rdi.pending_lock)
<interrupt>
   --> qib_7220intr()
   --> qib_ib_piobufavail()
   --> spin_lock_irqsave(&dev->rdi.pending_lock)

This flaw was found by an experimental static analysis tool I am
developing for irq-related deadlock.

To prevent the potential deadlock, the patch uses spin_lock_irqsave()
on &dev->rdi.pending_lock inside qib_notify_error_qp() and
qib_flush_qp_waiters() to prevent the possible deadlock scenario.

Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
---
 drivers/infiniband/hw/qib/qib_qp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Chengfeng Ye Sept. 28, 2023, 9:49 a.m. UTC | #1 
I just noticed the two callbacks already called with irq disabled,
then the patch is unnecessary, sorry for the interruption.

Regards,
Chengfeng
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/qib/qib_qp.c b/drivers/infiniband/hw/qib/qib_qp.c
index 1974ceb9d405..05a107fb6ebc 100644
--- a/drivers/infiniband/hw/qib/qib_qp.c
+++ b/drivers/infiniband/hw/qib/qib_qp.c
@@ -241,13 +241,14 @@  void qib_notify_error_qp(struct rvt_qp *qp)
 {
 	struct qib_qp_priv *priv = qp->priv;
 	struct qib_ibdev *dev = to_idev(qp->ibqp.device);
+	unsigned long flags;
 
-	spin_lock(&dev->rdi.pending_lock);
+	spin_lock_irqsave(&dev->rdi.pending_lock, flags);
 	if (!list_empty(&priv->iowait) && !(qp->s_flags & RVT_S_BUSY)) {
 		qp->s_flags &= ~RVT_S_ANY_WAIT_IO;
 		list_del_init(&priv->iowait);
 	}
-	spin_unlock(&dev->rdi.pending_lock);
+	spin_unlock_irqrestore(&dev->rdi.pending_lock, flags);
 
 	if (!(qp->s_flags & RVT_S_BUSY)) {
 		qp->s_hdrwords = 0;
@@ -367,11 +368,12 @@  void qib_flush_qp_waiters(struct rvt_qp *qp)
 {
 	struct qib_qp_priv *priv = qp->priv;
 	struct qib_ibdev *dev = to_idev(qp->ibqp.device);
+	unsigned long flags;
 
-	spin_lock(&dev->rdi.pending_lock);
+	spin_lock_irqsave(&dev->rdi.pending_lock, flags);
 	if (!list_empty(&priv->iowait))
 		list_del_init(&priv->iowait);
-	spin_unlock(&dev->rdi.pending_lock);
+	spin_unlock_irqrestore(&dev->rdi.pending_lock, flags);
 }
 
 /**