[PATCH] scsi: hisi_sas: free IRQs before hisi_hba on remove and probe error

Fan Wu <[email protected]> Tue, 11 Aug 2026 02:21:04 +0000
Newsgroups gmane.linux.kernel.stable,gmane.linux.scsi,gmane.linux.kernel
Message-ID <[email protected]>
The v1/v2 platform backends and the v3 PCI backend register their
interrupts with devm_request_irq()/devm_request_threaded_irq(), using
hisi_hba (or &hisi_hba->phy[i] / &hisi_hba->cq[i]) as the cookie. devm
releases those IRQs after the probe/remove callback returns, during
devres teardown; the .remove path and the probe error paths reached once
IRQs are registered free the cookie before then. A handler that fires in
the window dereferences freed memory.

The v3 PCI .remove already avoided this with hisi_sas_v3_destroy_irqs();
the platform backends and the v3 probe error path did not. Record each
successfully requested IRQ in a small per-HBA ledger and release them in
reverse order from .remove and the probe error path, before
hisi_sas_free(). interrupt_init_v1_hw/v2_hw/v3_hw can fail partway and
leave earlier registrations armed, so the ledger frees only what was
requested; this also replaces hisi_sas_v3_destroy_irqs() with the common
helper.

This issue was found by an in-house static analysis tool.

Fixes: d37a00829193 ("scsi: hisi_sas: fix free'ing in probe and remove")
Fixes: 2ebde94f2ea4 ("scsi: hisi_sas: Fix up probe error handling for v3 hw")
Cc: [email protected]
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <[email protected]>
---
 drivers/scsi/hisi_sas/hisi_sas.h       | 13 +++++++++++++
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 24 ++++++++++++++++++++++++
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c |  3 +++
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c |  4 ++++
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 24 ++++++------------------
 5 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 1323ed8..8a7e220 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -30,6 +30,14 @@
 
 #define HISI_SAS_MAX_PHYS	9
 #define HISI_SAS_MAX_QUEUES	32
+
+/* Maximum number of devm-requested IRQs recorded per hisi_hba. */
+#define HISI_SAS_MAX_IRQS	(HISI_SAS_MAX_PHYS * 3 + HISI_SAS_MAX_QUEUES + 2)
+
+struct hisi_sas_irq_entry {
+	unsigned int irq;
+	void *dev_id;
+};
 #define HISI_SAS_QUEUE_SLOTS	4096
 #define HISI_SAS_MAX_ITCT_ENTRIES 1024
 #define HISI_SAS_MAX_DEVICES HISI_SAS_MAX_ITCT_ENTRIES
