linux-kernel - [PATCH] scsi: libsas: Fix potential deadlock on &sas

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20230926110202.11589-1-dg573847474@gmail.com>
Date:   Tue, 26 Sep 2023 11:02:02 +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: libsas: Fix potential deadlock on &sas_dev->lock

Hard interrupt cq_interrupt_v1_hw() could introduce double locks on
&sas_dev->lock.

<Deadlock #1>
hisi_sas_abort_task_set()
--> hisi_sas_release_task()
--> spin_lock(&sas_dev->lock)
<interrupt>
   --> cq_interrupt_v1_hw()
   --> hisi_sas_slot_task_free()
   --> spin_lock(&sas_dev->lock)

<Deadlock #2>
hisi_sas_abort_task()
--> hisi_sas_softreset_ata_disk()
--> hisi_sas_release_task()
--> hisi_sas_do_release_task()
--> hisi_sas_slot_task_free()
--> spin_lock(&sas_dev->lock)
<interrupt>
   --> cq_interrupt_v1_hw()
   --> hisi_sas_slot_task_free()
   --> spin_lock(&sas_dev->lock)

<Deadlock #3>
hisi_sas_queue_command()
--> hisi_sas_task_deliver()
--> spin_lock(&sas_dev->lock)
<interrupt>
   --> cq_interrupt_v1_hw()
   --> hisi_sas_slot_task_free()
   --> spin_lock(&sas_dev->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_irqsave()
on &sas_dev->lock.

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

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 9472b9743aef..c2d3cc0e9e8d 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -207,6 +207,7 @@ static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
 void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 			     struct hisi_sas_slot *slot, bool need_lock)
 {
+	unsigned long flags;
 	int device_id = slot->device_id;
 	struct hisi_sas_device *sas_dev = &hisi_hba->devices[device_id];
 
@@ -240,9 +241,9 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 	}
 
 	if (need_lock) {
-		spin_lock(&sas_dev->lock);
+		spin_lock_irqsave(&sas_dev->lock, flags);
 		list_del_init(&slot->entry);
-		spin_unlock(&sas_dev->lock);
+		spin_unlock_irqrestore(&sas_dev->lock, flags);
 	} else {
 		list_del_init(&slot->entry);
 	}
@@ -403,6 +404,7 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
 	struct hisi_sas_cmd_hdr *cmd_hdr_base;
 	int dlvry_queue_slot, dlvry_queue;
 	struct sas_task *task = slot->task;
+	unsigned long flags;
 	int wr_q_index;
 
 	spin_lock(&dq->lock);
@@ -410,9 +412,9 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
 	dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
 	list_add_tail(&slot->delivery, &dq->list);
 	spin_unlock(&dq->lock);
-	spin_lock(&sas_dev->lock);
+	spin_lock_irqsave(&sas_dev->lock, flags);
 	list_add_tail(&slot->entry, &sas_dev->list);
-	spin_unlock(&sas_dev->lock);
+	spin_unlock_irqrestore(&sas_dev->lock, flags);
 
 	dlvry_queue = dq->id;
 	dlvry_queue_slot = wr_q_index;
@@ -1103,12 +1105,13 @@ static void hisi_sas_release_task(struct hisi_hba *hisi_hba,
 {
 	struct hisi_sas_slot *slot, *slot2;
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
+	unsigned long flags;
 
-	spin_lock(&sas_dev->lock);
+	spin_lock_irqsave(&sas_dev->lock, flags);
 	list_for_each_entry_safe(slot, slot2, &sas_dev->list, entry)
 		hisi_sas_do_release_task(hisi_hba, slot->task, slot, false);
 
-	spin_unlock(&sas_dev->lock);
+	spin_unlock_irqrestore(&sas_dev->lock, flags);
 }
 
 void hisi_sas_release_tasks(struct hisi_hba *hisi_hba)
-- 
2.17.1