[PATCH v2 1/1] scsi: lpfc: Clear rport node association before unregister

Pulling an FC cable can drive lpfc through remote-port devloss teardown.
In one observed crash, the SCSI transport workqueue called
lpfc_dev_loss_tmo_callbk() from fc_rport_final_delete(), and lpfc faulted
while dereferencing the ndlp-derived vport:

  Workqueue: fc_wq_19 fc_rport_final_delete [scsi_transport_fc]
  RIP: lpfc_dev_loss_tmo_callbk+0x58/0x48d [lpfc]

Call trace:

lpfc_dev_loss_tmo_callbk+0x58/0x48d [lpfc]
fc_rport_final_delete+0xeb/0x1b0 [scsi_transport_fc]
process_one_work+0x174/0x31a
worker_thread+0x191/0x300
kthread+0xcf/0x100
ret_from_fork+0x31/0x50
ret_from_fork_asm+0x1a/0x30

The rport private data still contained a non-NULL pnode, but that pointer
could refer to an ndlp whose lifetime had already ended. The callback then
treated the stale pnode as valid and dereferenced fields such as
ndlp->vport.

When lpfc unregisters a SCSI remote port, it calls fc_remote_port_delete()
and then drops the ndlp reference that was taken when rdata->pnode was set
during registration. However, the rport private pnode field was left intact
until the transport devloss callback path cleared it. This leaves a window
where a later transport callback can observe a stale pnode after the ndlp
reference has been released.

This patch detaches the lpfc node from the rport before calling
fc_remote_port_delete() and before dropping the transport-held ndlp
reference. Snapshot ndlp->rport under ndlp->lock, clear rdata->pnode,
clear ndlp->rport, and clear the SCSI transport registration flag while
holding the lock. Also make the devloss callback tolerate a NULL rport
or rport private data and use READ_ONCE() / WRITE_ONCE() for the lockless
pnode handoff.

This makes the unregister path match the callback teardown path: once lpfc
has severed the rport-to-ndlp association, later devloss callbacks see a
NULL pnode and return instead of touching a released node.

Assisted-by: Codex:gpt-5
Signed-off-by: Dai Ngo <[email protected]>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

V2:
  . in lpfc_unregister_remote_port(), snapshot ndlp->rport under ndlp->lock,
    clear rdata->pnode, clear ndlp->rport, and clear the SCSI transport
    registration flag while holding the ndlp->lock.

  . in lpfc_dev_loss_tmo_callbk, check rport's private data before
    using it and and use READ_ONCE() / WRITE_ONCE() for the lockless
    pnode handoff.

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 4f68038789b5..a04222ab4092 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -157,13 +157,21 @@ void
 lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 {
 	struct lpfc_nodelist *ndlp;
+	struct lpfc_rport_data *rdata;
 	struct lpfc_vport *vport;
 	struct lpfc_hba   *phba;
 	struct lpfc_work_evt *evtp;
 	unsigned long iflags;
 	bool drop_initial_node_ref = false;
 
-	ndlp = ((struct lpfc_rport_data *)rport->dd_data)->pnode;
+	if (!rport)
+		return;
+
+	rdata = rport->dd_data;
+	if (!rdata)
+		return;
+
+	ndlp = READ_ONCE(rdata->pnode);
 	if (!ndlp)
 		return;
 
@@ -187,6 +195,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	    !test_bit(HBA_SETUP, &phba->hba_flag))) {
 
 		spin_lock_irqsave(&ndlp->lock, iflags);
+		WRITE_ONCE(rdata->pnode, NULL);
 		ndlp->rport = NULL;
 
 		/* Only 1 thread can drop the initial node reference.
@@ -271,7 +280,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	 * rport. Remove the association between rport and ndlp.
 	 */
 	ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
-	((struct lpfc_rport_data *)rport->dd_data)->pnode = NULL;
+	WRITE_ONCE(((struct lpfc_rport_data *)rport->dd_data)->pnode, NULL);
 	ndlp->rport = NULL;
 	spin_unlock_irqrestore(&ndlp->lock, iflags);
 
@@ -4553,8 +4562,10 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 static void
 lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
 {
-	struct fc_rport *rport = ndlp->rport;
+	struct fc_rport *rport;
 	struct lpfc_vport *vport = ndlp->vport;
+	struct lpfc_rport_data *rdata;
+	unsigned long iflags;
 
 	if (vport->cfg_enable_fc4_type == LPFC_ENABLE_NVME)
 		return;
@@ -4566,9 +4577,22 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
 	lpfc_printf_vlog(vport, KERN_INFO, LOG_NODE,
 			 "3184 rport unregister x%06x, rport x%px "
 			 "xptflg x%x refcnt %d\n",
-			 ndlp->nlp_DID, rport, ndlp->fc4_xpt_flags,
+			 ndlp->nlp_DID, ndlp->rport, ndlp->fc4_xpt_flags,
 			 kref_read(&ndlp->kref));
 
+	spin_lock_irqsave(&ndlp->lock, iflags);
+	rport = ndlp->rport;
+	if (!rport) {
+		spin_unlock_irqrestore(&ndlp->lock, iflags);
+		return;
+	}
+	rdata = rport->dd_data;
+	if (rdata && READ_ONCE(rdata->pnode) == ndlp)
+		WRITE_ONCE(rdata->pnode, NULL);
+	ndlp->rport = NULL;
+	ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+	spin_unlock_irqrestore(&ndlp->lock, iflags);
+
 	fc_remote_port_delete(rport);
 	lpfc_nlp_put(ndlp);
 }
-- 
2.47.3