@@ -468,6 +476,8 @@ struct hisi_hba {
 	u32 intr_coal_count; /* Interrupt count to coalesce */
 
 	int cq_nvecs;
+	struct hisi_sas_irq_entry devm_irqs[HISI_SAS_MAX_IRQS];
+	int nr_irqs;
 
 	/* bist */
 	enum sas_linkrate debugfs_bist_linkrate;
@@ -680,6 +690,9 @@ extern void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba,
 extern void hisi_sas_init_mem(struct hisi_hba *hisi_hba);
 extern void hisi_sas_rst_work_handler(struct work_struct *work);
 extern void hisi_sas_sync_rst_work_handler(struct work_struct *work);
+void hisi_sas_track_irq(struct hisi_hba *hisi_hba, unsigned int irq,
+			void *dev_id);
+void hisi_sas_free_irqs(struct hisi_hba *hisi_hba);
 extern void hisi_sas_phy_oob_ready(struct hisi_hba *hisi_hba, int phy_no);
 extern bool hisi_sas_notify_phy_event(struct hisi_sas_phy *phy,
 				enum hisi_sas_phy_event event);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 30a9c66..fb32df1 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2621,6 +2621,7 @@ int hisi_sas_probe(struct platform_device *pdev,
 
 err_out_hw_init:
 	sas_unregister_ha(sha);
+	hisi_sas_free_irqs(hisi_hba);
 err_out_register_ha:
 	scsi_remove_host(shost);
 err_out_ha:
@@ -2630,6 +2631,27 @@ err_out_ha:
 }
 EXPORT_SYMBOL_GPL(hisi_sas_probe);
 
+/* Record a successfully requested IRQ; freed by hisi_sas_free_irqs(). */
+void hisi_sas_track_irq(struct hisi_hba *hisi_hba, unsigned int irq, void *dev_id)
+{
+	if (WARN_ON_ONCE(hisi_hba->nr_irqs >= HISI_SAS_MAX_IRQS))
+		return;
+	hisi_hba->devm_irqs[hisi_hba->nr_irqs].irq = irq;
+	hisi_hba->devm_irqs[hisi_hba->nr_irqs].dev_id = dev_id;
+	hisi_hba->nr_irqs++;
+}
+
+/* Release recorded IRQs in reverse order, before hisi_hba is freed. */
+void hisi_sas_free_irqs(struct hisi_hba *hisi_hba)
+{
+	int i;
+
+	for (i = hisi_hba->nr_irqs - 1; i >= 0; i--)
+		devm_free_irq(hisi_hba->dev, hisi_hba->devm_irqs[i].irq,
+			      hisi_hba->devm_irqs[i].dev_id);
+	hisi_hba->nr_irqs = 0;
+}
+
 void hisi_sas_remove(struct platform_device *pdev)
 {
 	struct sas_ha_struct *sha = platform_get_drvdata(pdev);
@@ -2641,6 +2663,8 @@ void hisi_sas_remove(struct platform_device *pdev)
 	sas_unregister_ha(sha);
 	sas_remove_host(shost);
 
+	hisi_sas_free_irqs(hisi_hba);
+
 	hisi_sas_free(hisi_hba);
 	scsi_host_put(shost);
 }
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index fa94d71..1cd769c 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1643,6 +1643,7 @@ static int interrupt_init_v1_hw(struct hisi_hba *hisi_hba)
 					irq, rc);
 				return rc;
 			}
+			hisi_sas_track_irq(hisi_hba, irq, phy);
 		}
 	}
 
@@ -1659,6 +1660,7 @@ static int interrupt_init_v1_hw(struct hisi_hba *hisi_hba)
 				irq, rc);
 			return rc;
 		}
+		hisi_sas_track_irq(hisi_hba, irq, &hisi_hba->cq[i]);
 	}
 
 	idx = (hisi_hba->n_phy * HISI_SAS_PHY_INT_NR) + hisi_hba->queue_count;
@@ -1674,6 +1676,7 @@ static int interrupt_init_v1_hw(struct hisi_hba *hisi_hba)
 				irq, rc);
 			return rc;
 		}
+		hisi_sas_track_irq(hisi_hba, irq, hisi_hba);
 	}
 
 	hisi_hba->cq_nvecs = hisi_hba->queue_count;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index f3516a0..57c388e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3343,6 +3343,7 @@ static int interrupt_init_v2_hw(struct hisi_hba *hisi_hba)
 			rc = -ENOENT;
 			goto err_out;
 		}
+		hisi_sas_track_irq(hisi_hba, irq, hisi_hba);
 	}
 
 	for (phy_no = 0; phy_no < hisi_hba->n_phy; phy_no++) {
@@ -3357,6 +3358,7 @@ static int interrupt_init_v2_hw(struct hisi_hba *hisi_hba)
 			rc = -ENOENT;
 			goto err_out;
 		}
+		hisi_sas_track_irq(hisi_hba, irq, phy);
 	}
 
 	for (fatal_no = 0; fatal_no < HISI_SAS_FATAL_INT_NR; fatal_no++) {
@@ -3369,6 +3371,7 @@ static int interrupt_init_v2_hw(struct hisi_hba *hisi_hba)
 			rc = -ENOENT;
 			goto err_out;
 		}
+		hisi_sas_track_irq(hisi_hba, irq, hisi_hba);
 	}
 
 	for (queue_no = 0; queue_no < hisi_hba->cq_nvecs; queue_no++) {
@@ -3385,6 +3388,7 @@ static int interrupt_init_v2_hw(struct hisi_hba *hisi_hba)
 			rc = -ENOENT;
 			goto err_out;
 		}
