[PATCH v2] scsi: hpsa: fix memory leak in hpsa_init_one

From: Keita Suzuki
Date: Tue Sep 08 2020 - 15:25:22 EST


When hpsa_scsi_add_host fails, h->lastlogicals is leaked since it lacks
free in the error handler.

Fix this by adding free when hpsa_scsi_add_host fails.

This patch also renames the numbered labels to detailed names.

Fixes: cf47723763a7 ("hpsa: correct initialization order issue")
Signed-off-by: Keita Suzuki <keitasuzuki.park@xxxxxxxxxxxxxxxxxxxx>
---
drivers/scsi/hpsa.c | 52 +++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 48d5da59262b..4911ca22efe4 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -8691,19 +8691,19 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (!h->lockup_detected) {
dev_err(&h->pdev->dev, "Failed to allocate lockup detector\n");
rc = -ENOMEM;
- goto clean1; /* aer/h */
+ goto out_destroy_workqueue; /* aer/h */
}
set_lockup_detected_for_all_cpus(h, 0);

rc = hpsa_pci_init(h);
if (rc)
- goto clean2; /* lu, aer/h */
+ goto out_free_lockup_detected; /* lu, aer/h */

/* relies on h-> settings made by hpsa_pci_init, including
* interrupt_mode h->intr */
rc = hpsa_scsi_host_alloc(h);
if (rc)
- goto clean2_5; /* pci, lu, aer/h */
+ goto out_free_pci_init; /* pci, lu, aer/h */

sprintf(h->devname, HPSA "%d", h->scsi_host->host_no);
h->ctlr = number_of_controllers;
@@ -8719,7 +8719,7 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dac = 0;
} else {
dev_err(&pdev->dev, "no suitable DMA available\n");
- goto clean3; /* shost, pci, lu, aer/h */
+ goto out_scsi_host_put; /* shost, pci, lu, aer/h */
}
}

@@ -8728,13 +8728,13 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)

rc = hpsa_request_irqs(h, do_hpsa_intr_msi, do_hpsa_intr_intx);
if (rc)
- goto clean3; /* shost, pci, lu, aer/h */
+ goto out_scsi_host_put; /* shost, pci, lu, aer/h */
rc = hpsa_alloc_cmd_pool(h);
if (rc)
- goto clean4; /* irq, shost, pci, lu, aer/h */
+ goto out_free_irqs; /* irq, shost, pci, lu, aer/h */
rc = hpsa_alloc_sg_chain_blocks(h);
if (rc)
- goto clean5; /* cmd, irq, shost, pci, lu, aer/h */
+ goto out_free_cmd_pool; /* cmd, irq, shost, pci, lu, aer/h */
init_waitqueue_head(&h->scan_wait_queue);
init_waitqueue_head(&h->event_sync_wait_queue);
mutex_init(&h->reset_mutex);
@@ -8747,25 +8747,25 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
spin_lock_init(&h->devlock);
rc = hpsa_put_ctlr_into_performant_mode(h);
if (rc)
- goto clean6; /* sg, cmd, irq, shost, pci, lu, aer/h */
+ goto out_free_sg_chain_blocks; /* sg, cmd, irq, shost, pci, lu, aer/h */

/* create the resubmit workqueue */
h->rescan_ctlr_wq = hpsa_create_controller_wq(h, "rescan");
if (!h->rescan_ctlr_wq) {
rc = -ENOMEM;
- goto clean7;
+ goto out_free_performant_mode;
}

h->resubmit_wq = hpsa_create_controller_wq(h, "resubmit");
if (!h->resubmit_wq) {
rc = -ENOMEM;
- goto clean7; /* aer/h */
+ goto out_free_performant_mode; /* aer/h */
}

h->monitor_ctlr_wq = hpsa_create_controller_wq(h, "monitor");
if (!h->monitor_ctlr_wq) {
rc = -ENOMEM;
- goto clean7;
+ goto out_free_performant_mode;
}

/*
@@ -8795,20 +8795,20 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
* cannot goto clean7 or free_irqs will be called
* again. Instead, do its work
*/
- hpsa_free_performant_mode(h); /* clean7 */
- hpsa_free_sg_chain_blocks(h); /* clean6 */
- hpsa_free_cmd_pool(h); /* clean5 */
+ hpsa_free_performant_mode(h); /* out_free_performant_mode */
+ hpsa_free_sg_chain_blocks(h); /* out_free_sg_chain_blocks */
+ hpsa_free_cmd_pool(h); /* out_free_cmd_pool */
/*
* skip hpsa_free_irqs(h) clean4 since that
* was just called before request_irqs failed
*/
- goto clean3;
+ goto out_scsi_host_put;
}

rc = hpsa_kdump_soft_reset(h);
if (rc)
/* Neither hard nor soft reset worked, we're hosed. */
- goto clean7;
+ goto out_free_performant_mode;

dev_info(&h->pdev->dev, "Board READY.\n");
dev_info(&h->pdev->dev,
@@ -8854,7 +8854,7 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* hook into SCSI subsystem */
rc = hpsa_scsi_add_host(h);
if (rc)
- goto clean7; /* perf, sg, cmd, irq, shost, pci, lu, aer/h */
+ goto out_free_lastlogicals; /* ll, perf, sg, cmd, irq, shost, pci, lu, aer/h */

/* Monitor the controller for firmware lockups */
h->heartbeat_sample_interval = HEARTBEAT_SAMPLE_INTERVAL;
@@ -8869,26 +8869,28 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
HPSA_EVENT_MONITOR_INTERVAL);
return 0;

-clean7: /* perf, sg, cmd, irq, shost, pci, lu, aer/h */
+out_free_lastlogicals: /* ll, perf, sg, cmd, irq, shost, pci, lu, aer/h */
+ kfree(h->lastlogicals);
+out_free_performant_mode: /* perf, sg, cmd, irq, shost, pci, lu, aer/h */
hpsa_free_performant_mode(h);
h->access.set_intr_mask(h, HPSA_INTR_OFF);
-clean6: /* sg, cmd, irq, pci, lockup, wq/aer/h */
+out_free_sg_chain_blocks: /* sg, cmd, irq, pci, lockup, wq/aer/h */
hpsa_free_sg_chain_blocks(h);
-clean5: /* cmd, irq, shost, pci, lu, aer/h */
+out_free_cmd_pool: /* cmd, irq, shost, pci, lu, aer/h */
hpsa_free_cmd_pool(h);
-clean4: /* irq, shost, pci, lu, aer/h */
+out_free_irqs: /* irq, shost, pci, lu, aer/h */
hpsa_free_irqs(h);
-clean3: /* shost, pci, lu, aer/h */
+out_scsi_host_put: /* shost, pci, lu, aer/h */
scsi_host_put(h->scsi_host);
h->scsi_host = NULL;
-clean2_5: /* pci, lu, aer/h */
+out_free_pci_init: /* pci, lu, aer/h */
hpsa_free_pci_init(h);
-clean2: /* lu, aer/h */
+out_free_lockup_detected: /* lu, aer/h */
if (h->lockup_detected) {
free_percpu(h->lockup_detected);
h->lockup_detected = NULL;
}
-clean1: /* wq/aer/h */
+out_destroy_workqueue: /* wq/aer/h */
if (h->resubmit_wq) {
destroy_workqueue(h->resubmit_wq);
h->resubmit_wq = NULL;
--
2.17.1