From patchwork Sat Sep 30 00:34:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Smart X-Patchwork-Id: 9978867 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6534B6037F for ; Sat, 30 Sep 2017 00:35:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 57152298D1 for ; Sat, 30 Sep 2017 00:35:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4C1BA298D5; Sat, 30 Sep 2017 00:35:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C684E298D1 for ; Sat, 30 Sep 2017 00:35:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752607AbdI3AfB (ORCPT ); Fri, 29 Sep 2017 20:35:01 -0400 Received: from mail-qt0-f194.google.com ([209.85.216.194]:48464 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752594AbdI3Ae7 (ORCPT ); Fri, 29 Sep 2017 20:34:59 -0400 Received: by mail-qt0-f194.google.com with SMTP id d13so932638qta.5; Fri, 29 Sep 2017 17:34:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=d03I6Zxf0wYfBcWKHPdJDRavUSk9f4cmsd6yfCaGW5M=; b=hrhOS6E4SEGDtZ5R/wjh0QVSRuD5BxHFr4EPzbgu6rXudBncmfew9eS40ZiLciLz3P J+i9OKoLLFY3e+Hp6YTYqeweGonMdQSvL4GiBhRFNeANCtrmiX8+ePgjTuyxkpB2Pebq hzPXCWDr6vrkqlz4icWTaKrRQzump90Rx0zIUMJkiMrsCG8bL3/8j2k17DxjBGyqvUwi NjHFyMixsbA4Aqtge4BryjVQ6rwFY4z0utPUrHzZKpR/JzSvaDJb68jQMRp/WKp81rhB CQpbEnIUay+NuOGcveX0BtQyKTgJ/pISdmlmFH8sJh9HR1K83IIt3uUgEXn9m/4IUR9r /Ryg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=d03I6Zxf0wYfBcWKHPdJDRavUSk9f4cmsd6yfCaGW5M=; b=cBvZJz27PBUgChiLNrb5/6ONZ6UQ1/fk5vWlTG9XP33oe32wPJRWh2ciaDe5pn01Xa wmw+UnY2YrOKVZJJryaHv7+bHAksggGQkuKdQJNZpKUCizD7Z+cWKsmQEUEIFVNo5IoU m6tv5aUIy0wD3u4SboRPR56a+gUiHQXl2yHx6W537Jku8nF9UhZl2qAs0X+xCJ+rzpzf hL3wqlfmk72DBY3z7pbyyaJNe6yM4Z0v64fRN8xIpBVq5CFrNSWPtCcbq+77kkzCNVYY KEO+MB7vhJRpAc9Yy3lYa9ClwhDP+YztsMLL9JUK/k5cJPf3scWAMs5uhbpT64Q2jk0u 77Gw== X-Gm-Message-State: AMCzsaUs0XPICQzBxSzcRgqfllbvjDm6DIayh/CbbPVmxErMep5usymJ wpMhzm4mveVkdqAfuMb/ysj96w== X-Google-Smtp-Source: AOwi7QDJvsgjuIfqf1x1XZOatCsHv4ELwJvFJ1f0Dz93IbCsIt/cG2p/GxcNqqd9/QUFoYYUoQ7MGg== X-Received: by 10.200.52.166 with SMTP id w35mr9188923qtb.305.1506731698108; Fri, 29 Sep 2017 17:34:58 -0700 (PDT) Received: from pallmd1.broadcom.com ([192.19.255.250]) by smtp.gmail.com with ESMTPSA id u32sm3596918qte.25.2017.09.29.17.34.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 29 Sep 2017 17:34:57 -0700 (PDT) From: James Smart To: linux-scsi@vger.kernel.org Cc: Dick Kennedy , stable@vger.kernel.org, James Smart Subject: [PATCH v2 03/21] lpfc: Fix crash receiving ELS while detaching driver Date: Fri, 29 Sep 2017 17:34:29 -0700 Message-Id: <20170930003447.10747-4-jsmart2021@gmail.com> X-Mailer: git-send-email 2.13.1 In-Reply-To: <20170930003447.10747-1-jsmart2021@gmail.com> References: <20170930003447.10747-1-jsmart2021@gmail.com> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Dick Kennedy 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: # 4.12+ Signed-off-by: Dick Kennedy Signed-off-by: James Smart Reviewed-by: Johannes Thumshirn --- drivers/scsi/lpfc/lpfc_attr.c | 6 ++++-- drivers/scsi/lpfc/lpfc_bsg.c | 4 +++- drivers/scsi/lpfc/lpfc_els.c | 7 ++++++- drivers/scsi/lpfc/lpfc_hbadisc.c | 5 ++++- drivers/scsi/lpfc/lpfc_init.c | 14 +++++++------- drivers/scsi/lpfc/lpfc_nportdisc.c | 2 +- drivers/scsi/lpfc/lpfc_sli.c | 12 ++++++++++++ 7 files changed, 37 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c index 0806323829e6..f02269b46049 100644 --- a/drivers/scsi/lpfc/lpfc_attr.c +++ b/drivers/scsi/lpfc/lpfc_attr.c @@ -3132,7 +3132,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, @@ -3145,7 +3146,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, diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c index fe9e1c079c20..d89816222b23 100644 --- a/drivers/scsi/lpfc/lpfc_bsg.c +++ b/drivers/scsi/lpfc/lpfc_bsg.c @@ -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; } @@ -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 diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c index 468a66371de9..3ebf6ccba6e6 100644 --- a/drivers/scsi/lpfc/lpfc_els.c +++ b/drivers/scsi/lpfc/lpfc_els.c @@ -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; @@ -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) { @@ -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; } diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c index 20808349a80e..499df9d17339 100644 --- a/drivers/scsi/lpfc/lpfc_hbadisc.c +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c @@ -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) { @@ -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. diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index 1773b9ce3149..b50c3b559a7a 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -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. @@ -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); diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c b/drivers/scsi/lpfc/lpfc_nportdisc.c index f3ad7cac355d..b6957d944b9a 100644 --- a/drivers/scsi/lpfc/lpfc_nportdisc.c +++ b/drivers/scsi/lpfc/lpfc_nportdisc.c @@ -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 */ diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c index b8513c1adcef..9024521b97c7 100644 --- a/drivers/scsi/lpfc/lpfc_sli.c +++ b/drivers/scsi/lpfc/lpfc_sli.c @@ -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. @@ -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); @@ -18694,6 +18704,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) {