Skip to content

Commit 2319f84

Browse files
Mauricio Faria de Oliveiramartinkpetersen
authored andcommitted
scsi: lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put()
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: 22466da ("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>
1 parent 23b98e4 commit 2319f84

1 file changed

Lines changed: 8 additions & 6 deletions

File tree

drivers/scsi/lpfc/lpfc_sli.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,18 +1323,20 @@ lpfc_sli_ringtxcmpl_put(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
13231323
{
13241324
lockdep_assert_held(&phba->hbalock);
13251325

1326-
BUG_ON(!piocb || !piocb->vport);
1326+
BUG_ON(!piocb);
13271327

13281328
list_add_tail(&piocb->list, &pring->txcmplq);
13291329
piocb->iocb_flag |= LPFC_IO_ON_TXCMPLQ;
13301330

13311331
if ((unlikely(pring->ringno == LPFC_ELS_RING)) &&
13321332
(piocb->iocb.ulpCommand != CMD_ABORT_XRI_CN) &&
1333-
(piocb->iocb.ulpCommand != CMD_CLOSE_XRI_CN) &&
1334-
(!(piocb->vport->load_flag & FC_UNLOADING)))
1335-
mod_timer(&piocb->vport->els_tmofunc,
1336-
jiffies +
1337-
msecs_to_jiffies(1000 * (phba->fc_ratov << 1)));
1333+
(piocb->iocb.ulpCommand != CMD_CLOSE_XRI_CN)) {
1334+
BUG_ON(!piocb->vport);
1335+
if (!(piocb->vport->load_flag & FC_UNLOADING))
1336+
mod_timer(&piocb->vport->els_tmofunc,
1337+
jiffies +
1338+
msecs_to_jiffies(1000 * (phba->fc_ratov << 1)));
1339+
}
13381340

13391341
return 0;
13401342
}

0 commit comments

Comments
 (0)