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

scsi: lpfc: Fix rmmod crash due to bad ring pointers to abort_iotag 

Rmmod on SLI-4 adapters is sometimes hitting a bad ptr dereference in
lpfc_els_free_iocb().

A prior patch refactored the lpfc_sli_abort_iocb() routine. One of the
changes was to convert from building/sending an abort within the routine to
using a common routine. The reworked routine passes, without modification,
the pring ptr to the new common routine. The older routine had logic to
check SLI-3 vs SLI-4 and adapt the pring ptr if necessary as callers were
passing SLI-3 pointers even when not on an SLI-4 adapter. The new routine
is missing this check and adapt, so the SLI-3 ring pointers are being used
in SLI-4 paths.

Fix by cleaning up the calling routines. In review, there is no need to
pass the ring ptr argument to abort_iocb at all. The routine can look at
the adapter type itself and reference the proper ring.

Link: https://lore.kernel.org/r/20210412013127.2387-2-jsmart2021@gmail.com


Fixes: db7531d2 ("scsi: lpfc: Convert abort handling to SLI-3 and SLI-4 handlers")
Cc: <stable@vger.kernel.org> # v5.11+
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 8350e196

drivers/scsi/lpfc/lpfc_crtn.h

+2 −2
Original line number Diff line number Diff line
@@ -353,8 +353,8 @@ int lpfc_sli_hbq_size(void); 
int lpfc_sli_issue_abort_iotag(struct lpfc_hba *, struct lpfc_sli_ring *,

			       struct lpfc_iocbq *, void *);

int lpfc_sli_sum_iocb(struct lpfc_vport *, uint16_t, uint64_t, lpfc_ctx_cmd);

int lpfc_sli_abort_iocb(struct lpfc_vport *, struct lpfc_sli_ring *, uint16_t,

			uint64_t, lpfc_ctx_cmd);

int lpfc_sli_abort_iocb(struct lpfc_vport *vport, u16 tgt_id, u64 lun_id,

			lpfc_ctx_cmd abort_cmd);

int

lpfc_sli_abort_taskmgmt(struct lpfc_vport *, struct lpfc_sli_ring *,

			uint16_t, uint64_t, lpfc_ctx_cmd);

drivers/scsi/lpfc/lpfc_hbadisc.c

+3 −7
Original line number Diff line number Diff line
@@ -140,11 +140,8 @@ lpfc_terminate_rport_io(struct fc_rport *rport) 
			      "rport terminate: sid:x%x did:x%x flg:x%x",

			      ndlp->nlp_sid, ndlp->nlp_DID, ndlp->nlp_flag);



	if (ndlp->nlp_sid != NLP_NO_SID) {

		lpfc_sli_abort_iocb(vport,

				    &vport->phba->sli.sli3_ring[LPFC_FCP_RING],

				    ndlp->nlp_sid, 0, LPFC_CTX_TGT);

	}

	if (ndlp->nlp_sid != NLP_NO_SID)

		lpfc_sli_abort_iocb(vport, ndlp->nlp_sid, 0, LPFC_CTX_TGT);

}



/*
@@ -299,8 +296,7 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp) 


	if (ndlp->nlp_sid != NLP_NO_SID) {

		warn_on = 1;

		lpfc_sli_abort_iocb(vport, &phba->sli.sli3_ring[LPFC_FCP_RING],

				    ndlp->nlp_sid, 0, LPFC_CTX_TGT);

		lpfc_sli_abort_iocb(vport, ndlp->nlp_sid, 0, LPFC_CTX_TGT);

	}



	if (warn_on) {

drivers/scsi/lpfc/lpfc_nportdisc.c

+1 −3
Original line number Diff line number Diff line
@@ -2559,12 +2559,10 @@ static uint32_t 
lpfc_rcv_prlo_mapped_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,

			  void *arg, uint32_t evt)

{

	struct lpfc_hba  *phba = vport->phba;

	struct lpfc_iocbq *cmdiocb = (struct lpfc_iocbq *) arg;



	/* flush the target */

	lpfc_sli_abort_iocb(vport, &phba->sli.sli3_ring[LPFC_FCP_RING],

			    ndlp->nlp_sid, 0, LPFC_CTX_TGT);

	lpfc_sli_abort_iocb(vport, ndlp->nlp_sid, 0, LPFC_CTX_TGT);



	/* Treat like rcv logo */

	lpfc_rcv_logo(vport, ndlp, cmdiocb, ELS_CMD_PRLO);

drivers/scsi/lpfc/lpfc_sli.c

+15 −5
Original line number Diff line number Diff line
@@ -11644,7 +11644,7 @@ lpfc_sli_issue_abort_iotag(struct lpfc_hba *phba, struct lpfc_sli_ring *pring, 
	icmd = &cmdiocb->iocb;

	if (icmd->ulpCommand == CMD_ABORT_XRI_CN ||

	    icmd->ulpCommand == CMD_CLOSE_XRI_CN ||

	    (cmdiocb->iocb_flag & LPFC_DRIVER_ABORTED) != 0)

	    cmdiocb->iocb_flag & LPFC_DRIVER_ABORTED)

		return IOCB_ABORTING;









	if (!pring) {








@@ -11942,7 +11942,6 @@ lpfc_sli_abort_fcp_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,







/**









 * lpfc_sli_abort_iocb - issue abort for all commands on a host/target/LUN









 * @vport: Pointer to virtual port.









 * @pring: Pointer to driver SLI ring object.









 * @tgt_id: SCSI ID of the target.









 * @lun_id: LUN ID of the scsi device.









 * @abort_cmd: LPFC_CTX_LUN/LPFC_CTX_TGT/LPFC_CTX_HOST.








@@ -11957,18 +11956,22 @@ lpfc_sli_abort_fcp_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,







 * FCP iocbs associated with SCSI target specified by tgt_id parameter.









 * When abort_cmd == LPFC_CTX_HOST, the function sends abort to all









 * FCP iocbs associated with virtual port.









 * The pring used for SLI3 is sli3_ring[LPFC_FCP_RING], for SLI4









 * lpfc_sli4_calc_ring is used.









 * This function returns number of iocbs it failed to abort.









 * This function is called with no locks held.









 **/









int









lpfc_sli_abort_iocb(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,









		    uint16_t tgt_id, uint64_t lun_id, lpfc_ctx_cmd abort_cmd)









lpfc_sli_abort_iocb(struct lpfc_vport *vport, u16 tgt_id, u64 lun_id,









		    lpfc_ctx_cmd abort_cmd)









{









	struct lpfc_hba *phba = vport->phba;









	struct lpfc_sli_ring *pring = NULL;









	struct lpfc_iocbq *iocbq;









	int errcnt = 0, ret_val = 0;









	unsigned long iflags;









	int i;









	void *fcp_cmpl = NULL;

















	/* all I/Os are in process of being flushed */









	if (phba->hba_flag & HBA_IOQ_FLUSH)








@@ -11982,8 +11985,15 @@ lpfc_sli_abort_iocb(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,







			continue;

















		spin_lock_irqsave(&phba->hbalock, iflags);









		if (phba->sli_rev == LPFC_SLI_REV3) {









			pring = &phba->sli.sli3_ring[LPFC_FCP_RING];









			fcp_cmpl = lpfc_sli_abort_fcp_cmpl;









		} else if (phba->sli_rev == LPFC_SLI_REV4) {









			pring = lpfc_sli4_calc_ring(phba, iocbq);









			fcp_cmpl = lpfc_sli4_abort_fcp_cmpl;









		}









		ret_val = lpfc_sli_issue_abort_iotag(phba, pring, iocbq,









						     lpfc_sli_abort_fcp_cmpl);









						     fcp_cmpl);









		spin_unlock_irqrestore(&phba->hbalock, iflags);









		if (ret_val != IOCB_SUCCESS)









			errcnt++;