linux-kernel - [PATCH] drbd: Fix potential deadlock on &session->back_lock and &session->frwd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20230926110018.11344-1-dg573847474@gmail.com>
Date:   Tue, 26 Sep 2023 11:00:18 +0000
From:   Chengfeng Ye <dg573847474@...il.com>
To:     njavali@...vell.com, mrangankar@...vell.com,
        GR-QLogic-Storage-Upstream@...vell.com, jejb@...ux.ibm.com,
        martin.petersen@...cle.com
Cc:     linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        Chengfeng Ye <dg573847474@...il.com>
Subject: [PATCH] drbd: Fix potential deadlock on &session->back_lock and &session->frwd_lock

Timer interrupt could introduce inverse lock ordering between
&session->frwd_lock and &session->back_lock if qedi_tmf_resp_work()
is interrupted by timer.

<Deadlock #1>

          [CPU0]                            |          [CPU1]
qedi_tmf_resp_work()                        |
--> spin_lock(&session->back_lock)          |
                                            | iscsi_eh_cmd_timed_out()
                                            | --> spin_lock_bh(&session->frwd_lock)
                                            |
                                            | --> spin_lock(&session->back_lock)
<timer interrupt>                           |
   --> iscsi_tmf_timedout()                 |
   --> spin_lock(&session->frwd_lock)       |

<Deadlock #2>

          [CPU0]                            |          [CPU1]
qedi_tmf_resp_work()                        |
--> __iscsi_complete_pdu()                  |
--> spin_lock(&conn->session->frwd_lock)    |
                                            | qedi_process_async_mesg()
                                            | --> spin_lock_bh(&session->back_lock)
                                            |
                                            | --> __iscsi_complete_pdu()
                                            | --> spin_lock(&conn->session->frwd_lock)
<timer interrupt>                           |
   --> iscsi_check_transport_timeouts()     |
   --> iscsi_send_nopout()                  |
   --> iscsi_alloc_mgmt_task()              |
   --> iscsi_put_task()                     |
   --> spin_lock_bh(&session->back_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 use spin_lock_bh()
on &session->back_lock inside qedi_tmf_resp_work() to prevent the
timer interrupts.

Signed-off-by: Chengfeng Ye <dg573847474@...il.com>
---
 drivers/scsi/qedi/qedi_fw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
index 6901738324da..60a168ca178c 100644
--- a/drivers/scsi/qedi/qedi_fw.c
+++ b/drivers/scsi/qedi/qedi_fw.c
@@ -162,9 +162,9 @@ static void qedi_tmf_resp_work(struct work_struct *work)
 	if (rval)
 		goto exit_tmf_resp;
 
-	spin_lock(&session->back_lock);
+	spin_lock_bh(&session->back_lock);
 	__iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr_ptr, NULL, 0);
-	spin_unlock(&session->back_lock);
+	spin_unlock_bh(&session->back_lock);
 
 exit_tmf_resp:
 	kfree(resp_hdr_ptr);
-- 
2.17.1