Commit a4de8356 authored by James Smart's avatar James Smart Committed by Martin K. Petersen
Browse files

scsi: lpfc: Fix various issues reported by tools 

This patch fixes below Smatch reported issues:

 1. lpfc_hbadisc.c:3020 lpfc_mbx_cmpl_fcf_rr_read_fcf_rec()
    error: uninitialized symbol 'vlan_id'.

 2. lpfc_hbadisc.c:3121 lpfc_mbx_cmpl_read_fcf_rec()
    error: uninitialized symbol 'vlan_id'.

 3. lpfc_init.c:335 lpfc_dump_wakeup_param_cmpl()
    warn: always true condition '(prg->dist < 4) => (0-3 < 4)'

 4. lpfc_init.c:2419 lpfc_parse_vpd()
    warn: inconsistent indenting.

 5. lpfc_init.c:13248 lpfc_sli4_enable_msi()
    warn: 'phba->pcidev->irq' 2147483648 can't fit into 65535
    'eqhdl->irq'

 6. lpfc_debugfs.c:5300 lpfc_idiag_extacc_avail_get()
    error: uninitialized symbol 'ext_cnt'

 7. lpfc_debugfs.c:5300 lpfc_idiag_extacc_avail_get()
    error: uninitialized symbol 'ext_size'

 8. lpfc_vmid.c:248 lpfc_vmid_get_appid()
    warn: sleeping in atomic context.

 9. lpfc_init.c:8342 lpfc_sli4_driver_resource_setup()
    warn: missing error code 'rc'.

10. lpfc_init.c:13573 lpfc_sli4_hba_unset()
    warn: variable dereferenced before check 'phba->pport' (see
    line 13546)

11. lpfc_auth.c:1923 lpfc_auth_handle_dhchap_reply()
    error: double free of 'hash_value'

Fixes:

 1. Initialize vlan_id to LPFC_FCOE_NULL_VID.

 2. Initialize vlan_id to LPFC_FCOE_NULL_VID.

 3. prg->dist is a 2 bit field. Its value can only be between 0-3.
    Remove redundent check 'if (prg->dist < 4)'.

 4. Fix inconsistent indenting.  Moved logic into helper function
    lpfc_fill_vpd().

 5. Define 'eqhdl->irq' as int value as pci_irq_vector() returns int.
    Also, check for return value of pci_irq_vector() and log message in
    case of failure.

 6. Initialize 'ext_cnt' to 0.

 7. Initialize 'ext_size' to 0.

 8. Use alloc_percpu_gfp() with GFP_ATOMIC flag.

 9. 'rc' was not updated when dma_pool_create() fails.  Update 'rc =
     -ENOMEM' when dma_pool_create() fails before calling goto statement.

10. Add check for 'phba->pport' in lpfc_cpuhp_remove().

11. Initialize 'hash_value' to NULL, same like 'aug_chal' variable.

Link: https://lore.kernel.org/r/20220911221505.117655-13-jsmart2021@gmail.com


Co-developed-by: default avatarJustin Tee <justin.tee@broadcom.com>
Signed-off-by: default avatarJustin Tee <justin.tee@broadcom.com>
Signed-off-by: default avatarJames Smart <jsmart2021@gmail.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent dbb1e2ff

drivers/scsi/lpfc/lpfc_debugfs.c

