public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] qla2x00t: Fix a memory leak in an error path
@ 2013-05-11 12:38 Bart Van Assche
  2013-05-17  5:00 ` Saurav Kashyap
  0 siblings, 1 reply; 3+ messages in thread
From: Bart Van Assche @ 2013-05-11 12:38 UTC (permalink / raw)
  To: Chad Dupuis, Saurav Kashyap; +Cc: linux-scsi

Avoid that the fcport structure gets leaked if
bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN, the fcport
allocation succeeds and the !vha->flags.online branch is taken.
Detected by Coverity.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_bsg.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 39719f8..af35707 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -329,7 +329,7 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
 	if (!vha->flags.online) {
 		ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
 		rval = -EIO;
-		goto done;
+		goto done_free_fcport;
 	}
 
 	req_sg_cnt =
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] qla2x00t: Fix a memory leak in an error path
  2013-05-11 12:38 [PATCH] qla2x00t: Fix a memory leak in an error path Bart Van Assche
@ 2013-05-17  5:00 ` Saurav Kashyap
  2013-05-17 12:03   ` Bart Van Assche
  0 siblings, 1 reply; 3+ messages in thread
From: Saurav Kashyap @ 2013-05-17  5:00 UTC (permalink / raw)
  To: Bart Van Assche, Chad Dupuis; +Cc: linux-scsi

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]

Hi Bart,

Instead of doing this I would move this check to the top of the function,
something like this
-----------------8<----------
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
b/drivers/scsi/qla2xxx/qla_bsg.c
index 371bb86..b5f84ae 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -255,6 +255,12 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
        int rval =  (DRIVER_ERROR << 16);
        uint16_t nextlid = 0;

 
+       if (!vha->flags.online) {
+               ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
+               rval = -EIO;
+               goto done;
+       }
+
        if (bsg_job->request->msgcode == FC_BSG_RPT_ELS) {
                rport = bsg_job->rport;
                fcport = *(fc_port_t **) rport->dd_data;
@@ -326,12 +332,6 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
                        NPH_FABRIC_CONTROLLER : NPH_F_PORT;
        }

 
-       if (!vha->flags.online) {
-               ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
-               rval = -EIO;
-               goto done;
-       }
-
        req_sg_cnt =
                dma_map_sg(&ha->pdev->dev,
bsg_job->request_payload.sg_list,
                bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);

----------------8<---------

Thanks,
~Saurav




>Avoid that the fcport structure gets leaked if
>bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN, the fcport
>allocation succeeds and the !vha->flags.online branch is taken.
>Detected by Coverity.
>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>Cc: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_bsg.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
>b/drivers/scsi/qla2xxx/qla_bsg.c
>index 39719f8..af35707 100644
>--- a/drivers/scsi/qla2xxx/qla_bsg.c
>+++ b/drivers/scsi/qla2xxx/qla_bsg.c
>@@ -329,7 +329,7 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
> 	if (!vha->flags.online) {
> 		ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
> 		rval = -EIO;
>-		goto done;
>+		goto done_free_fcport;
> 	}
> 
> 	req_sg_cnt =
>-- 
>1.7.10.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4769 bytes --]

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] qla2x00t: Fix a memory leak in an error path
  2013-05-17  5:00 ` Saurav Kashyap
@ 2013-05-17 12:03   ` Bart Van Assche
  0 siblings, 0 replies; 3+ messages in thread
From: Bart Van Assche @ 2013-05-17 12:03 UTC (permalink / raw)
  To: Saurav Kashyap; +Cc: Chad Dupuis, linux-scsi

On 05/17/13 07:00, Saurav Kashyap wrote:
> Instead of doing this I would move this check to the top of the function,
> something like this
> -----------------8<----------
> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
> b/drivers/scsi/qla2xxx/qla_bsg.c
> index 371bb86..b5f84ae 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -255,6 +255,12 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
>          int rval =  (DRIVER_ERROR << 16);
>          uint16_t nextlid = 0;
>
>
> +       if (!vha->flags.online) {
> +               ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
> +               rval = -EIO;
> +               goto done;
> +       }
> +
>          if (bsg_job->request->msgcode == FC_BSG_RPT_ELS) {
>                  rport = bsg_job->rport;
>                  fcport = *(fc_port_t **) rport->dd_data;
> @@ -326,12 +332,6 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
>                          NPH_FABRIC_CONTROLLER : NPH_F_PORT;
>          }
>
>
> -       if (!vha->flags.online) {
> -               ql_log(ql_log_warn, vha, 0x7005, "Host not online.\n");
> -               rval = -EIO;
> -               goto done;
> -       }
> -
>          req_sg_cnt =
>                  dma_map_sg(&ha->pdev->dev,
> bsg_job->request_payload.sg_list,
>                  bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);
>
> ----------------8<---------

Hello Saurav,

I will update the patch, retest and repost it. Thanks for the feedback.

Bart.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-05-17 12:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-11 12:38 [PATCH] qla2x00t: Fix a memory leak in an error path Bart Van Assche
2013-05-17  5:00 ` Saurav Kashyap
2013-05-17 12:03   ` Bart Van Assche

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox