2023-09-26 15:53:15

by Chengfeng Ye

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 <[email protected]>
---
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