stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Harsha Thyagaraja <hathyaga@in.ibm.com>,
	Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>,
	Johannes Thumshirn <jthumshirn@suse.de>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Subject: [PATCH 4.8 12/33] scsi: lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put()
Date: Tue, 13 Dec 2016 09:16:13 -0800	[thread overview]
Message-ID: <20161213171534.688694356@linuxfoundation.org> (raw)
In-Reply-To: <20161213171534.171564506@linuxfoundation.org>

4.8-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>

commit 2319f847a8910cff1d46c9b66aa1dd7cc3e836a9 upstream.

The BUG_ON() recently introduced in lpfc_sli_ringtxcmpl_put() is hit in
the lpfc_els_abort() > lpfc_sli_issue_abort_iotag() >
lpfc_sli_abort_iotag_issue() function path [similar names], due to
'piocb->vport == NULL':

	BUG_ON(!piocb || !piocb->vport);

This happens because lpfc_sli_abort_iotag_issue() doesn't set the
'abtsiocbp->vport' pointer -- but this is not the problem.

Previously, lpfc_sli_ringtxcmpl_put() accessed 'piocb->vport' only if
'piocb->iocb.ulpCommand' is neither CMD_ABORT_XRI_CN nor
CMD_CLOSE_XRI_CN, which are the only possible values for
lpfc_sli_abort_iotag_issue():

    lpfc_sli_ringtxcmpl_put():

        if ((unlikely(pring->ringno == LPFC_ELS_RING)) &&
           (piocb->iocb.ulpCommand != CMD_ABORT_XRI_CN) &&
           (piocb->iocb.ulpCommand != CMD_CLOSE_XRI_CN) &&
            (!(piocb->vport->load_flag & FC_UNLOADING)))

    lpfc_sli_abort_iotag_issue():

        if (phba->link_state >= LPFC_LINK_UP)
                iabt->ulpCommand = CMD_ABORT_XRI_CN;
        else
                iabt->ulpCommand = CMD_CLOSE_XRI_CN;

So, this function path would not have hit this possible NULL pointer
dereference before.

In order to fix this regression, move the second part of the BUG_ON()
check prior to the pointer dereference that it does check for.

