diff mbox series

[RESEND] qla2xxx: Fix gnl.l memory leak on adapter init failure

Message ID 1565792681-9641-1-git-send-email-William.Kuzeja@stratus.com (mailing list archive)
State Mainlined
Commit 26fa656e9a0cbccddf7db132ea020d2169dbe46e
Headers show
Series [RESEND] qla2xxx: Fix gnl.l memory leak on adapter init failure | expand

Commit Message

Bill Kuzeja Aug. 14, 2019, 2:24 p.m. UTC
If HBA initialization fails unexpectedly (exiting via probe_failed:), we 
may fail to free vha->gnl.l. So that we don't attempt to double free,
set this pointer to NULL after a free and check for NULL at probe_failed:
so we know whether or not to call dma_free_coherent. 

Signed-off-by: Bill Kuzeja <william.kuzeja@stratus.com>
---
 drivers/scsi/qla2xxx/qla_attr.c |  2 ++
 drivers/scsi/qla2xxx/qla_os.c   | 11 ++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Himanshu Madhani Aug. 14, 2019, 5:37 p.m. UTC | #1
On 8/14/19, 9:25 AM, "linux-scsi-owner@vger.kernel.org on behalf of Bill Kuzeja" <linux-scsi-owner@vger.kernel.org on behalf of William.Kuzeja@stratus.com> wrote:

    If HBA initialization fails unexpectedly (exiting via probe_failed:), we 
    may fail to free vha->gnl.l. So that we don't attempt to double free,
    set this pointer to NULL after a free and check for NULL at probe_failed:
    so we know whether or not to call dma_free_coherent. 
    
    Signed-off-by: Bill Kuzeja <william.kuzeja@stratus.com>
    ---
     drivers/scsi/qla2xxx/qla_attr.c |  2 ++
     drivers/scsi/qla2xxx/qla_os.c   | 11 ++++++++++-
     2 files changed, 12 insertions(+), 1 deletion(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
    index 8d560c5..6b7b390 100644
    --- a/drivers/scsi/qla2xxx/qla_attr.c
    +++ b/drivers/scsi/qla2xxx/qla_attr.c
    @@ -2956,6 +2956,8 @@ void qla_insert_tgt_attrs(void)
     	dma_free_coherent(&ha->pdev->dev, vha->gnl.size, vha->gnl.l,
     	    vha->gnl.ldma);
     
    +	vha->gnl.l = NULL;
    +
     	vfree(vha->scan.l);
     
     	if (vha->qpair && vha->qpair->vp_idx == vha->vp_idx) {
    diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
    index 2e58cff..98e60a3 100644
    --- a/drivers/scsi/qla2xxx/qla_os.c
    +++ b/drivers/scsi/qla2xxx/qla_os.c
    @@ -3440,6 +3440,12 @@ static void qla2x00_iocb_work_fn(struct work_struct *work)
     	return 0;
     
     probe_failed:
    +	if (base_vha->gnl.l) {
    +		dma_free_coherent(&ha->pdev->dev, base_vha->gnl.size,
    +				base_vha->gnl.l, base_vha->gnl.ldma);
    +		base_vha->gnl.l = NULL;
    +	}
    +
     	if (base_vha->timer_active)
     		qla2x00_stop_timer(base_vha);
     	base_vha->flags.online = 0;
    @@ -3673,7 +3679,7 @@ static void qla2x00_iocb_work_fn(struct work_struct *work)
     	if (!atomic_read(&pdev->enable_cnt)) {
     		dma_free_coherent(&ha->pdev->dev, base_vha->gnl.size,
     		    base_vha->gnl.l, base_vha->gnl.ldma);
    -
    +		base_vha->gnl.l = NULL;
     		scsi_host_put(base_vha->host);
     		kfree(ha);
     		pci_set_drvdata(pdev, NULL);
    @@ -3713,6 +3719,8 @@ static void qla2x00_iocb_work_fn(struct work_struct *work)
     	dma_free_coherent(&ha->pdev->dev,
     		base_vha->gnl.size, base_vha->gnl.l, base_vha->gnl.ldma);
     
    +	base_vha->gnl.l = NULL;
    +
     	vfree(base_vha->scan.l);
     
     	if (IS_QLAFX00(ha))
    @@ -4816,6 +4824,7 @@ struct scsi_qla_host *qla2x00_create_host(struct scsi_host_template *sht,
     		    "Alloc failed for scan database.\n");
     		dma_free_coherent(&ha->pdev->dev, vha->gnl.size,
     		    vha->gnl.l, vha->gnl.ldma);
    +		vha->gnl.l = NULL;
     		scsi_remove_host(vha->host);
     		return NULL;
     	}
    -- 
    1.8.3.1

Thanks for resend Bill
    
Patch Looks Good. 

Acked-by: Himanshu Madhani <hmadhani@marvell.com>
Martin K. Petersen Aug. 15, 2019, 1:51 a.m. UTC | #2
Bill,

> If HBA initialization fails unexpectedly (exiting via probe_failed:),
> we may fail to free vha->gnl.l. So that we don't attempt to double
> free, set this pointer to NULL after a free and check for NULL at
> probe_failed: so we know whether or not to call dma_free_coherent.

Applied to 5.3/scsi-fixes, thanks!
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 8d560c5..6b7b390 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -2956,6 +2956,8 @@  void qla_insert_tgt_attrs(void)
 	dma_free_coherent(&ha->pdev->dev, vha->gnl.size, vha->gnl.l,
 	    vha->gnl.ldma);
 
+	vha->gnl.l = NULL;
+
 	vfree(vha->scan.l);
 
 	if (vha->qpair && vha->qpair->vp_idx == vha->vp_idx) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 2e58cff..98e60a3 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3440,6 +3440,12 @@  static void qla2x00_iocb_work_fn(struct work_struct *work)
 	return 0;
 
 probe_failed:
+	if (base_vha->gnl.l) {
+		dma_free_coherent(&ha->pdev->dev, base_vha->gnl.size,
+				base_vha->gnl.l, base_vha->gnl.ldma);
+		base_vha->gnl.l = NULL;
+	}
+
 	if (base_vha->timer_active)
 		qla2x00_stop_timer(base_vha);
 	base_vha->flags.online = 0;
@@ -3673,7 +3679,7 @@  static void qla2x00_iocb_work_fn(struct work_struct *work)
 	if (!atomic_read(&pdev->enable_cnt)) {
 		dma_free_coherent(&ha->pdev->dev, base_vha->gnl.size,
 		    base_vha->gnl.l, base_vha->gnl.ldma);
-
+		base_vha->gnl.l = NULL;
 		scsi_host_put(base_vha->host);
 		kfree(ha);
 		pci_set_drvdata(pdev, NULL);
@@ -3713,6 +3719,8 @@  static void qla2x00_iocb_work_fn(struct work_struct *work)
 	dma_free_coherent(&ha->pdev->dev,
 		base_vha->gnl.size, base_vha->gnl.l, base_vha->gnl.ldma);
 
+	base_vha->gnl.l = NULL;
+
 	vfree(base_vha->scan.l);
 
 	if (IS_QLAFX00(ha))
@@ -4816,6 +4824,7 @@  struct scsi_qla_host *qla2x00_create_host(struct scsi_host_template *sht,
 		    "Alloc failed for scan database.\n");
 		dma_free_coherent(&ha->pdev->dev, vha->gnl.size,
 		    vha->gnl.l, vha->gnl.ldma);
+		vha->gnl.l = NULL;
 		scsi_remove_host(vha->host);
 		return NULL;
 	}