+		hisi_sas_track_irq(hisi_hba, cq->irq_no, cq);
 		cq->irq_mask = irq_get_affinity_mask(cq->irq_no);
 	}
 err_out:
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 2f9e017..512ba17 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2635,6 +2635,7 @@ static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
 		dev_err(dev, "could not request phy interrupt, rc=%d\n", rc);
 		return -ENOENT;
 	}
+	hisi_sas_track_irq(hisi_hba, pci_irq_vector(pdev, IRQ_PHY_UP_DOWN_INDEX), hisi_hba);
 
 	rc = devm_request_irq(dev, pci_irq_vector(pdev, IRQ_CHL_INDEX),
 			      int_chnl_int_v3_hw, 0,
@@ -2643,6 +2644,7 @@ static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
 		dev_err(dev, "could not request chnl interrupt, rc=%d\n", rc);
 		return -ENOENT;
 	}
+	hisi_sas_track_irq(hisi_hba, pci_irq_vector(pdev, IRQ_CHL_INDEX), hisi_hba);
 
 	rc = devm_request_irq(dev, pci_irq_vector(pdev, IRQ_AXI_INDEX),
 			      fatal_axi_int_v3_hw, 0,
@@ -2651,6 +2653,7 @@ static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
 		dev_err(dev, "could not request fatal interrupt, rc=%d\n", rc);
 		return -ENOENT;
 	}
+	hisi_sas_track_irq(hisi_hba, pci_irq_vector(pdev, IRQ_AXI_INDEX), hisi_hba);
 
 	if (hisi_sas_intr_conv)
 		dev_info(dev, "Enable interrupt converge\n");
@@ -2673,6 +2676,7 @@ static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
 				i, rc);
 			return -ENOENT;
 		}
+		hisi_sas_track_irq(hisi_hba, cq->irq_no, cq);
 		cq->irq_mask = pci_irq_get_affinity(pdev, i + BASE_VECTORS_V3_HW);
 		if (!cq->irq_mask) {
 			dev_err(dev, "could not get cq%d irq affinity!\n", i);
@@ -5058,6 +5062,7 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 err_out_unregister_ha:
 	sas_unregister_ha(sha);
+	hisi_sas_free_irqs(hisi_hba);
 err_out_remove_host:
 	scsi_remove_host(shost);
 err_out_free_host:
@@ -5067,23 +5072,6 @@ err_out:
 	return rc;
 }
 
-static void
-hisi_sas_v3_destroy_irqs(struct pci_dev *pdev, struct hisi_hba *hisi_hba)
-{
-	int i;
-
-	devm_free_irq(&pdev->dev, pci_irq_vector(pdev, IRQ_PHY_UP_DOWN_INDEX), hisi_hba);
-	devm_free_irq(&pdev->dev, pci_irq_vector(pdev, IRQ_CHL_INDEX), hisi_hba);
-	devm_free_irq(&pdev->dev, pci_irq_vector(pdev, IRQ_AXI_INDEX), hisi_hba);
-	for (i = 0; i < hisi_hba->cq_nvecs; i++) {
-		struct hisi_sas_cq *cq = &hisi_hba->cq[i];
-		int nr = hisi_sas_intr_conv ? BASE_VECTORS_V3_HW :
-					      BASE_VECTORS_V3_HW + i;
-
-		devm_free_irq(&pdev->dev, pci_irq_vector(pdev, nr), cq);
-	}
-}
-
 static void hisi_sas_v3_remove(struct pci_dev *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -5099,7 +5087,7 @@ static void hisi_sas_v3_remove(struct pci_dev *pdev)
 	flush_workqueue(hisi_hba->wq);
 	sas_remove_host(shost);
 
-	hisi_sas_v3_destroy_irqs(pdev, hisi_hba);
+	hisi_sas_free_irqs(hisi_hba);
 	hisi_sas_free(hisi_hba);
 	scsi_host_put(shost);
 }