For reference, this is the stack trace observed. The problem happened
because an unsolicited event was received - a PLOGI was received after
our PLOGI was issued but not yet complete, so the discovery state
machine goes on to sw-abort our PLOGI.

    kernel BUG at drivers/scsi/lpfc/lpfc_sli.c:1326!
    Oops: Exception in kernel mode, sig: 5 [#1]
    <...>
    NIP [...] lpfc_sli_ringtxcmpl_put+0x1c/0xf0 [lpfc]
    LR  [...] __lpfc_sli_issue_iocb_s4+0x188/0x200 [lpfc]
    Call Trace:
    [...] [...] __lpfc_sli_issue_iocb_s4+0xb0/0x200 [lpfc] (unreliable)
    [...] [...] lpfc_sli_issue_abort_iotag+0x2b4/0x350 [lpfc]
    [...] [...] lpfc_els_abort+0x1a8/0x4a0 [lpfc]
    [...] [...] lpfc_rcv_plogi+0x6d4/0x700 [lpfc]
    [...] [...] lpfc_rcv_plogi_plogi_issue+0xd8/0x1d0 [lpfc]
    [...] [...] lpfc_disc_state_machine+0xc0/0x2b0 [lpfc]
    [...] [...] lpfc_els_unsol_buffer+0xcc0/0x26c0 [lpfc]
    [...] [...] lpfc_els_unsol_event+0xa8/0x220 [lpfc]
    [...] [...] lpfc_complete_unsol_iocb+0xb8/0x138 [lpfc]
    [...] [...] lpfc_sli4_handle_received_buffer+0x6a0/0xec0 [lpfc]
    [...] [...] lpfc_sli_handle_slow_ring_event_s4+0x1c4/0x240 [lpfc]
    [...] [...] lpfc_sli_handle_slow_ring_event+0x24/0x40 [lpfc]
    [...] [...] lpfc_do_work+0xd88/0x1970 [lpfc]
    [...] [...] kthread+0x108/0x130
    [...] [...] ret_from_kernel_thread+0x5c/0xbc
    <...>

Fixes: 22466da5b4b7 ("lpfc: Fix possible NULL pointer dereference")
Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/scsi/lpfc/lpfc_sli.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -1323,18 +1323,20 @@ lpfc_sli_ringtxcmpl_put(struct lpfc_hba
 {
 	lockdep_assert_held(&phba->hbalock);
 
-	BUG_ON(!piocb || !piocb->vport);
+	BUG_ON(!piocb);
 
 	list_add_tail(&piocb->list, &pring->txcmplq);
 	piocb->iocb_flag |= LPFC_IO_ON_TXCMPLQ;
 
 	if ((unlikely(pring->ringno == LPFC_ELS_RING)) &&
 	   (piocb->iocb.ulpCommand != CMD_ABORT_XRI_CN) &&
-	   (piocb->iocb.ulpCommand != CMD_CLOSE_XRI_CN) &&
-	    (!(piocb->vport->load_flag & FC_UNLOADING)))
-		mod_timer(&piocb->vport->els_tmofunc,
-			  jiffies +
-			  msecs_to_jiffies(1000 * (phba->fc_ratov << 1)));
+	   (piocb->iocb.ulpCommand != CMD_CLOSE_XRI_CN)) {
+		BUG_ON(!piocb->vport);
+		if (!(piocb->vport->load_flag & FC_UNLOADING))
+			mod_timer(&piocb->vport->els_tmofunc,
+				  jiffies +
+				  msecs_to_jiffies(1000 * (phba->fc_ratov << 1)));
+	}
 
 	return 0;
 }



  parent reply	other threads:[~2016-12-13 17:16 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20161213171629epcas1p4dc2737fb27df6f1d6148edf348dde1c7@epcas1p4.samsung.com>
2016-12-13 17:16 ` [PATCH 4.8 00/33] 4.8.15-stable review Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 01/33] powerpc/eeh: Fix deadlock when PE frozen state cant be cleared Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 02/33] powerpc/mm: Fix lazy icache flush on pre-POWER5 Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 03/33] powerpc/boot: Fix build failure in 32-bit boot wrapper Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 04/33] fuse: fix clearing suid, sgid for chown() Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 05/33] parisc: Purge TLB before setting PTE Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 06/33] parisc: Remove unnecessary TLB purges from flush_dcache_page_asm and flush_icache_page_asm Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 07/33] parisc: Fix TLB related boot crash on SMP machines Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 08/33] zram: restrict add/remove attributes to root only Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 09/33] locking/rtmutex: Prevent dequeue vs. unlock race Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 10/33] locking/rtmutex: Use READ_ONCE() in rt_mutex_owner() Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 11/33] device-dax: fix private mapping restriction, permit read-only Greg Kroah-Hartman
2016-12-13 17:16   ` Greg Kroah-Hartman [this message]
2016-12-13 17:16   ` [PATCH 4.8 13/33] sched/autogroup: Fix 64-bit kernel nice level adjustment Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 14/33] vhost-vsock: fix orphan connection reset Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 15/33] perf/x86: Fix full width counter, counter overflow Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 16/33] acpi, nfit: fix extended status translations for ACPI DSMs Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 17/33] acpi, nfit, libnvdimm: fix / harden ars_status output length handling Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 18/33] acpi, nfit: validate ars_status output buffer size Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 19/33] acpi, nfit: fix bus vs dimm confusion in xlat_status Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 20/33] crypto: marvell - Dont copy hash operation twice into the SRAM Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 22/33] crypto: mcryptd - Check mcryptd algorithm compatibility Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 23/33] crypto: marvell - Dont corrupt state of an STD req for re-stepped ahash Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 24/33] can: raw: raw_setsockopt: limit number of can_filter that can be set Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 26/33] Revert "ACPI: Execute _PTS before system reboot" Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 27/33] ARM: dts: orion5x: fix number of sata port for linkstation ls-gl Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 28/33] ARM: dts: imx7d: fix LCDIF clock assignment Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 29/33] ceph: dont set req->r_locked_dir in ceph_d_revalidate Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 30/33] m68k: Fix ndelay() macro Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 31/33] batman-adv: Check for alloc errors when preparing TT local data Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 32/33] hotplug: Make register and unregister notifier API symmetric Greg Kroah-Hartman
2016-12-13 17:16   ` [PATCH 4.8 33/33] crypto: rsa - Add Makefile dependencies to fix parallel builds Greg Kroah-Hartman
2016-12-13 19:32   ` [PATCH 4.8 00/33] 4.8.15-stable review Shuah Khan
2016-12-13 20:07     ` Greg Kroah-Hartman
2016-12-14  4:13   ` Guenter Roeck
2016-12-14 12:49     ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161213171534.688694356@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=hathyaga@in.ibm.com \
    --cc=jthumshirn@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mauricfo@linux.vnet.ibm.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).