All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: linux-scsi <linux-scsi@vger.kernel.org>
Cc: Chad Dupuis <chad.dupuis@qlogic.com>,
	Saurav Kashyap <saurav.kashyap@qlogic.com>
Subject: [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els()
Date: Wed, 05 Jun 2013 15:09:59 +0200	[thread overview]
Message-ID: <51AF38A7.6020808@acm.org> (raw)
In-Reply-To: <51AF36BF.3030602@acm.org>

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.
This was detected by Coverity. However, Coverity does not recognize
that all qla2x00_process_els() callers specify either
FC_BSG_RPT_ELS or FC_BSG_HST_ELS_NOLOGIN in the field
bsg_job->request->msgcode and that the value of that field is not
modified inside that function. This results in a false positive
report about a possible memory leak in an error path for
bsg_job->request->msgcode values other than the two mentioned
values.  Make it easy for Coverity (and for humans) to recognize
that there is no fcport leak in the error path by changing the
bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN test into
bsg_job->request->msgcode != FC_BSG_RPT_ELS.

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 |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index cf07491..f8a2634 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);
@@ -399,7 +399,7 @@ done_unmap_sg:
 	goto done_free_fcport;
 
 done_free_fcport:
-	if (bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN)
+	if (bsg_job->request->msgcode != FC_BSG_RPT_ELS)
 		kfree(fcport);
 done:
 	return rval;
-- 
1.7.10.4


  parent reply	other threads:[~2013-06-05 13:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-05 13:01 [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Bart Van Assche
2013-06-05 13:02 ` [PATCH 01/10] qla2xxx: Clean up qla24xx_iidma() Bart Van Assche
2013-06-12  8:05   ` Saurav Kashyap
2013-06-12 18:59     ` James Bottomley
2013-06-13  5:20       ` Saurav Kashyap
2013-06-05 13:03 ` [PATCH 02/10] qla2xxx: Clean up qla84xx_mgmt_cmd() Bart Van Assche
2013-06-12  8:06   ` Saurav Kashyap
2013-06-05 13:04 ` [PATCH 03/10] qla2xxx: Remove dead code in qla2x00_configure_hba() Bart Van Assche
2013-06-12  8:08   ` Saurav Kashyap
2013-06-05 13:05 ` [PATCH 04/10] qla2xxx: Remove two superfluous tests Bart Van Assche
2013-06-12  8:08   ` Saurav Kashyap
2013-06-05 13:06 ` [PATCH 05/10] qla2xxx: Remove a dead assignment in qla24xx_build_scsi_crc_2_iocbs() Bart Van Assche
2013-06-12  8:08   ` Saurav Kashyap
2013-06-05 13:06 ` [PATCH 06/10] qla2xxx: Remove redundant assignments Bart Van Assche
2013-06-12  8:09   ` Saurav Kashyap
2013-06-05 13:07 ` [PATCH 07/10] qla2xxx: Help Coverity with analyzing ct_sns_pkt initialization Bart Van Assche
2013-06-12  8:09   ` Saurav Kashyap
2013-06-05 13:08 ` [PATCH 08/10] qla2xxx: Fix qla2xxx_check_risc_status() Bart Van Assche
2013-06-12  8:10   ` Saurav Kashyap
2013-06-05 13:09 ` [PATCH 09/10] qla2xxx: Remove an unused variable from qla2x00_remove_one() Bart Van Assche
2013-06-12  8:10   ` Saurav Kashyap
2013-06-05 13:09 ` Bart Van Assche [this message]
2013-06-12  8:11   ` [PATCH 10/10] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els() Saurav Kashyap
2013-06-07 19:06 ` [PATCH 0/10] qla2xxx: Reduce the number of Coverity warnings Saurav Kashyap
2013-06-08  8:27   ` Bart Van Assche
2013-06-12  8:03     ` Saurav Kashyap

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=51AF38A7.6020808@acm.org \
    --to=bvanassche@acm.org \
    --cc=chad.dupuis@qlogic.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=saurav.kashyap@qlogic.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.