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>] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 28 Jun 2023 15:30:10 +0000
From:   Chengfeng Ye <dg573847474@...il.com>
To:     chenxiang66@...ilicon.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] scsi: hisi_sas: Fix potential deadlock on &hisi_hba->lock

As &hisi_hba->lock is acquired by hard irq int_abnormal_v1_hw(),
other acquisition of the same lock under process context should
disable irq, otherwise deadlock could happen if the
irq preempt the execution while the lock is held in process context
on the same CPU.

[Interrupt]: int_abnormal_v1_hw()
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c:1447
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:2050
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:1079
    -->spin_lock_irqsave(&hisi_hba->lock, flags);

[Process Context]: hisi_sas_clear_nexus_ha()
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:1932
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:1135
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:1116
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:1105
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:166
    -->spin_lock(&hisi_hba->lock);

[Process Context]: hisi_sas_dev_found()
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:665
    -->spin_lock(&hisi_hba->lock);

[Process Context]: hisi_sas_queue_command()
    -->/root/linux/drivers/scsi/hisi_sas/hisi_sas_main.c:188
    -->spin_lock(&hisi_hba->lock);

This flaw was found by an experimental static analysis tool I am
developing for irq-related deadlock, which reported the above
warning when analyzing the linux kernel 6.4-rc7 release.

The tentative patch fix the potential deadlock by spin_lock_irqsave().

Signed-off-by: Chengfeng Ye <dg573847474@...il.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 412431c901a7..47c5062a732e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -161,11 +161,12 @@ static void hisi_sas_slot_index_clear(struct hisi_hba *hisi_hba, int slot_idx)
 
 static void hisi_sas_slot_index_free(struct hisi_hba *hisi_hba, int slot_idx)
 {
+	unsigned long flags;
 	if (hisi_hba->hw->slot_index_alloc ||
 	    slot_idx < HISI_SAS_RESERVED_IPTT) {
-		spin_lock(&hisi_hba->lock);
+		spin_lock_irqsave(&hisi_hba->lock, flags);
 		hisi_sas_slot_index_clear(hisi_hba, slot_idx);
-		spin_unlock(&hisi_hba->lock);
+		spin_unlock_irqrestore(&hisi_hba->lock, flags);
 	}
 }
 
@@ -181,11 +182,12 @@ static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
 {
 	int index;
 	void *bitmap = hisi_hba->slot_index_tags;
+	unsigned long flags;
 
 	if (rq)
 		return rq->tag + HISI_SAS_RESERVED_IPTT;
 
-	spin_lock(&hisi_hba->lock);
+	spin_lock_irqsave(&hisi_hba->lock, flags);
 	index = find_next_zero_bit(bitmap, HISI_SAS_RESERVED_IPTT,
 				   hisi_hba->last_slot_index + 1);
 	if (index >= HISI_SAS_RESERVED_IPTT) {
@@ -193,13 +195,13 @@ static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
 				HISI_SAS_RESERVED_IPTT,
 				0);
 		if (index >= HISI_SAS_RESERVED_IPTT) {
-			spin_unlock(&hisi_hba->lock);
+			spin_unlock_irqrestore(&hisi_hba->lock, flags);
 			return -SAS_QUEUE_FULL;
 		}
 	}
 	hisi_sas_slot_index_set(hisi_hba, index);
 	hisi_hba->last_slot_index = index;
-	spin_unlock(&hisi_hba->lock);
+	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	return index;
 }
@@ -658,11 +660,12 @@ static struct hisi_sas_device *hisi_sas_alloc_dev(struct domain_device *device)
 {
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct hisi_sas_device *sas_dev = NULL;
+	unsigned long flags;
 	int last = hisi_hba->last_dev_id;
 	int first = (hisi_hba->last_dev_id + 1) % HISI_SAS_MAX_DEVICES;
 	int i;
 
-	spin_lock(&hisi_hba->lock);
+	spin_lock_irqsave(&hisi_hba->lock, flags);
 	for (i = first; i != last; i %= HISI_SAS_MAX_DEVICES) {
 		if (hisi_hba->devices[i].dev_type == SAS_PHY_UNUSED) {
 			int queue = i % hisi_hba->queue_count;
@@ -682,7 +685,7 @@ static struct hisi_sas_device *hisi_sas_alloc_dev(struct domain_device *device)
 		i++;
 	}
 	hisi_hba->last_dev_id = i;
-	spin_unlock(&hisi_hba->lock);
+	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	return sas_dev;
 }
-- 
2.17.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