Skip to content

Commit

Permalink
scsi: lpfc: Fix crash receiving ELS while detaching driver
Browse files Browse the repository at this point in the history
The driver crashes when attempting to use a freed ndpl pointer.

The pci_remove_one handler runs on a separate kernel thread. The order
of the removal is starting by freeing all of the ndlps and then
disabling interrupts. In between these two events the driver can still
receive an ELS and process it. When it tries to use the ndlp pointer
will be NULL

Change the order of the pci_remove_one vs disable interrupts so that
interrupts are disabled before the ndlp's are freed.

Cc: <stable@vger.kernel.org> # 4.12+
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <james.smart@broadcom.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
Dick Kennedy authored and Martin K. Petersen committed Oct 3, 2017
1 parent 401bb41 commit 1234a6d
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 13 deletions.
6 changes: 4 additions & 2 deletions drivers/scsi/lpfc/lpfc_attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -3134,7 +3134,8 @@ lpfc_txq_hw_show(struct device *dev, struct device_attribute *attr, char *buf)
struct lpfc_hba *phba = ((struct lpfc_vport *) shost->hostdata)->phba;
struct lpfc_sli_ring *pring = lpfc_phba_elsring(phba);

return snprintf(buf, PAGE_SIZE, "%d\n", pring->txq_max);
return snprintf(buf, PAGE_SIZE, "%d\n",
pring ? pring->txq_max : 0);
}

