[PATCH] 53c700: fix memory leak of dma non-cosistent memory

[PATCH] 53c700: fix memory leak of dma non-cosistent memory

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

In NCR_700_detect function, if an error occurs it will return
NULL without freeing the dma non-cosistent memory once allocated.
This patch avoid this.

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 drivers/scsi/53c700.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 95e32a4..d5a2ba3 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -332,8 +332,10 @@ struct Scsi_Host *
 		tpnt->proc_name = "53c700";
 
 	host = scsi_host_alloc(tpnt, 4);
-	if (!host)
+	if (!host) {
+		dma_free_noncoherent(hostdata->dev, TOTAL_MEM_SIZE, memory, pScript);
 		return NULL;
+	}
 	memset(hostdata->slots, 0, sizeof(struct NCR_700_command_slot)
 	       * NCR_700_COMMAND_SLOTS_PER_HOST);
 	for (j = 0; j < NCR_700_COMMAND_SLOTS_PER_HOST; j++) {
@@ -394,6 +396,7 @@ struct Scsi_Host *
 
 	if (scsi_add_host(host, dev)) {
 		dev_printk(KERN_ERR, dev, "53c700: scsi_add_host failed\n");
+		dma_free_noncoherent(hostdata->dev, TOTAL_MEM_SIZE, memory, pScript);
 		scsi_host_put(host);
 		return NULL;
 	}
-- 
1.8.3.1

Re: [PATCH] 53c700: fix memory leak of dma non-cosistent memory

[Martin K. Petersen]

>>>>> "Li" == Li Qiang <liq3ea@gmail.com> writes:

Li> In NCR_700_detect function, if an error occurs it will return NULL
Li> without freeing the dma non-cosistent memory once allocated.  This
Li> patch avoid this.

James: Please review!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] 53c700: fix memory leak of dma non-cosistent memory

[James Bottomley]

On Mon, 2016-11-14 at 15:07 +0800, Li Qiang wrote:
> From: Li Qiang <liq3ea@gmail.com>
> 
> In NCR_700_detect function, if an error occurs it will return
> NULL without freeing the dma non-cosistent memory once allocated.
> This patch avoid this.
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>

There's not a lot of point doing this, since the memory leak is so tiny
and the failures would be instantly noticeable.  However:

> ---
>  drivers/scsi/53c700.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
> index 95e32a4..d5a2ba3 100644
> --- a/drivers/scsi/53c700.c
> +++ b/drivers/scsi/53c700.c
> @@ -332,8 +332,10 @@ struct Scsi_Host *
>  		tpnt->proc_name = "53c700";
>  
>  	host = scsi_host_alloc(tpnt, 4);
> -	if (!host)
> +	if (!host) {
> +		dma_free_noncoherent(hostdata->dev, TOTAL_MEM_SIZE,
> memory, pScript);
>  		return NULL;
> +	}

This you could do.

>  	memset(hostdata->slots, 0, sizeof(struct
> NCR_700_command_slot)
>  	       * NCR_700_COMMAND_SLOTS_PER_HOST);
>  	for (j = 0; j < NCR_700_COMMAND_SLOTS_PER_HOST; j++) {
> @@ -394,6 +396,7 @@ struct Scsi_Host *
>  
>  	if (scsi_add_host(host, dev)) {
>  		dev_printk(KERN_ERR, dev, "53c700: scsi_add_host
> failed\n");
> +		dma_free_noncoherent(hostdata->dev, TOTAL_MEM_SIZE,
> memory, pScript);
>  		scsi_host_put(host);
>  		return NULL;
>  	}

This I'm not sure about; I'd have to dig more deeply into how the
driver works.  If the scripts engine is already started, you can't just
free the memory it's running from without actually stopping it,
otherwise nasty things may happen if something else reuses the memory. 
 I have a vague feeling that this driver is activation driven rather
than mailbox driven, so it might be OK, but it needs checking.

James