+1 −1
Original line number Diff line number Diff line
@@ -5156,7 +5156,7 @@ lpfc_idiag_mbxacc_write(struct file *file, const char __user *buf, 
static int

lpfc_idiag_extacc_avail_get(struct lpfc_hba *phba, char *pbuffer, int len)

{

	uint16_t ext_cnt, ext_size;

	uint16_t ext_cnt = 0, ext_size = 0;



	len += scnprintf(pbuffer+len, LPFC_EXT_ACC_BUF_SIZE-len,

			"\nAvailable Extents Information:\n");

drivers/scsi/lpfc/lpfc_hbadisc.c

+2 −2
Original line number Diff line number Diff line
@@ -2970,7 +2970,7 @@ lpfc_mbx_cmpl_fcf_rr_read_fcf_rec(struct lpfc_hba *phba, LPFC_MBOXQ_t *mboxq) 
	uint32_t boot_flag, addr_mode;

	uint16_t next_fcf_index, fcf_index;

	uint16_t current_fcf_index;

	uint16_t vlan_id;

	uint16_t vlan_id = LPFC_FCOE_NULL_VID;

	int rc;



	/* If link state is not up, stop the roundrobin failover process */
@@ -3075,7 +3075,7 @@ lpfc_mbx_cmpl_read_fcf_rec(struct lpfc_hba *phba, LPFC_MBOXQ_t *mboxq) 
	struct fcf_record *new_fcf_record;

	uint32_t boot_flag, addr_mode;

	uint16_t fcf_index, next_fcf_index;

	uint16_t vlan_id;

	uint16_t vlan_id =  LPFC_FCOE_NULL_VID;

	int rc;



	/* If link state is not up, no need to proceed */

drivers/scsi/lpfc/lpfc_init.c

+138 −111
Original line number Diff line number Diff line
@@ -325,7 +325,6 @@ lpfc_dump_wakeup_param_cmpl(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmboxq) 
	prog_id_word = pmboxq->u.mb.un.varWords[7];



	/* Decode the Option rom version word to a readable string */

	if (prg->dist < 4)

	dist = dist_char[prg->dist];



	if ((prg->dist == 3) && (prg->num == 0))
@@ -2258,134 +2257,83 @@ lpfc_handle_latt(struct lpfc_hba *phba) 
	return;

}



/**

 * lpfc_parse_vpd - Parse VPD (Vital Product Data)

 * @phba: pointer to lpfc hba data structure.

 * @vpd: pointer to the vital product data.

 * @len: length of the vital product data in bytes.

 *

 * This routine parses the Vital Product Data (VPD). The VPD is treated as

 * an array of characters. In this routine, the ModelName, ProgramType, and

 * ModelDesc, etc. fields of the phba data structure will be populated.

 *

 * Return codes

 *   0 - pointer to the VPD passed in is NULL

 *   1 - success

 **/

int

lpfc_parse_vpd(struct lpfc_hba *phba, uint8_t *vpd, int len)

static void

lpfc_fill_vpd(struct lpfc_hba *phba, uint8_t *vpd, int length, int *pindex)

{

	uint8_t lenlo, lenhi;

	int Length;

	int i, j;

	int finished = 0;

	int index = 0;



	if (!vpd)

		return 0;



	/* Vital Product */

	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,

			"0455 Vital Product Data: x%x x%x x%x x%x\n",

			(uint32_t) vpd[0], (uint32_t) vpd[1], (uint32_t) vpd[2],

			(uint32_t) vpd[3]);

	while (!finished && (index < (len - 4))) {

		switch (vpd[index]) {

		case 0x82:

		case 0x91:

			index += 1;

			lenlo = vpd[index];

			index += 1;

			lenhi = vpd[index];

			index += 1;

			i = ((((unsigned short)lenhi) << 8) + lenlo);

			index += i;

			break;

		case 0x90:

			index += 1;

			lenlo = vpd[index];

			index += 1;

			lenhi = vpd[index];

			index += 1;

			Length = ((((unsigned short)lenhi) << 8) + lenlo);

			if (Length > len - index)

				Length = len - index;

			while (Length > 0) {

	while (length > 0) {

		/* Look for Serial Number */

			if ((vpd[index] == 'S') && (vpd[index+1] == 'N')) {

				index += 2;

				i = vpd[index];

				index += 1;

		if ((vpd[*pindex] == 'S') && (vpd[*pindex + 1] == 'N')) {

			*pindex += 2;

			i = vpd[*pindex];

			*pindex += 1;

			j = 0;

				Length -= (3+i);

			length -= (3+i);

			while (i--) {

					phba->SerialNumber[j++] = vpd[index++];

				phba->SerialNumber[j++] = vpd[(*pindex)++];

				if (j == 31)

					break;

			}

			phba->SerialNumber[j] = 0;

			continue;

			}

			else if ((vpd[index] == 'V') && (vpd[index+1] == '1')) {

		} else if ((vpd[*pindex] == 'V') && (vpd[*pindex + 1] == '1')) {

			phba->vpd_flag |= VPD_MODEL_DESC;

				index += 2;

				i = vpd[index];

				index += 1;

			*pindex += 2;

			i = vpd[*pindex];

			*pindex += 1;

			j = 0;

				Length -= (3+i);

			length -= (3+i);

			while (i--) {

					phba->ModelDesc[j++] = vpd[index++];

				phba->ModelDesc[j++] = vpd[(*pindex)++];

				if (j == 255)

					break;

			}

			phba->ModelDesc[j] = 0;

			continue;

			}

			else if ((vpd[index] == 'V') && (vpd[index+1] == '2')) {

		} else if ((vpd[*pindex] == 'V') && (vpd[*pindex + 1] == '2')) {

			phba->vpd_flag |= VPD_MODEL_NAME;

				index += 2;

				i = vpd[index];

				index += 1;

			*pindex += 2;

			i = vpd[*pindex];

			*pindex += 1;

			j = 0;

				Length -= (3+i);

			length -= (3+i);

			while (i--) {

					phba->ModelName[j++] = vpd[index++];

				phba->ModelName[j++] = vpd[(*pindex)++];

				if (j == 79)

					break;

			}

			phba->ModelName[j] = 0;

			continue;

			}

			else if ((vpd[index] == 'V') && (vpd[index+1] == '3')) {

		} else if ((vpd[*pindex] == 'V') && (vpd[*pindex + 1] == '3')) {

			phba->vpd_flag |= VPD_PROGRAM_TYPE;

				index += 2;

				i = vpd[index];

				index += 1;

			*pindex += 2;

			i = vpd[*pindex];

			*pindex += 1;

			j = 0;

				Length -= (3+i);

			length -= (3+i);

			while (i--) {

					phba->ProgramType[j++] = vpd[index++];

				phba->ProgramType[j++] = vpd[(*pindex)++];

				if (j == 255)

					break;

			}

			phba->ProgramType[j] = 0;

			continue;

			}

			else if ((vpd[index] == 'V') && (vpd[index+1] == '4')) {

		} else if ((vpd[*pindex] == 'V') && (vpd[*pindex + 1] == '4')) {

			phba->vpd_flag |= VPD_PORT;

				index += 2;

				i = vpd[index];

				index += 1;

			*pindex += 2;

			i = vpd[*pindex];

			*pindex += 1;

			j = 0;

				Length -= (3+i);

			length -= (3 + i);

			while (i--) {

				if ((phba->sli_rev == LPFC_SLI_REV4) &&

				    (phba->sli4_hba.pport_name_sta ==

				     LPFC_SLI4_PPNAME_GET)) {

					j++;

						index++;

					(*pindex)++;

				} else

						phba->Port[j++] = vpd[index++];

					phba->Port[j++] = vpd[(*pindex)++];

				if (j == 19)

					break;

			}
@@ -2394,15 +2342,70 @@ lpfc_parse_vpd(struct lpfc_hba *phba, uint8_t *vpd, int len) 
			     LPFC_SLI4_PPNAME_NON))

				phba->Port[j] = 0;

			continue;

		} else {

			*pindex += 2;

			i = vpd[*pindex];

			*pindex += 1;

			*pindex += i;

			length -= (3 + i);

		}

			else {

				index += 2;

				i = vpd[index];

				index += 1;

				index += i;

				Length -= (3 + i);

	}

}



/**

 * lpfc_parse_vpd - Parse VPD (Vital Product Data)

 * @phba: pointer to lpfc hba data structure.

 * @vpd: pointer to the vital product data.

 * @len: length of the vital product data in bytes.

 *

 * This routine parses the Vital Product Data (VPD). The VPD is treated as

 * an array of characters. In this routine, the ModelName, ProgramType, and

 * ModelDesc, etc. fields of the phba data structure will be populated.

 *

 * Return codes

 *   0 - pointer to the VPD passed in is NULL

 *   1 - success

 **/

int

lpfc_parse_vpd(struct lpfc_hba *phba, uint8_t *vpd, int len)

{

	uint8_t lenlo, lenhi;

	int Length;

	int i;

	int finished = 0;

	int index = 0;



	if (!vpd)

		return 0;



	/* Vital Product */

	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,

			"0455 Vital Product Data: x%x x%x x%x x%x\n",

			(uint32_t) vpd[0], (uint32_t) vpd[1], (uint32_t) vpd[2],

			(uint32_t) vpd[3]);

	while (!finished && (index < (len - 4))) {

		switch (vpd[index]) {

		case 0x82:

		case 0x91:

			index += 1;

			lenlo = vpd[index];

			index += 1;

			lenhi = vpd[index];

			index += 1;

			i = ((((unsigned short)lenhi) << 8) + lenlo);

			index += i;

			break;

		case 0x90:

			index += 1;

			lenlo = vpd[index];

			index += 1;

			lenhi = vpd[index];

			index += 1;

			Length = ((((unsigned short)lenhi) << 8) + lenlo);

			if (Length > len - index)

				Length = len - index;



			lpfc_fill_vpd(phba, vpd, Length, &index);

			finished = 0;

			break;

		case 0x78:
@@ -8304,8 +8307,10 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba) 
					&phba->pcidev->dev,

					phba->cfg_sg_dma_buf_size,

					i, 0);

	if (!phba->lpfc_sg_dma_buf_pool)

	if (!phba->lpfc_sg_dma_buf_pool) {

		rc = -ENOMEM;

		goto out_free_bsmbx;

	}



	phba->lpfc_cmd_rsp_buf_pool =

			dma_pool_create("lpfc_cmd_rsp_buf_pool",
@@ -8313,8 +8318,10 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba) 
					sizeof(struct fcp_cmnd) +

					sizeof(struct fcp_rsp),

					i, 0);

	if (!phba->lpfc_cmd_rsp_buf_pool)

	if (!phba->lpfc_cmd_rsp_buf_pool) {

		rc = -ENOMEM;

		goto out_free_sg_dma_buf;

	}



	mempool_free(mboxq, phba->mbox_mem_pool);
@@ -12402,7 +12409,7 @@ lpfc_hba_eq_hdl_array_init(struct lpfc_hba *phba) 


	for (i = 0; i < phba->cfg_irq_chann; i++) {

		eqhdl = lpfc_get_eq_hdl(i);

		eqhdl->irq = LPFC_VECTOR_MAP_EMPTY;

		eqhdl->irq = LPFC_IRQ_EMPTY;

		eqhdl->phba = phba;

	}

}
@@ -12775,7 +12782,7 @@ static void __lpfc_cpuhp_remove(struct lpfc_hba *phba) 


static void lpfc_cpuhp_remove(struct lpfc_hba *phba)

{

	if (phba->pport->fc_flag & FC_OFFLINE_MODE)

	if (phba->pport && (phba->pport->fc_flag & FC_OFFLINE_MODE))

		return;



	__lpfc_cpuhp_remove(phba);
@@ -13039,8 +13046,16 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba) 
			 LPFC_DRIVER_HANDLER_NAME"%d", index);



		eqhdl->idx = index;

		rc = request_irq(pci_irq_vector(phba->pcidev, index),

			 &lpfc_sli4_hba_intr_handler, 0,

		rc = pci_irq_vector(phba->pcidev, index);

		if (rc < 0) {

			lpfc_printf_log(phba, KERN_WARNING, LOG_INIT,

					"0489 MSI-X fast-path (%d) "

					"pci_irq_vec failed (%d)\n", index, rc);

			goto cfg_fail_out;

		}

		eqhdl->irq = rc;



		rc = request_irq(eqhdl->irq, &lpfc_sli4_hba_intr_handler, 0,

				 name, eqhdl);

		if (rc) {

			lpfc_printf_log(phba, KERN_WARNING, LOG_INIT,
@@ -13049,8 +13064,6 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba) 
			goto cfg_fail_out;

		}



		eqhdl->irq = pci_irq_vector(phba->pcidev, index);



		if (aff_mask) {

			/* If found a neighboring online cpu, set affinity */

			if (cpu_select < nr_cpu_ids)
@@ -13167,7 +13180,14 @@ lpfc_sli4_enable_msi(struct lpfc_hba *phba) 
	}



	eqhdl = lpfc_get_eq_hdl(0);

	eqhdl->irq = pci_irq_vector(phba->pcidev, 0);

	rc = pci_irq_vector(phba->pcidev, 0);

	if (rc < 0) {

		pci_free_irq_vectors(phba->pcidev);

		lpfc_printf_log(phba, KERN_WARNING, LOG_INIT,

				"0496 MSI pci_irq_vec failed (%d)\n", rc);

		return rc;

	}

	eqhdl->irq = rc;



	cpu = cpumask_first(cpu_present_mask);

	lpfc_assign_eq_map_info(phba, 0, LPFC_CPU_FIRST_IRQ, cpu);
@@ -13194,8 +13214,8 @@ lpfc_sli4_enable_msi(struct lpfc_hba *phba) 
 * MSI-X -> MSI -> IRQ.

 *

 * Return codes

 * 	0 - successful

 * 	other values - error

 *	Interrupt mode (2, 1, 0) - successful

 *	LPFC_INTR_ERROR - error

 **/

static uint32_t

lpfc_sli4_enable_intr(struct lpfc_hba *phba, uint32_t cfg_mode)
@@ -13240,7 +13260,14 @@ lpfc_sli4_enable_intr(struct lpfc_hba *phba, uint32_t cfg_mode) 
			intr_mode = 0;



			eqhdl = lpfc_get_eq_hdl(0);

			eqhdl->irq = pci_irq_vector(phba->pcidev, 0);

			retval = pci_irq_vector(phba->pcidev, 0);

			if (retval < 0) {

				lpfc_printf_log(phba, KERN_WARNING, LOG_INIT,

					"0502 INTR pci_irq_vec failed (%d)\n",

					 retval);

				return LPFC_INTR_ERROR;

			}

			eqhdl->irq = retval;



			cpu = cpumask_first(cpu_present_mask);

			lpfc_assign_eq_map_info(phba, 0, LPFC_CPU_FIRST_IRQ,

drivers/scsi/lpfc/lpfc_sli.c

+3 −0
Original line number Diff line number Diff line
@@ -6215,6 +6215,9 @@ lpfc_sli4_get_avail_extnt_rsrc(struct lpfc_hba *phba, uint16_t type, 
	struct lpfc_mbx_get_rsrc_extent_info *rsrc_info;

	LPFC_MBOXQ_t *mbox;









	*extnt_count = 0;









	*extnt_size = 0;

















	mbox = (LPFC_MBOXQ_t *) mempool_alloc(phba->mbox_mem_pool, GFP_KERNEL);









	if (!mbox)









		return -ENOMEM;
































drivers/scsi/lpfc/lpfc_sli4.h



+3
−1






























Original line number
Diff line number
Diff line







@@ -489,7 +489,7 @@ struct lpfc_hba;







#define LPFC_SLI4_HANDLER_NAME_SZ	16









struct lpfc_hba_eq_hdl {









	uint32_t idx;









	uint16_t irq;









	int irq;









	char handler_name[LPFC_SLI4_HANDLER_NAME_SZ];









	struct lpfc_hba *phba;









	struct lpfc_queue *eq;








@@ -611,6 +611,8 @@ struct lpfc_vector_map_info {







};









#define LPFC_VECTOR_MAP_EMPTY	0xffff



















#define LPFC_IRQ_EMPTY 0xffffffff



















/* Multi-XRI pool */









#define XRI_BATCH               8