Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1528370 > unrolled thread

[PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put()

Started byMauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
First post2016-11-23 13:40 +0100
Last post2016-11-23 16:50 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put() Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> - 2016-11-23 13:40 +0100
    Re: [PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put() Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> - 2016-11-23 13:50 +0100
    Re: [PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put() Johannes Thumshirn <jthumshirn@suse.de> - 2016-11-23 15:20 +0100
      Re: [PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put() Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> - 2016-11-23 16:50 +0100

#1528370 — [PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put()

FromMauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Date2016-11-23 13:40 +0100
Subject[PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put()
Message-ID<sGF0d-4Vd-19@gated-at.bofh.it>
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
    <...>

Cc: stable@vger.kernel.org # v4.8
Fixes: 22466da5b4b7 ("lpfc: Fix possible NULL pointer dereference")
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 drivers/scsi/lpfc/lpfc_sli.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index c5326055beee..f4f77c5b0c83 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -1323,18 +1323,20 @@ struct lpfc_iocbq *
 {
 	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;
 }
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1528377

FromMauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Date2016-11-23 13:50 +0100
Message-ID<sGF9T-4Yp-15@gated-at.bofh.it>
In reply to#1528370
Due credit; an oversight.

On 11/23/2016 10:33 AM, Mauricio Faria de Oliveira wrote:

Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>

> Cc: stable@vger.kernel.org # v4.8
> Fixes: 22466da5b4b7 ("lpfc: Fix possible NULL pointer dereference")
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>



-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center

[toc] | [prev] | [next] | [standalone]

#1528435

FromJohannes Thumshirn <jthumshirn@suse.de>
Date2016-11-23 15:20 +0100
Message-ID<sGGz0-5Zq-11@gated-at.bofh.it>
In reply to#1528370
On Wed, Nov 23, 2016 at 10:33:19AM -0200, Mauricio Faria de Oliveira wrote:
> 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
>     <...>
> 
> Cc: stable@vger.kernel.org # v4.8
> Fixes: 22466da5b4b7 ("lpfc: Fix possible NULL pointer dereference")
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> ---

Looks good and sorry for the bug,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[toc] | [prev] | [next] | [standalone]

#1528528

FromMauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Date2016-11-23 16:50 +0100
Message-ID<sGHY5-6Gh-9@gated-at.bofh.it>
In reply to#1528435
On 11/23/2016 12:12 PM, Johannes Thumshirn wrote:
> Looks good and sorry for the bug,
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

Thanks for the quick review. Not a problem!
This problem turned out to be a good learning exercise. :)


-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web