* [PATCH 5.15.y] scsi: lpfc: Fix null pointer dereference after failing to issue FLOGI and PLOGI
@ 2025-04-23 11:56 bin.lan.cn
2025-04-23 17:47 ` Sasha Levin
0 siblings, 1 reply; 2+ messages in thread
From: bin.lan.cn @ 2025-04-23 11:56 UTC (permalink / raw)
To: gregkh, stable; +Cc: bin.lan.cn, justin.tee, jsmart2021, martin.petersen
From: James Smart <jsmart2021@gmail.com>
[ Upstream commit 577a942df3de2666f6947bdd3a5c9e8d30073424 ]
If lpfc_issue_els_flogi() fails and returns non-zero status, the node
reference count is decremented to trigger the release of the nodelist
structure. However, if there is a prior registration or dev-loss-evt work
pending, the node may be released prematurely. When dev-loss-evt
completes, the released node is referenced causing a use-after-free null
pointer dereference.
Similarly, when processing non-zero ELS PLOGI completion status in
lpfc_cmpl_els_plogi(), the ndlp flags are checked for a transport
registration before triggering node removal. If dev-loss-evt work is
pending, the node may be released prematurely and a subsequent call to
lpfc_dev_loss_tmo_handler() results in a use after free ndlp dereference.
Add test for pending dev-loss before decrementing the node reference count
for FLOGI, PLOGI, PRLI, and ADISC handling.
Link: https://lore.kernel.org/r/20220412222008.126521-9-jsmart2021@gmail.com
Co-developed-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
[Minor context change fixed.]
Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
Build test passed.
---
drivers/scsi/lpfc/lpfc_els.c | 51 +++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 5f44a0763f37..134d56bd00da 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -1517,10 +1517,13 @@ lpfc_initial_flogi(struct lpfc_vport *vport)
}
if (lpfc_issue_els_flogi(vport, ndlp, 0)) {
- /* This decrement of reference count to node shall kick off
- * the release of the node.
+ /* A node reference should be retained while registered with a
+ * transport or dev-loss-evt work is pending.
+ * Otherwise, decrement node reference to trigger release.
*/
- lpfc_nlp_put(ndlp);
+ if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)) &&
+ !(ndlp->nlp_flag & NLP_IN_DEV_LOSS))
+ lpfc_nlp_put(ndlp);
return 0;
}
return 1;
@@ -1563,10 +1566,13 @@ lpfc_initial_fdisc(struct lpfc_vport *vport)
}
if (lpfc_issue_els_fdisc(vport, ndlp, 0)) {
- /* decrement node reference count to trigger the release of
- * the node.
+ /* A node reference should be retained while registered with a
+ * transport or dev-loss-evt work is pending.
+ * Otherwise, decrement node reference to trigger release.
*/
- lpfc_nlp_put(ndlp);
+ if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)) &&
+ !(ndlp->nlp_flag & NLP_IN_DEV_LOSS))
+ lpfc_nlp_put(ndlp);
return 0;
}
return 1;
@@ -1967,6 +1973,7 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
struct lpfc_dmabuf *prsp;
int disc;
struct serv_parm *sp = NULL;
+ bool release_node = false;
/* we pass cmdiocb to state machine which needs rspiocb as well */
cmdiocb->context_un.rsp_iocb = rspiocb;
@@ -2047,19 +2054,21 @@ lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
spin_unlock_irq(&ndlp->lock);
goto out;
}
- spin_unlock_irq(&ndlp->lock);
/* No PLOGI collision and the node is not registered with the
* scsi or nvme transport. It is no longer an active node. Just
* start the device remove process.
*/
if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD))) {
- spin_lock_irq(&ndlp->lock);
ndlp->nlp_flag &= ~NLP_NPR_2B_DISC;
- spin_unlock_irq(&ndlp->lock);
+ if (!(ndlp->nlp_flag & NLP_IN_DEV_LOSS))
+ release_node = true;
+ }
+ spin_unlock_irq(&ndlp->lock);
+
+ if (release_node)
lpfc_disc_state_machine(vport, ndlp, cmdiocb,
NLP_EVT_DEVICE_RM);
- }
} else {
/* Good status, call state machine */
prsp = list_entry(((struct lpfc_dmabuf *)
@@ -2269,6 +2278,7 @@ lpfc_cmpl_els_prli(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
struct lpfc_nodelist *ndlp;
char *mode;
u32 loglevel;
+ bool release_node = false;
/* we pass cmdiocb to state machine which needs rspiocb as well */
cmdiocb->context_un.rsp_iocb = rspiocb;
@@ -2335,14 +2345,18 @@ lpfc_cmpl_els_prli(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
* it is no longer an active node. Otherwise devloss
* handles the final cleanup.
*/
+ spin_lock_irq(&ndlp->lock);
if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD)) &&
!ndlp->fc4_prli_sent) {
- spin_lock_irq(&ndlp->lock);
ndlp->nlp_flag &= ~NLP_NPR_2B_DISC;
- spin_unlock_irq(&ndlp->lock);
+ if (!(ndlp->nlp_flag & NLP_IN_DEV_LOSS))
+ release_node = true;
+ }
+ spin_unlock_irq(&ndlp->lock);
+
+ if (release_node)
lpfc_disc_state_machine(vport, ndlp, cmdiocb,
NLP_EVT_DEVICE_RM);
- }
} else {
/* Good status, call state machine. However, if another
* PRLI is outstanding, don't call the state machine
@@ -2713,6 +2727,7 @@ lpfc_cmpl_els_adisc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
IOCB_t *irsp;
struct lpfc_nodelist *ndlp;
int disc;
+ bool release_node = false;
/* we pass cmdiocb to state machine which needs rspiocb as well */
cmdiocb->context_un.rsp_iocb = rspiocb;
@@ -2771,13 +2786,17 @@ lpfc_cmpl_els_adisc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
* transport, it is no longer an active node. Otherwise
* devloss handles the final cleanup.
*/
+ spin_lock_irq(&ndlp->lock);
if (!(ndlp->fc4_xpt_flags & (SCSI_XPT_REGD | NVME_XPT_REGD))) {
- spin_lock_irq(&ndlp->lock);
ndlp->nlp_flag &= ~NLP_NPR_2B_DISC;
- spin_unlock_irq(&ndlp->lock);
+ if (!(ndlp->nlp_flag & NLP_IN_DEV_LOSS))
+ release_node = true;
+ }
+ spin_unlock_irq(&ndlp->lock);
+
+ if (release_node)
lpfc_disc_state_machine(vport, ndlp, cmdiocb,
NLP_EVT_DEVICE_RM);
- }
} else
/* Good status, call state machine */
lpfc_disc_state_machine(vport, ndlp, cmdiocb,
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH 5.15.y] scsi: lpfc: Fix null pointer dereference after failing to issue FLOGI and PLOGI
2025-04-23 11:56 [PATCH 5.15.y] scsi: lpfc: Fix null pointer dereference after failing to issue FLOGI and PLOGI bin.lan.cn
@ 2025-04-23 17:47 ` Sasha Levin
0 siblings, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-04-23 17:47 UTC (permalink / raw)
To: stable; +Cc: bin.lan.cn, Sasha Levin
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected.
No action required from the submitter.
The upstream commit SHA1 provided is correct: 577a942df3de2666f6947bdd3a5c9e8d30073424
WARNING: Author mismatch between patch and upstream commit:
Backport author: bin.lan.cn@windriver.com
Commit author: James Smart<jsmart2021@gmail.com>
Status in newer kernel trees:
6.14.y | Present (exact SHA1)
6.12.y | Present (exact SHA1)
6.6.y | Present (exact SHA1)
6.1.y | Present (exact SHA1)
Note: The patch differs from the upstream commit:
---
1: 577a942df3de2 ! 1: 84e3972d52302 scsi: lpfc: Fix null pointer dereference after failing to issue FLOGI and PLOGI
@@ Metadata
## Commit message ##
scsi: lpfc: Fix null pointer dereference after failing to issue FLOGI and PLOGI
+ [ Upstream commit 577a942df3de2666f6947bdd3a5c9e8d30073424 ]
+
If lpfc_issue_els_flogi() fails and returns non-zero status, the node
reference count is decremented to trigger the release of the nodelist
structure. However, if there is a prior registration or dev-loss-evt work
@@ Commit message
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
+ [Minor context change fixed.]
+ Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
+ Signed-off-by: He Zhe <zhe.he@windriver.com>
## drivers/scsi/lpfc/lpfc_els.c ##
@@ drivers/scsi/lpfc/lpfc_els.c: lpfc_initial_flogi(struct lpfc_vport *vport)
- /* Reset the Fabric flag, topology change may have happened */
- vport->fc_flag &= ~FC_FABRIC;
+ }
+
if (lpfc_issue_els_flogi(vport, ndlp, 0)) {
- /* This decrement of reference count to node shall kick off
- * the release of the node.
@@ drivers/scsi/lpfc/lpfc_els.c: lpfc_initial_fdisc(struct lpfc_vport *vport)
}
return 1;
@@ drivers/scsi/lpfc/lpfc_els.c: lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
+ struct lpfc_dmabuf *prsp;
int disc;
struct serv_parm *sp = NULL;
- u32 ulp_status, ulp_word4, did, iotag;
+ bool release_node = false;
/* we pass cmdiocb to state machine which needs rspiocb as well */
@@ drivers/scsi/lpfc/lpfc_els.c: lpfc_cmpl_els_plogi(struct lpfc_hba *phba, struct
/* Good status, call state machine */
prsp = list_entry(((struct lpfc_dmabuf *)
@@ drivers/scsi/lpfc/lpfc_els.c: lpfc_cmpl_els_prli(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
+ struct lpfc_nodelist *ndlp;
+ char *mode;
u32 loglevel;
- u32 ulp_status;
- u32 ulp_word4;
+ bool release_node = false;
/* we pass cmdiocb to state machine which needs rspiocb as well */
@@ drivers/scsi/lpfc/lpfc_els.c: lpfc_cmpl_els_prli(struct lpfc_hba *phba, struct l
/* Good status, call state machine. However, if another
* PRLI is outstanding, don't call the state machine
@@ drivers/scsi/lpfc/lpfc_els.c: lpfc_cmpl_els_adisc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
+ IOCB_t *irsp;
struct lpfc_nodelist *ndlp;
int disc;
- u32 ulp_status, ulp_word4, tmo;
+ bool release_node = false;
/* we pass cmdiocb to state machine which needs rspiocb as well */
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y | Success | Success |
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-04-23 17:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 11:56 [PATCH 5.15.y] scsi: lpfc: Fix null pointer dereference after failing to issue FLOGI and PLOGI bin.lan.cn
2025-04-23 17:47 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox