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

scsi: lpfc: Fix FCP I/O flush functionality for TMF routines

A prior patch inadvertently caused lpfc_sli_sum_iocb() to exclude counting
of outstanding aborted I/Os and ABORT IOCBs.  Thus,
lpfc_reset_flush_io_context() called from any TMF routine does not properly
wait to flush all outstanding FCP IOCBs leading to a block layer crash on
an invalid scsi_cmnd->request pointer.

  kernel BUG at ../block/blk-core.c:1489!
  RIP: 0010:blk_requeue_request+0xaf/0xc0
  ...
  Call Trace:
  <IRQ>
  __scsi_queue_insert+0x90/0xe0 [scsi_mod]
  blk_done_softirq+0x7e/0x90
  __do_softirq+0xd2/0x280
  irq_exit+0xd5/0xe0
  do_IRQ+0x4c/0xd0
  common_interrupt+0x87/0x87
  </IRQ>

Fix by separating out the LPFC_IO_FCP, LPFC_IO_ON_TXCMPLQ,
LPFC_DRIVER_ABORTED, and CMD_ABORT_XRI_CN || CMD_CLOSE_XRI_CN checks into a
new lpfc_sli_validate_fcp_iocb_for_abort() routine when determining to
build an ABORT iocb.

Restore lpfc_reset_flush_io_context() functionality by including counting
of outstanding aborted IOCBs and ABORT IOCBs in lpfc_sli_sum_iocb().

Link: https://lore.kernel.org/r/20210910233159.115896-9-jsmart2021@gmail.com


Fixes: e1364711 ("scsi: lpfc: Fix illegal memory access on Abort IOCBs")
Cc: <stable@vger.kernel.org> # v5.12+
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 b507357f
Loading
Loading
Loading
Loading
+78 −23
Original line number Original line Diff line number Diff line
@@ -12485,15 +12485,54 @@ lpfc_sli_hba_iocb_abort(struct lpfc_hba *phba)
}
}
/**
/**
 * lpfc_sli_validate_fcp_iocb - find commands associated with a vport or LUN
 * lpfc_sli_validate_fcp_iocb_for_abort - filter iocbs appropriate for FCP aborts
 * @iocbq: Pointer to iocb object.
 * @vport: Pointer to driver virtual port object.
 *
 * This function acts as an iocb filter for functions which abort FCP iocbs.
 *
 * Return values
 * -ENODEV, if a null iocb or vport ptr is encountered
 * -EINVAL, if the iocb is not an FCP I/O, not on the TX cmpl queue, premarked as
 *          driver already started the abort process, or is an abort iocb itself
 * 0, passes criteria for aborting the FCP I/O iocb
 **/
static int
lpfc_sli_validate_fcp_iocb_for_abort(struct lpfc_iocbq *iocbq,
				     struct lpfc_vport *vport)
{
	IOCB_t *icmd = NULL;
	/* No null ptr vports */
	if (!iocbq || iocbq->vport != vport)
		return -ENODEV;
	/* iocb must be for FCP IO, already exists on the TX cmpl queue,
	 * can't be premarked as driver aborted, nor be an ABORT iocb itself
	 */
	icmd = &iocbq->iocb;
	if (!(iocbq->iocb_flag & LPFC_IO_FCP) ||
	    !(iocbq->iocb_flag & LPFC_IO_ON_TXCMPLQ) ||
	    (iocbq->iocb_flag & LPFC_DRIVER_ABORTED) ||
	    (icmd->ulpCommand == CMD_ABORT_XRI_CN ||
	     icmd->ulpCommand == CMD_CLOSE_XRI_CN))
		return -EINVAL;
	return 0;
}
/**
 * lpfc_sli_validate_fcp_iocb - validate commands associated with a SCSI target
 * @iocbq: Pointer to driver iocb object.
 * @iocbq: Pointer to driver iocb object.
 * @vport: Pointer to driver virtual port object.
 * @vport: Pointer to driver virtual port object.
 * @tgt_id: SCSI ID of the target.
 * @tgt_id: SCSI ID of the target.
 * @lun_id: LUN ID of the scsi device.
 * @lun_id: LUN ID of the scsi device.
 * @ctx_cmd: LPFC_CTX_LUN/LPFC_CTX_TGT/LPFC_CTX_HOST
 * @ctx_cmd: LPFC_CTX_LUN/LPFC_CTX_TGT/LPFC_CTX_HOST
 *
 *
 * This function acts as an iocb filter for functions which abort or count
 * This function acts as an iocb filter for validating a lun/SCSI target/SCSI
 * all FCP iocbs pending on a lun/SCSI target/SCSI host. It will return
 * host.
 *
 * It will return
 * 0 if the filtering criteria is met for the given iocb and will return
 * 0 if the filtering criteria is met for the given iocb and will return
 * 1 if the filtering criteria is not met.
 * 1 if the filtering criteria is not met.
 * If ctx_cmd == LPFC_CTX_LUN, the function returns 0 only if the
 * If ctx_cmd == LPFC_CTX_LUN, the function returns 0 only if the
@@ -12512,22 +12551,8 @@ lpfc_sli_validate_fcp_iocb(struct lpfc_iocbq *iocbq, struct lpfc_vport *vport,
			   lpfc_ctx_cmd ctx_cmd)
			   lpfc_ctx_cmd ctx_cmd)
{
{
	struct lpfc_io_buf *lpfc_cmd;
	struct lpfc_io_buf *lpfc_cmd;
	IOCB_t *icmd = NULL;
	int rc = 1;
	int rc = 1;
	if (!iocbq || iocbq->vport != vport)
		return rc;
	if (!(iocbq->iocb_flag & LPFC_IO_FCP) ||
	    !(iocbq->iocb_flag & LPFC_IO_ON_TXCMPLQ) ||
	      iocbq->iocb_flag & LPFC_DRIVER_ABORTED)
		return rc;
	icmd = &iocbq->iocb;
	if (icmd->ulpCommand == CMD_ABORT_XRI_CN ||
	    icmd->ulpCommand == CMD_CLOSE_XRI_CN)
		return rc;
	lpfc_cmd = container_of(iocbq, struct lpfc_io_buf, cur_iocbq);
	lpfc_cmd = container_of(iocbq, struct lpfc_io_buf, cur_iocbq);
	if (lpfc_cmd->pCmd == NULL)
	if (lpfc_cmd->pCmd == NULL)
@@ -12582,17 +12607,33 @@ lpfc_sli_sum_iocb(struct lpfc_vport *vport, uint16_t tgt_id, uint64_t lun_id,
{
{
	struct lpfc_hba *phba = vport->phba;
	struct lpfc_hba *phba = vport->phba;
	struct lpfc_iocbq *iocbq;
	struct lpfc_iocbq *iocbq;
	IOCB_t *icmd = NULL;
	int sum, i;
	int sum, i;
	unsigned long iflags;
	spin_lock_irq(&phba->hbalock);
	spin_lock_irqsave(&phba->hbalock, iflags);
	for (i = 1, sum = 0; i <= phba->sli.last_iotag; i++) {
	for (i = 1, sum = 0; i <= phba->sli.last_iotag; i++) {
		iocbq = phba->sli.iocbq_lookup[i];
		iocbq = phba->sli.iocbq_lookup[i];
		if (!iocbq || iocbq->vport != vport)
			continue;
		if (!(iocbq->iocb_flag & LPFC_IO_FCP) ||
		    !(iocbq->iocb_flag & LPFC_IO_ON_TXCMPLQ))
			continue;
		/* Include counting outstanding aborts */
		icmd = &iocbq->iocb;
		if (icmd->ulpCommand == CMD_ABORT_XRI_CN ||
		    icmd->ulpCommand == CMD_CLOSE_XRI_CN) {
			sum++;
			continue;
		}
		if (lpfc_sli_validate_fcp_iocb(iocbq, vport, tgt_id, lun_id,
		if (lpfc_sli_validate_fcp_iocb(iocbq, vport, tgt_id, lun_id,
					       ctx_cmd) == 0)
					       ctx_cmd) == 0)
			sum++;
			sum++;
	}
	}
	spin_unlock_irq(&phba->hbalock);
	spin_unlock_irqrestore(&phba->hbalock, iflags);
	return sum;
	return sum;
}
}
@@ -12659,7 +12700,11 @@ lpfc_sli_abort_fcp_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
 *
 *
 * This function sends an abort command for every SCSI command
 * This function sends an abort command for every SCSI command
 * associated with the given virtual port pending on the ring
 * associated with the given virtual port pending on the ring
 * filtered by lpfc_sli_validate_fcp_iocb function.
 * filtered by lpfc_sli_validate_fcp_iocb_for_abort and then
 * lpfc_sli_validate_fcp_iocb function.  The ordering for validation before
 * submitting abort iocbs must be lpfc_sli_validate_fcp_iocb_for_abort
 * followed by lpfc_sli_validate_fcp_iocb.
 *
 * When abort_cmd == LPFC_CTX_LUN, the function sends abort only to the
 * When abort_cmd == LPFC_CTX_LUN, the function sends abort only to the
 * FCP iocbs associated with lun specified by tgt_id and lun_id
 * FCP iocbs associated with lun specified by tgt_id and lun_id
 * parameters
 * parameters
@@ -12691,6 +12736,9 @@ lpfc_sli_abort_iocb(struct lpfc_vport *vport, u16 tgt_id, u64 lun_id,
	for (i = 1; i <= phba->sli.last_iotag; i++) {
	for (i = 1; i <= phba->sli.last_iotag; i++) {
		iocbq = phba->sli.iocbq_lookup[i];
		iocbq = phba->sli.iocbq_lookup[i];
		if (lpfc_sli_validate_fcp_iocb_for_abort(iocbq, vport))
			continue;
		if (lpfc_sli_validate_fcp_iocb(iocbq, vport, tgt_id, lun_id,
		if (lpfc_sli_validate_fcp_iocb(iocbq, vport, tgt_id, lun_id,
					       abort_cmd) != 0)
					       abort_cmd) != 0)
			continue;
			continue;
@@ -12723,7 +12771,11 @@ lpfc_sli_abort_iocb(struct lpfc_vport *vport, u16 tgt_id, u64 lun_id,
 *
 *
 * This function sends an abort command for every SCSI command
 * This function sends an abort command for every SCSI command
 * associated with the given virtual port pending on the ring
 * associated with the given virtual port pending on the ring
 * filtered by lpfc_sli_validate_fcp_iocb function.
 * filtered by lpfc_sli_validate_fcp_iocb_for_abort and then
 * lpfc_sli_validate_fcp_iocb function.  The ordering for validation before
 * submitting abort iocbs must be lpfc_sli_validate_fcp_iocb_for_abort
 * followed by lpfc_sli_validate_fcp_iocb.
 *
 * When taskmgmt_cmd == LPFC_CTX_LUN, the function sends abort only to the
 * When taskmgmt_cmd == LPFC_CTX_LUN, the function sends abort only to the
 * FCP iocbs associated with lun specified by tgt_id and lun_id
 * FCP iocbs associated with lun specified by tgt_id and lun_id
 * parameters
 * parameters
@@ -12761,6 +12813,9 @@ lpfc_sli_abort_taskmgmt(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,
	for (i = 1; i <= phba->sli.last_iotag; i++) {
	for (i = 1; i <= phba->sli.last_iotag; i++) {
		iocbq = phba->sli.iocbq_lookup[i];
		iocbq = phba->sli.iocbq_lookup[i];
		if (lpfc_sli_validate_fcp_iocb_for_abort(iocbq, vport))
			continue;
		if (lpfc_sli_validate_fcp_iocb(iocbq, vport, tgt_id, lun_id,
		if (lpfc_sli_validate_fcp_iocb(iocbq, vport, tgt_id, lun_id,
					       cmd) != 0)
					       cmd) != 0)
			continue;
			continue;