linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Keita Suzuki <keitasuzuki.park@sslab.ics.keio.ac.jp>
To: unlisted-recipients:; (no To-header on input)
Cc: keitasuzuki.park@sslab.ics.keio.ac.jp,
	takafumi@sslab.ics.keio.ac.jp,
	Don Brace <don.brace@microsemi.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Kevin Barnett <kevin.barnett@microsemi.com>,
	Johannes Thumshirn <jthumshirn@suse.de>,
	Scott Teel <scott.teel@microsemi.com>,
	esc.storagedev@microsemi.com, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [RESEND PATCH v2] scsi: hpsa: fix memory leak in hpsa_init_one
Date: Wed, 30 Sep 2020 15:50:59 +0000	[thread overview]
Message-ID: <20200930155100.11528-1-keitasuzuki.park@sslab.ics.keio.ac.jp> (raw)

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@sslab.ics.keio.ac.jp>
---
 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


             reply	other threads:[~2020-09-30 15:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30 15:50 Keita Suzuki [this message]
2020-10-26 21:49 ` [RESEND PATCH v2] scsi: hpsa: fix memory leak in hpsa_init_one Martin K. Petersen
2020-10-27  1:22   ` Keita Suzuki
2020-10-27  7:31     ` [PATCH v3] " Keita Suzuki
2020-10-27 15:27       ` Don.Brace
2020-10-30  2:20       ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200930155100.11528-1-keitasuzuki.park@sslab.ics.keio.ac.jp \
    --to=keitasuzuki.park@sslab.ics.keio.ac.jp \
    --cc=don.brace@microsemi.com \
    --cc=esc.storagedev@microsemi.com \
    --cc=jejb@linux.ibm.com \
    --cc=jthumshirn@suse.de \
    --cc=kevin.barnett@microsemi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=scott.teel@microsemi.com \
    --cc=takafumi@sslab.ics.keio.ac.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).