Commit 7fb23a78 authored by Damien Le Moal's avatar Damien Le Moal Committed by Martin K. Petersen
Browse files

scsi: pm8001: Fix tag values handling 

The function pm8001_tag_alloc() determines free tags using the function
find_first_zero_bit() which can return 0 when the first bit of the bitmap
being inspected is 0. As such, tag 0 is a valid tag value that should not
be dismissed as invalid. Fix the functions pm8001_work_fn(),
mpi_sata_completion(), pm8001_mpi_task_abort_resp() and
pm8001_open_reject_retry() to not dismiss 0 tags as invalid.

The value 0xffffffff is used for invalid tags for unused ccb information
structures. Add the macro definition PM8001_INVALID_TAG to define this
value.

Link: https://lore.kernel.org/r/20220220031810.738362-20-damien.lemoal@opensource.wdc.com


Reviewed-by: default avatarJack Wang <jinpu.wang@ionos.com>
Signed-off-by: default avatarDamien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 7e6b7e74

drivers/scsi/pm8001/pm8001_hwi.c

+18 −34
Original line number Diff line number Diff line
@@ -1522,7 +1522,6 @@ void pm8001_work_fn(struct work_struct *work) 
	case IO_XFER_ERROR_BREAK:

	{	/* This one stashes the sas_task instead */

		struct sas_task *t = (struct sas_task *)pm8001_dev;

		u32 tag;

		struct pm8001_ccb_info *ccb;

		struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;

		unsigned long flags, flags1;
@@ -1544,8 +1543,8 @@ void pm8001_work_fn(struct work_struct *work) 
		/* Search for a possible ccb that matches the task */

		for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {

			ccb = &pm8001_ha->ccb_info[i];

			tag = ccb->ccb_tag;

			if ((tag != 0xFFFFFFFF) && (ccb->task == t))

			if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&

			    (ccb->task == t))

				break;

		}

		if (!ccb) {
@@ -1566,11 +1565,11 @@ void pm8001_work_fn(struct work_struct *work) 
			spin_unlock_irqrestore(&t->task_state_lock, flags1);

			pm8001_dbg(pm8001_ha, FAIL, "task 0x%p done with event 0x%x resp 0x%x stat 0x%x but aborted by upper layer!\n",

				   t, pw->handler, ts->resp, ts->stat);

			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);

			pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);

			spin_unlock_irqrestore(&pm8001_ha->lock, flags);

		} else {

			spin_unlock_irqrestore(&t->task_state_lock, flags1);

			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);

			pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);

			mb();/* in order to force CPU ordering */

			spin_unlock_irqrestore(&pm8001_ha->lock, flags);

			t->task_done(t);
@@ -1579,7 +1578,6 @@ void pm8001_work_fn(struct work_struct *work) 
	case IO_XFER_OPEN_RETRY_TIMEOUT:

	{	/* This one stashes the sas_task instead */

		struct sas_task *t = (struct sas_task *)pm8001_dev;

		u32 tag;

		struct pm8001_ccb_info *ccb;

		struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;

		unsigned long flags, flags1;
@@ -1613,8 +1611,8 @@ void pm8001_work_fn(struct work_struct *work) 
		/* Search for a possible ccb that matches the task */

		for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {

			ccb = &pm8001_ha->ccb_info[i];

			tag = ccb->ccb_tag;

			if ((tag != 0xFFFFFFFF) && (ccb->task == t))

			if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&

			    (ccb->task == t))

				break;

		}

		if (!ccb) {
@@ -1685,19 +1683,13 @@ void pm8001_work_fn(struct work_struct *work) 
		struct task_status_struct *ts;

		struct sas_task *task;

		int i;

		u32 tag, device_id;

		u32 device_id;



		for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {

			ccb = &pm8001_ha->ccb_info[i];

			task = ccb->task;

			ts = &task->task_status;

			tag = ccb->ccb_tag;

			/* check if tag is NULL */

			if (!tag) {

				pm8001_dbg(pm8001_ha, FAIL,

					"tag Null\n");

				continue;

			}



			if (task != NULL) {

				dev = task->dev;

				if (!dev) {
@@ -1706,10 +1698,11 @@ void pm8001_work_fn(struct work_struct *work) 
					continue;

				}

				/*complete sas task and update to top layer */

				pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);

				pm8001_ccb_task_free(pm8001_ha, task, ccb,

						     ccb->ccb_tag);

				ts->resp = SAS_TASK_COMPLETE;

				task->task_done(task);

			} else if (tag != 0xFFFFFFFF) {

			} else if (ccb->ccb_tag != PM8001_INVALID_TAG) {

				/* complete the internal commands/non-sas task */

				pm8001_dev = ccb->device;

				if (pm8001_dev->dcompletion) {
@@ -1717,7 +1710,7 @@ void pm8001_work_fn(struct work_struct *work) 
					pm8001_dev->dcompletion = NULL;

				}

				complete(pm8001_ha->nvmd_completion);

				pm8001_tag_free(pm8001_ha, tag);

				pm8001_tag_free(pm8001_ha, ccb->ccb_tag);

			}

		}

		/* Deregister all the device ids  */
@@ -2313,11 +2306,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb) 
	param = le32_to_cpu(psataPayload->param);

	tag = le32_to_cpu(psataPayload->tag);



	if (!tag) {

		pm8001_dbg(pm8001_ha, FAIL, "tag null\n");

		return;

	}



	ccb = &pm8001_ha->ccb_info[tag];

	t = ccb->task;

	pm8001_dev = ccb->device;
@@ -3051,7 +3039,7 @@ void pm8001_mpi_set_dev_state_resp(struct pm8001_hba_info *pm8001_ha, 
		   device_id, pds, nds, status);

	complete(pm8001_dev->setds_completion);

	ccb->task = NULL;

	ccb->ccb_tag = 0xFFFFFFFF;

	ccb->ccb_tag = PM8001_INVALID_TAG;

	pm8001_tag_free(pm8001_ha, tag);

}
@@ -3069,7 +3057,7 @@ void pm8001_mpi_set_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb) 
				dlen_status);

	}

	ccb->task = NULL;

	ccb->ccb_tag = 0xFFFFFFFF;

	ccb->ccb_tag = PM8001_INVALID_TAG;

	pm8001_tag_free(pm8001_ha, tag);

}
@@ -3096,7 +3084,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb) 
		 * freed by requesting path anywhere.

		 */

		ccb->task = NULL;

		ccb->ccb_tag = 0xFFFFFFFF;

		ccb->ccb_tag = PM8001_INVALID_TAG;

		pm8001_tag_free(pm8001_ha, tag);

		return;

	}
@@ -3142,7 +3130,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb) 
	complete(pm8001_ha->nvmd_completion);

	pm8001_dbg(pm8001_ha, MSG, "Get nvmd data complete!\n");

	ccb->task = NULL;

	ccb->ccb_tag = 0xFFFFFFFF;

	ccb->ccb_tag = PM8001_INVALID_TAG;

	pm8001_tag_free(pm8001_ha, tag);

}
@@ -3555,7 +3543,7 @@ int pm8001_mpi_reg_resp(struct pm8001_hba_info *pm8001_ha, void *piomb) 
	}

	complete(pm8001_dev->dcompletion);

	ccb->task = NULL;

	ccb->ccb_tag = 0xFFFFFFFF;

	ccb->ccb_tag = PM8001_INVALID_TAG;

	pm8001_tag_free(pm8001_ha, htag);

	return 0;

}
@@ -3627,7 +3615,7 @@ int pm8001_mpi_fw_flash_update_resp(struct pm8001_hba_info *pm8001_ha, 
	}

	kfree(ccb->fw_control_context);

	ccb->task = NULL;

	ccb->ccb_tag = 0xFFFFFFFF;

	ccb->ccb_tag = PM8001_INVALID_TAG;

	pm8001_tag_free(pm8001_ha, tag);

	complete(pm8001_ha->nvmd_completion);

	return 0;
@@ -3663,10 +3651,6 @@ int pm8001_mpi_task_abort_resp(struct pm8001_hba_info *pm8001_ha, void *piomb) 


	status = le32_to_cpu(pPayload->status);

	tag = le32_to_cpu(pPayload->tag);

	if (!tag) {

		pm8001_dbg(pm8001_ha, FAIL, " TAG NULL. RETURNING !!!\n");

		return -1;

	}



	scp = le32_to_cpu(pPayload->scp);

	ccb = &pm8001_ha->ccb_info[tag];

drivers/scsi/pm8001/pm8001_init.c

+2 −1
Original line number Diff line number Diff line
@@ -1217,10 +1217,11 @@ pm8001_init_ccb_tag(struct pm8001_hba_info *pm8001_ha, struct Scsi_Host *shost, 
			goto err_out;

		}

		pm8001_ha->ccb_info[i].task = NULL;

		pm8001_ha->ccb_info[i].ccb_tag = 0xffffffff;

		pm8001_ha->ccb_info[i].ccb_tag = PM8001_INVALID_TAG;

		pm8001_ha->ccb_info[i].device = NULL;

		++pm8001_ha->tags_num;

	}



	return 0;



err_out_noccb:

drivers/scsi/pm8001/pm8001_sas.c

+6 −7
Original line number Diff line number Diff line
@@ -553,7 +553,7 @@ void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha, 


	task->lldd_task = NULL;

	ccb->task = NULL;

	ccb->ccb_tag = 0xFFFFFFFF;

	ccb->ccb_tag = PM8001_INVALID_TAG;

	ccb->open_retry = 0;

	pm8001_tag_free(pm8001_ha, ccb_idx);

}
@@ -831,9 +831,11 @@ void pm8001_open_reject_retry( 
		struct task_status_struct *ts;

		struct pm8001_device *pm8001_dev;

		unsigned long flags1;

		u32 tag;

		struct pm8001_ccb_info *ccb = &pm8001_ha->ccb_info[i];



		if (ccb->ccb_tag == PM8001_INVALID_TAG)

			continue;



		pm8001_dev = ccb->device;

		if (!pm8001_dev || (pm8001_dev->dev_type == SAS_PHY_UNUSED))

			continue;
@@ -845,9 +847,6 @@ void pm8001_open_reject_retry( 
				continue;

		} else if (pm8001_dev != device_to_close)

			continue;

		tag = ccb->ccb_tag;

		if (!tag || (tag == 0xFFFFFFFF))

			continue;

		task = ccb->task;

		if (!task || !task->task_done)

			continue;
@@ -867,11 +866,11 @@ void pm8001_open_reject_retry( 
				& SAS_TASK_STATE_ABORTED))) {

			spin_unlock_irqrestore(&task->task_state_lock,

				flags1);

			pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);

			pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);

		} else {

			spin_unlock_irqrestore(&task->task_state_lock,

				flags1);

			pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);

			pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);

			mb();/* in order to force CPU ordering */

			spin_unlock_irqrestore(&pm8001_ha->lock, flags);

			task->task_done(task);

drivers/scsi/pm8001/pm8001_sas.h

+2 −0
Original line number Diff line number Diff line
@@ -732,6 +732,8 @@ void pm8001_free_dev(struct pm8001_device *pm8001_dev); 
/* ctl shared API */

extern const struct attribute_group *pm8001_host_groups[];



#define PM8001_INVALID_TAG	((u32)-1)



static inline void

pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha,

			struct sas_task *task, struct pm8001_ccb_info *ccb,

drivers/scsi/pm8001/pm80xx_hwi.c

+0 −5
Original line number Diff line number Diff line
@@ -2402,11 +2402,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, 
	param = le32_to_cpu(psataPayload->param);

	tag = le32_to_cpu(psataPayload->tag);



	if (!tag) {

		pm8001_dbg(pm8001_ha, FAIL, "tag null\n");

		return;

	}



	ccb = &pm8001_ha->ccb_info[tag];

	t = ccb->task;

	pm8001_dev = ccb->device;