[PATCH 0/2] scsi: qla2xxx: Fix bugs found by CodeSonar

[PATCH 0/2] scsi: qla2xxx: Fix bugs found by CodeSonar

[Joy Gu]

Joy Gu (2):
  scsi: qla2xxx: Fix a memory leak in an error path of
    qla2x00_process_els()
  scsi: qla2xxx: Initialize uninitialized variables

 drivers/scsi/qla2xxx/qla_attr.c | 2 +-
 drivers/scsi/qla2xxx/qla_bsg.c  | 2 +-
 drivers/scsi/qla2xxx/qla_init.c | 6 +++---
 drivers/scsi/qla2xxx/qla_mbx.c  | 6 +++---
 drivers/scsi/qla2xxx/qla_nx2.c  | 2 +-
 drivers/scsi/qla2xxx/qla_os.c   | 8 ++++----
 6 files changed, 13 insertions(+), 13 deletions(-)

-- 
2.17.1

[PATCH 1/2] scsi: qla2xxx: Fix a memory leak in an error path of qla2x00_process_els()

[Joy Gu]

Commit 8c0eb596baa5 ("[SCSI] qla2xxx: Fix a memory leak in an error path of
qla2x00_process_els()"), intended to change

        bsg_job->request->msgcode == FC_BSG_HST_ELS_NOLOGIN

to

        bsg_job->request->msgcode != FC_BSG_RPT_ELS

but changed it to

        bsg_job->request->msgcode == FC_BSG_RPT_ELS

instead.

Change the == to a != to avoid leaking the fcport structure or freeing
unallocated memory.

Fixes: 8c0eb596baa5 ("[SCSI] qla2xxx: Fix a memory leak in an error path of qla2x00_process_els()")
Signed-off-by: Joy Gu <jgu@purestorage.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 4b5d28d89d69..655cf5de604b 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -431,7 +431,7 @@ qla2x00_process_els(struct bsg_job *bsg_job)
 	goto done_free_fcport;
 
 done_free_fcport:
-	if (bsg_request->msgcode == FC_BSG_RPT_ELS)
+	if (bsg_request->msgcode != FC_BSG_RPT_ELS)
 		qla2x00_free_fcport(fcport);
 done:
 	return rval;
-- 
2.17.1

[PATCH 2/2] scsi: qla2xxx: Initialize uninitialized variables

[Joy Gu]

Zero-initialize variables left uninitialized by qla2x00_mailbox_command(),
qla8044_reg_indirect(), qla83xx_rd_reg(), and
__qla83xx_get_idc_control(). Initialize mc in qla2x00_dump_mctp_data()
so that mcp->mb[10] |= BIT_7 doesn't have an unexpected result.

Signed-off-by: Joy Gu <jgu@purestorage.com>
---
 drivers/scsi/qla2xxx/qla_attr.c | 2 +-
 drivers/scsi/qla2xxx/qla_init.c | 6 +++---
 drivers/scsi/qla2xxx/qla_mbx.c  | 6 +++---
 drivers/scsi/qla2xxx/qla_nx2.c  | 2 +-
 drivers/scsi/qla2xxx/qla_os.c   | 8 ++++----
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index d09776b77af2..6f504b313089 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -698,7 +698,7 @@ qla2x00_sysfs_write_reset(struct file *filp, struct kobject *kobj,
 	struct qla_hw_data *ha = vha->hw;
 	struct scsi_qla_host *base_vha = pci_get_drvdata(ha->pdev);
 	int type;
-	uint32_t idc_control;
+	uint32_t idc_control = 0;
 	uint8_t *tmp_data = NULL;
 
 	if (off != 0)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 5fc7697f0af4..8f8ba47ac2c0 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -6631,8 +6631,8 @@ void
 qla83xx_reset_ownership(scsi_qla_host_t *vha)
 {
 	struct qla_hw_data *ha = vha->hw;
-	uint32_t drv_presence, drv_presence_mask;
-	uint32_t dev_part_info1, dev_part_info2, class_type;
+	uint32_t drv_presence = 0, drv_presence_mask;
+	uint32_t dev_part_info1 = 0, dev_part_info2 = 0, class_type;
 	uint32_t class_type_mask = 0x3;
 	uint16_t fcoe_other_function = 0xffff, i;
 
@@ -6776,7 +6776,7 @@ static int
 qla83xx_initiating_reset(scsi_qla_host_t *vha)
 {
 	struct qla_hw_data *ha = vha->hw;
-	uint32_t  idc_control, dev_state;
+	uint32_t  idc_control = 0, dev_state = 0;
 
 	__qla83xx_get_idc_control(vha, &idc_control);
 	if ((idc_control & QLA83XX_IDC_RESET_DISABLED)) {
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 7811c4952035..b8037763c174 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -1681,7 +1681,7 @@ qla2x00_get_adapter_id(scsi_qla_host_t *vha, uint16_t *id, uint8_t *al_pa,
     uint8_t *area, uint8_t *domain, uint16_t *top, uint16_t *sw_cap)
 {
 	int rval;
-	mbx_cmd_t mc;
+	mbx_cmd_t mc = { 0, };
 	mbx_cmd_t *mcp = &mc;
 
 	ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x1046,
@@ -2257,7 +2257,7 @@ qla2x00_get_port_name(scsi_qla_host_t *vha, uint16_t loop_id, uint8_t *name,
     uint8_t opt)
 {
 	int rval;
-	mbx_cmd_t mc;
+	mbx_cmd_t mc = { 0, };
 	mbx_cmd_t *mcp = &mc;
 
 	ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x1057,
@@ -6366,7 +6366,7 @@ qla2x00_dump_mctp_data(scsi_qla_host_t *vha, dma_addr_t req_dma, uint32_t addr,
 	uint32_t size)
 {
 	int rval;
-	mbx_cmd_t mc;
+	mbx_cmd_t mc = { 0, };
 	mbx_cmd_t *mcp = &mc;
 
 	if (!IS_MCTP_CAPABLE(vha->hw))
diff --git a/drivers/scsi/qla2xxx/qla_nx2.c b/drivers/scsi/qla2xxx/qla_nx2.c
index 5ceecc9642fc..18beeb062ee3 100644
--- a/drivers/scsi/qla2xxx/qla_nx2.c
+++ b/drivers/scsi/qla2xxx/qla_nx2.c
@@ -2466,7 +2466,7 @@ qla8044_minidump_process_l2tag(struct scsi_qla_host *vha,
 	uint32_t addr, r_addr, c_addr, t_r_addr;
 	uint32_t i, k, loop_count, t_value, r_cnt, r_value;
 	unsigned long p_wait, w_time, p_mask;
-	uint32_t c_value_w, c_value_r;
+	uint32_t c_value_w, c_value_r = 0;
 	struct qla8044_minidump_entry_cache *cache_hdr;
 	int rval = QLA_FUNCTION_FAILED;
 	uint32_t *data_ptr = *d_ptr;
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index d2e40aaba734..4fe00da03a20 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -5656,7 +5656,7 @@ qla83xx_check_nic_core_fw_alive(scsi_qla_host_t *base_vha)
 {
 	int rval = QLA_SUCCESS;
 	unsigned long heart_beat_wait = jiffies + (1 * HZ);
-	uint32_t heart_beat_counter1, heart_beat_counter2;
+	uint32_t heart_beat_counter1 = 0, heart_beat_counter2 = 0;
 
 	do {
 		if (time_after(jiffies, heart_beat_wait)) {
@@ -5726,7 +5726,7 @@ qla83xx_service_idc_aen(struct work_struct *work)
 	struct qla_hw_data *ha =
 		container_of(work, struct qla_hw_data, idc_aen);
 	scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev);
-	uint32_t dev_state, idc_control;
+	uint32_t dev_state = 0, idc_control = 0;
 
 	qla83xx_idc_lock(base_vha, 0);
 	qla83xx_rd_reg(base_vha, QLA83XX_IDC_DEV_STATE, &dev_state);
@@ -6507,7 +6507,7 @@ static void
 qla83xx_need_reset_handler(scsi_qla_host_t *vha)
 {
 	struct qla_hw_data *ha = vha->hw;
-	uint32_t drv_ack, drv_presence;
+	uint32_t drv_ack = 0, drv_presence = 0;
 	unsigned long ack_timeout;
 
 	/* Wait for IDC ACK from all functions (DRV-ACK == DRV-PRESENCE) */
@@ -6546,7 +6546,7 @@ static int
 qla83xx_device_bootstrap(scsi_qla_host_t *vha)
 {
 	int rval = QLA_SUCCESS;
-	uint32_t idc_control;
+	uint32_t idc_control = 0;
 
 	qla83xx_wr_reg(vha, QLA83XX_IDC_DEV_STATE, QLA8XXX_DEV_INITIALIZING);
 	ql_log(ql_log_info, vha, 0xb069, "HW State: INITIALIZING.\n");
-- 
2.17.1

Re: [PATCH 1/2] scsi: qla2xxx: Fix a memory leak in an error path of qla2x00_process_els()

[Bart Van Assche]

On 10/12/21 12:18 PM, Joy Gu wrote:
> Change the == to a != to avoid leaking the fcport structure or freeing
> unallocated memory.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH 0/2] scsi: qla2xxx: Fix bugs found by CodeSonar

[Martin K. Petersen]

On Tue, 12 Oct 2021 12:18:32 -0700, Joy Gu wrote:

> Joy Gu (2):
>   scsi: qla2xxx: Fix a memory leak in an error path of
>     qla2x00_process_els()
>   scsi: qla2xxx: Initialize uninitialized variables
> 
> drivers/scsi/qla2xxx/qla_attr.c | 2 +-
>  drivers/scsi/qla2xxx/qla_bsg.c  | 2 +-
>  drivers/scsi/qla2xxx/qla_init.c | 6 +++---
>  drivers/scsi/qla2xxx/qla_mbx.c  | 6 +++---
>  drivers/scsi/qla2xxx/qla_nx2.c  | 2 +-
>  drivers/scsi/qla2xxx/qla_os.c   | 8 ++++----
>  6 files changed, 13 insertions(+), 13 deletions(-)
> 
> [...]

Applied to 5.15/scsi-fixes, thanks!

[1/2] scsi: qla2xxx: Fix a memory leak in an error path of qla2x00_process_els()
      https://git.kernel.org/mkp/scsi/c/7fb223d0ad80

-- 
Martin K. Petersen	Oracle Linux Engineering