static DEVICE_ATTR(txq_hw, S_IRUGO,
Expand All @@ -3147,7 +3148,8 @@ lpfc_txcmplq_hw_show(struct device *dev, struct device_attribute *attr,
struct lpfc_hba *phba = ((struct lpfc_vport *) shost->hostdata)->phba;
struct lpfc_sli_ring *pring = lpfc_phba_elsring(phba);

return snprintf(buf, PAGE_SIZE, "%d\n", pring->txcmplq_max);
return snprintf(buf, PAGE_SIZE, "%d\n",
pring ? pring->txcmplq_max : 0);
}

static DEVICE_ATTR(txcmplq_hw, S_IRUGO,
Expand Down
4 changes: 3 additions & 1 deletion drivers/scsi/lpfc/lpfc_bsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -2911,7 +2911,7 @@ static int lpfcdiag_loop_post_rxbufs(struct lpfc_hba *phba, uint16_t rxxri,
}
}

if (!cmdiocbq || !rxbmp || !rxbpl || !rxbuffer) {
if (!cmdiocbq || !rxbmp || !rxbpl || !rxbuffer || !pring) {
ret_val = -ENOMEM;
goto err_post_rxbufs_exit;
}
Expand Down Expand Up @@ -5421,6 +5421,8 @@ lpfc_bsg_timeout(struct bsg_job *job)
struct lpfc_iocbq *check_iocb, *next_iocb;

pring = lpfc_phba_elsring(phba);
if (unlikely(!pring))
return -EIO;

/* if job's driver data is NULL, the command completed or is in the
* the process of completing. In this case, return status to request
Expand Down
7 changes: 6 additions & 1 deletion drivers/scsi/lpfc/lpfc_els.c
Original file line number Diff line number Diff line change
Expand Up @@ -7430,6 +7430,8 @@ lpfc_els_timeout_handler(struct lpfc_vport *vport)
timeout = (uint32_t)(phba->fc_ratov << 1);

pring = lpfc_phba_elsring(phba);
if (unlikely(!pring))
return;

if ((phba->pport->load_flag & FC_UNLOADING))
return;
Expand Down Expand Up @@ -9310,6 +9312,9 @@ void lpfc_fabric_abort_nport(struct lpfc_nodelist *ndlp)

pring = lpfc_phba_elsring(phba);

if (unlikely(!pring))
return;

spin_lock_irq(&phba->hbalock);
list_for_each_entry_safe(piocb, tmp_iocb, &phba->fabric_iocb_list,
list) {
Expand Down Expand Up @@ -9416,7 +9421,7 @@ lpfc_sli4_els_xri_aborted(struct lpfc_hba *phba,
rxid, 1);

/* Check if TXQ queue needs to be serviced */
if (!(list_empty(&pring->txq)))
if (pring && !list_empty(&pring->txq))
lpfc_worker_wake_up(phba);
return;
}
Expand Down
5 changes: 4 additions & 1 deletion drivers/scsi/lpfc/lpfc_hbadisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3324,7 +3324,8 @@ lpfc_mbx_cmpl_read_topology(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb)

/* Unblock ELS traffic */
pring = lpfc_phba_elsring(phba);
pring->flag &= ~LPFC_STOP_IOCB_EVENT;
if (pring)
pring->flag &= ~LPFC_STOP_IOCB_EVENT;

/* Check for error */
if (mb->mbxStatus) {
Expand Down Expand Up @@ -5430,6 +5431,8 @@ lpfc_free_tx(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)

psli = &phba->sli;
pring = lpfc_phba_elsring(phba);
if (unlikely(!pring))
return;

/* Error matching iocb on txq or txcmplq
* First check the txq.
Expand Down
14 changes: 7 additions & 7 deletions drivers/scsi/lpfc/lpfc_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -11403,6 +11403,13 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
/* Remove FC host and then SCSI host with the physical port */
fc_remove_host(shost);
scsi_remove_host(shost);
/*
* Bring down the SLI Layer. This step disables all interrupts,
* clears the rings, discards all mailbox commands, and resets
* the HBA FCoE function.
*/
lpfc_debugfs_terminate(vport);
lpfc_sli4_hba_unset(phba);

/* Perform ndlp cleanup on the physical port. The nvme and nvmet
* localports are destroyed after to cleanup all transport memory.
Expand All @@ -11411,13 +11418,6 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
lpfc_nvmet_destroy_targetport(phba);
lpfc_nvme_destroy_localport(vport);

/*
* Bring down the SLI Layer. This step disables all interrupts,
* clears the rings, discards all mailbox commands, and resets
* the HBA FCoE function.
*/
lpfc_debugfs_terminate(vport);
lpfc_sli4_hba_unset(phba);

lpfc_stop_hba_timers(phba);
spin_lock_irq(&phba->hbalock);
Expand Down
2 changes: 1 addition & 1 deletion drivers/scsi/lpfc/lpfc_nportdisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
pring = lpfc_phba_elsring(phba);

/* In case of error recovery path, we might have a NULL pring here */
if (!pring)
if (unlikely(!pring))
return;

/* Abort outstanding I/O on NPort <nlp_DID> */
Expand Down
12 changes: 12 additions & 0 deletions drivers/scsi/lpfc/lpfc_sli.c
Original file line number Diff line number Diff line change
Expand Up @@ -10632,6 +10632,14 @@ lpfc_sli_issue_abort_iotag(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
(cmdiocb->iocb_flag & LPFC_DRIVER_ABORTED) != 0)
return 0;

if (!pring) {
if (cmdiocb->iocb_flag & LPFC_IO_FABRIC)
cmdiocb->fabric_iocb_cmpl = lpfc_ignore_els_cmpl;
else
cmdiocb->iocb_cmpl = lpfc_ignore_els_cmpl;
goto abort_iotag_exit;
}

/*
* If we're unloading, don't abort iocb on the ELS ring, but change
* the callback so that nothing happens when it finishes.
Expand Down Expand Up @@ -12500,6 +12508,8 @@ lpfc_sli4_els_wcqe_to_rspiocbq(struct lpfc_hba *phba,
unsigned long iflags;

pring = lpfc_phba_elsring(phba);
if (unlikely(!pring))
return NULL;

wcqe = &irspiocbq->cq_event.cqe.wcqe_cmpl;
spin_lock_irqsave(&pring->ring_lock, iflags);
Expand Down Expand Up @@ -18691,6 +18701,8 @@ lpfc_drain_txq(struct lpfc_hba *phba)
uint32_t txq_cnt = 0;

pring = lpfc_phba_elsring(phba);
if (unlikely(!pring))
return 0;

spin_lock_irqsave(&pring->ring_lock, iflags);
list_for_each_entry(piocbq, &pring->txq, list) {
Expand Down

0 comments on commit 1234a6d

Please sign in to comment.