[PATCH v5 1/1] scsi: lpfc: defer SCSI rport node put until devloss callback

[PATCH v5 1/1] scsi: lpfc: defer SCSI rport node put until devloss callback

[Dai Ngo]

lpfc_register_remote_port() stores an lpfc_nodelist pointer in the SCSI
transport rport private data and takes a node reference for that
association.

lpfc_unregister_remote_port() calls fc_remote_port_delete(), which start
SCSI transport devloss processing.  The rport remains usable until that
processing completes: the transport can still invoke
lpfc_terminate_rport_io() and lpfc_dev_loss_tmo_callbk(), both of which
obtain the lpfc node through rport->dd_data->pnode.

Do not drop the association reference immediately after
fc_remote_port_delete().  Retain it until lpfc_dev_loss_tmo_callbk()
detaches the rport and rport private data from the node, clears
SCSI_XPT_REGD, and drops the association reference.  This prevents the
node from being freed while the transport still has callbacks pending.

An rport can reappear while devloss processing is in progress.  Before
calling fc_remote_port_add(), acquire a provisional node reference.
When the transport reuses the prior fc_rport and its old SCSI association
remains registered, transfer that old reference to the new association
and release the provisional one.  Otherwise retain the provisional
reference because the old association was already released by devloss.

Associate a generation with each rport private-data assignment. A
devloss callback captures the generation and, while holding ndlp->lock,
verifies the pnode, generation, and rport before changing association
state or dropping a reference.  Thus, an old callback cannot operate
on a reused rport after the transport has cleared and reinitialized
its private data.

Set NLP_IN_DEV_LOSS only after these ownership checks succeed.  A stale
callback therefore cannot leave the flag set and suppress a later
node-reference release.

This preserves one node reference for each active SCSI rport association
and closes the devloss callback use-after-free and rport-reuse races.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 drivers/scsi/lpfc/lpfc_disc.h    |   1 +
 drivers/scsi/lpfc/lpfc_hbadisc.c | 139 ++++++++++++++++++++++++++-----
 drivers/scsi/lpfc/lpfc_scsi.h    |   1 +
 3 files changed, 121 insertions(+), 20 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.

V3:
. Redo the patch based on Paul's review.
Delay dropping the SCSI rport node reference until
lpfc_dev_loss_tmo_callbk() detaches the rport from the lpfc node.

V4:
. Fix the node lead reference when the remote port reconnects before
dev_loss_tmo expires. If fc_remote_port_add() reuses the existing SCSI
rport, transfer the reference that is still held on the rport instead
of acquiring a new reference.

V5:
. Fix devloss callback use-after-free and rport-reuse races identified
in saskiko review by introducing rport association generation number.
lpfc_dev_loss_tmo_callbk checks the generation number verifies the pnode,
generation, and rport before changing association state or dropping a
reference.

diff --git a/drivers/scsi/lpfc/lpfc_disc.h b/drivers/scsi/lpfc/lpfc_disc.h
index 51cb8571c049..4236e2397357 100644
--- a/drivers/scsi/lpfc/lpfc_disc.h
+++ b/drivers/scsi/lpfc/lpfc_disc.h
@@ -141,6 +141,7 @@ struct lpfc_nodelist {
 	struct timer_list   nlp_delayfunc;	/* Used for delayed ELS cmds */
 	struct lpfc_hba *phba;
 	struct fc_rport *rport;		/* scsi_transport_fc port structure */
+	u32 rport_gen;			/* SCSI rport association generation */
 	struct lpfc_nvme_rport *nrport;	/* nvme transport rport struct. */
 	struct lpfc_vport *vport;
 	struct lpfc_work_evt els_retry_evt;
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 4f68038789b5..c47a8ccdf2b5 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -157,15 +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;
+	u32 rport_gen;
 	bool drop_initial_node_ref = false;
+	bool drop_scsi_node_ref = false;
+	bool stale_rport = false;
 
-	ndlp = ((struct lpfc_rport_data *)rport->dd_data)->pnode;
+	rdata = rport->dd_data;
+	ndlp = READ_ONCE(rdata->pnode);
 	if (!ndlp)
 		return;
+	rport_gen = READ_ONCE(rdata->generation);
 
 	vport = ndlp->vport;
 	phba  = vport->phba;
@@ -187,6 +193,16 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	    !test_bit(HBA_SETUP, &phba->hba_flag))) {
 
 		spin_lock_irqsave(&ndlp->lock, iflags);
+		if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
+			spin_unlock_irqrestore(&ndlp->lock, iflags);
+			return;
+		}
+		if (ndlp->rport != rport) {
+			spin_unlock_irqrestore(&ndlp->lock, iflags);
+			lpfc_nlp_put(ndlp);
+			return;
+		}
+		rdata->pnode = NULL;
 		ndlp->rport = NULL;
 
 		/* Only 1 thread can drop the initial node reference.
@@ -202,6 +218,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 		 */
 		if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
 			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+			drop_scsi_node_ref = true;
 
 			/* If NLP_XPT_REGD was cleared in lpfc_nlp_unreg_node,
 			 * unregister calls were made to the scsi and nvme
@@ -213,9 +230,6 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 				if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
 					ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
 				spin_unlock_irqrestore(&ndlp->lock, iflags);
-
-				/* Release scsi transport reference */
-				lpfc_nlp_put(ndlp);
 			} else {
 				spin_unlock_irqrestore(&ndlp->lock, iflags);
 			}
@@ -223,14 +237,13 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 			spin_unlock_irqrestore(&ndlp->lock, iflags);
 		}
 
+		if (drop_scsi_node_ref)
+			lpfc_nlp_put(ndlp);
 		if (drop_initial_node_ref)
 			lpfc_nlp_put(ndlp);
 		return;
 	}
 
-	if (ndlp->nlp_state == NLP_STE_MAPPED_NODE)
-		return;
-
 	/* Ignore callback for a mismatched (stale) rport */
 	if (ndlp->rport != rport) {
 		lpfc_vlog_msg(vport, KERN_WARNING, LOG_NODE,
@@ -239,6 +252,31 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 			      "refcnt %u\n",
 			      ndlp->nlp_DID, ndlp, rport, ndlp->rport,
 			      ndlp->nlp_state, kref_read(&ndlp->kref));
+		/* Drop the reference held by this stale rport. */
+		lpfc_nlp_put(ndlp);
+		return;
+	}
+
+	if (ndlp->nlp_state == NLP_STE_MAPPED_NODE) {
+		spin_lock_irqsave(&ndlp->lock, iflags);
+		if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
+			spin_unlock_irqrestore(&ndlp->lock, iflags);
+			return;
+		}
+		if (ndlp->rport != rport) {
+			stale_rport = true;
+		} else if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
+			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+			rdata->pnode = NULL;
+			ndlp->rport = NULL;
+			drop_scsi_node_ref = true;
+		}
+		spin_unlock_irqrestore(&ndlp->lock, iflags);
+
+		if (stale_rport)
+			lpfc_nlp_put(ndlp);
+		if (drop_scsi_node_ref)
+			lpfc_nlp_put(ndlp);
 		return;
 	}
 
@@ -254,15 +292,44 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 		lpfc_printf_vlog(vport, KERN_ERR, LOG_TRACE_EVENT,
 				 "6790 rport name %llx dev_loss_evt pending\n",
 				 rport->port_name);
+		spin_lock_irqsave(&ndlp->lock, iflags);
+		if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
+			spin_unlock_irqrestore(&ndlp->lock, iflags);
+			return;
+		}
+		if (ndlp->rport != rport) {
+			stale_rport = true;
+		} else if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
+			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+			rdata->pnode = NULL;
+			ndlp->rport = NULL;
+			drop_scsi_node_ref = true;
+		}
+		spin_unlock_irqrestore(&ndlp->lock, iflags);
+
+		if (stale_rport || drop_scsi_node_ref)
+			lpfc_nlp_put(ndlp);
 		return;
 	}
 
-	set_bit(NLP_IN_DEV_LOSS, &ndlp->nlp_flag);
-
 	spin_lock_irqsave(&ndlp->lock, iflags);
 	/* If there is a PLOGI in progress, and we are in a
 	 * NLP_NPR_2B_DISC state, don't turn off the flag.
 	 */
+	if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
+		spin_unlock_irqrestore(&ndlp->lock, iflags);
+		return;
+	}
+	if (ndlp->rport != rport) {
+		spin_unlock_irqrestore(&ndlp->lock, iflags);
+		lpfc_nlp_put(ndlp);
+		return;
+	}
+	if (!(ndlp->fc4_xpt_flags & SCSI_XPT_REGD)) {
+		spin_unlock_irqrestore(&ndlp->lock, iflags);
+		return;
+	}
+	set_bit(NLP_IN_DEV_LOSS, &ndlp->nlp_flag);
 	if (ndlp->nlp_state != NLP_STE_PLOGI_ISSUE)
 		clear_bit(NLP_NPR_2B_DISC, &ndlp->nlp_flag);
 
@@ -271,7 +338,8 @@ 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;
+	drop_scsi_node_ref = true;
+	rdata->pnode = NULL;
 	ndlp->rport = NULL;
 	spin_unlock_irqrestore(&ndlp->lock, iflags);
 
@@ -287,6 +355,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 			list_add_tail(&evtp->evt_listp, &phba->work_list);
 			spin_unlock_irqrestore(&phba->hbalock, iflags);
 			lpfc_worker_wake_up(phba);
+			if (drop_scsi_node_ref)
+				lpfc_nlp_put(ndlp);
 			return;
 		}
 		spin_unlock_irqrestore(&phba->hbalock, iflags);
@@ -304,6 +374,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 						NLP_EVT_DEVICE_RM);
 		}
 	}
+	if (drop_scsi_node_ref)
+		lpfc_nlp_put(ndlp);
 }
 
 /**
@@ -4475,11 +4547,14 @@ static void
 lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 {
 	struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
+	struct fc_rport  *old_rport;
 	struct fc_rport  *rport;
 	struct lpfc_rport_data *rdata;
+	struct lpfc_nodelist *pnode;
 	struct fc_rport_identifiers rport_ids;
 	struct lpfc_hba  *phba = vport->phba;
 	unsigned long flags;
+	bool drop_new_scsi_ref = false;
 
 	if (vport->cfg_enable_fc4_type == LPFC_ENABLE_NVME)
 		return;
@@ -4499,10 +4574,27 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	if (test_bit(FC_UNLOADING, &vport->load_flag))
 		return;
 
-	ndlp->rport = rport = fc_remote_port_add(shost, 0, &rport_ids);
+	/*
+	 * Keep ndlp alive while the transport cancels or completes an old
+	 * rport's devloss work. This reference becomes the new association
+	 * reference after fc_remote_port_add() succeeds.
+	 */
+	pnode = lpfc_nlp_get(ndlp);
+	if (!pnode) {
+		dev_warn(&phba->pcidev->dev,
+			 "Warning - node ref failed. Unreg rport\n");
+		return;
+	}
+
+	spin_lock_irqsave(&ndlp->lock, flags);
+	old_rport = ndlp->rport;
+	spin_unlock_irqrestore(&ndlp->lock, flags);
+
+	rport = fc_remote_port_add(shost, 0, &rport_ids);
 	if (!rport) {
 		dev_printk(KERN_WARNING, &phba->pcidev->dev,
 			   "Warning: fc_remote_port_add failed\n");
+		lpfc_nlp_put(pnode);
 		return;
 	}
 
@@ -4510,19 +4602,27 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	rport->maxframe_size = ndlp->nlp_maxframe;
 	rport->supported_classes = ndlp->nlp_class_sup;
 	rdata = rport->dd_data;
-	rdata->pnode = lpfc_nlp_get(ndlp);
-	if (!rdata->pnode) {
-		dev_warn(&phba->pcidev->dev,
-			 "Warning - node ref failed. Unreg rport\n");
-		fc_remote_port_delete(rport);
-		ndlp->rport = NULL;
-		return;
-	}
 
 	spin_lock_irqsave(&ndlp->lock, flags);
+	/*
+	 * A reused rport normally retains its old association reference.
+	 * However, devloss may have completed while fc_remote_port_add()
+	 * was running, in which case it cleared SCSI_XPT_REGD and released
+	 * that reference. In the former case, transfer the old reference to
+	 * the new association and release the provisional new reference.
+	 */
+	if (rport == old_rport &&
+	    (ndlp->fc4_xpt_flags & SCSI_XPT_REGD))
+		drop_new_scsi_ref = true;
+	rdata->pnode = pnode;
+	rdata->generation = ++ndlp->rport_gen;
+	ndlp->rport = rport;
 	ndlp->fc4_xpt_flags |= SCSI_XPT_REGD;
 	spin_unlock_irqrestore(&ndlp->lock, flags);
 
+	if (drop_new_scsi_ref)
+		lpfc_nlp_put(pnode);
+
 	if (ndlp->nlp_type & NLP_FCP_TARGET)
 		rport_ids.roles |= FC_PORT_ROLE_FCP_TARGET;
 	if (ndlp->nlp_type & NLP_FCP_INITIATOR)
@@ -4570,7 +4670,6 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
 			 kref_read(&ndlp->kref));
 
 	fc_remote_port_delete(rport);
-	lpfc_nlp_put(ndlp);
 }
 
 static void
diff --git a/drivers/scsi/lpfc/lpfc_scsi.h b/drivers/scsi/lpfc/lpfc_scsi.h
index a05d203e4777..6e8bb2232cba 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.h
+++ b/drivers/scsi/lpfc/lpfc_scsi.h
@@ -42,6 +42,7 @@ struct lpfc_hba;
 /* per-port data that is allocated in the FC transport for us */
 struct lpfc_rport_data {
 	struct lpfc_nodelist *pnode;	/* Pointer to the node structure. */
+	u32 generation;		/* rport association generation */
 };
 
 struct lpfc_device_id {
-- 
2.47.3

Re: [PATCH v5 1/1] scsi: lpfc: defer SCSI rport node put until devloss callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: lpfc: memory leak of old ndlp on rport reuse
- [Medium] scsi: lpfc: data races in lockless rport association handoff
--

commit 0e3550afaa0e5fec6d02f203dc40ee6302db8e5a
Author: Dai Ngo <dai.ngo@oracle.com>

scsi: lpfc: defer SCSI rport node put until devloss callback

This commit delays dropping the SCSI rport node reference until
lpfc_dev_loss_tmo_callbk() detaches the rport from the lpfc node. It
introduces an rport association generation number to verify the node,
generation, and rport before changing association state or dropping a
reference, closing use-after-free and rport-reuse races.

> diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
> --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
> +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
> @@ -187,6 +193,16 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>  	    !test_bit(HBA_SETUP, &phba->hba_flag))) {
>  
>  		spin_lock_irqsave(&ndlp->lock, iflags);
> +		if (rdata->pnode != ndlp || rdata->generation != rport_gen) {

[Severity: Medium]
Does this plain read race with writers updating rdata->pnode and
rdata->generation?

The commit message mentions using READ_ONCE() / WRITE_ONCE() for the
lockless pnode handoff. While READ_ONCE() is used initially in
lpfc_dev_loss_tmo_callbk(), this read inside old_ndlp->lock is a plain
read. Since a concurrent writer for a reused rport holds a different lock
(new_ndlp->lock), this plain read races with the writer.

[ ... ]

> @@ -223,14 +237,13 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
[ ... ]
> -	if (ndlp->nlp_state == NLP_STE_MAPPED_NODE)
> -		return;
> -
>  	/* Ignore callback for a mismatched (stale) rport */
>  	if (ndlp->rport != rport) {

[Severity: Medium]
Could this plain read cause reader threads to observe torn stores or
out-of-order writes?

This read of ndlp->rport occurs outside the lock and races with concurrent
writers updating it. Without proper WRITE_ONCE() / READ_ONCE() pairing,
compiler optimizations can break the intended semantics.

[ ... ]

> @@ -4517,19 +4609,27 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
[ ... ]
>  	spin_lock_irqsave(&ndlp->lock, flags);
> +	/*
> +	 * A reused rport normally retains its old association reference.
> +	 * However, devloss may have completed while fc_remote_port_add()
> +	 * was running, in which case it cleared SCSI_XPT_REGD and released
> +	 * that reference. In the former case, transfer the old reference to
> +	 * the new association and release the provisional new reference.
> +	 */
> +	if (rport == old_rport &&
> +	    (ndlp->fc4_xpt_flags & SCSI_XPT_REGD))
> +		drop_new_scsi_ref = true;
> +	rdata->pnode = pnode;

[Severity: High]
Does this code leak the old lpfc_nodelist on rport reuse?

When a device logs out and logs back in (e.g., with a different NPort ID),
the transport's fc_remote_port_add() matches by WWPN, unblocks the
existing rport, and cancels its dev_loss_work timer.

In lpfc_register_remote_port(), rport == old_rport evaluates to false
because old_rport is new_ndlp->rport (usually NULL), so drop_new_scsi_ref
is false. Then rdata->pnode is overwritten with the new ndlp. The old ndlp
is permanently orphaned with SCSI_XPT_REGD set. Because its devloss timer
was cancelled and its association removed, the reference taken during its
registration is permanently leaked.

> +	rdata->generation = ++ndlp->rport_gen;
> +	ndlp->rport = rport;

[Severity: Medium]
Do these plain writes race with lockless readers?

As mentioned earlier, the commit message claims to use READ_ONCE() and
WRITE_ONCE() for the lockless pnode handoff, but plain writes are used
here for rdata->pnode, rdata->generation, and ndlp->rport.

>  	ndlp->fc4_xpt_flags |= SCSI_XPT_REGD;
>  	spin_unlock_irqrestore(&ndlp->lock, flags);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929223133.2922017-1-dai.ngo@oracle.com?part=1

Re: [PATCH v5 1/1] scsi: lpfc: defer SCSI rport node put until devloss callback

[Dai Ngo]

Hi all,

I’d like to withdraw this patch. The changes are growing larger and more 
complex than intended.

I’m working on a simpler follow-up patch that addresses only the 
use-after-free issue triggered when the FC cable is pulled, while 
leaving the pre-existing issues unchanged.

Thanks,

-Dai

On 9/29/26 3:28 PM, Dai Ngo wrote:
> lpfc_register_remote_port() stores an lpfc_nodelist pointer in the SCSI
> transport rport private data and takes a node reference for that
> association.
>
> lpfc_unregister_remote_port() calls fc_remote_port_delete(), which start
> SCSI transport devloss processing.  The rport remains usable until that
> processing completes: the transport can still invoke
> lpfc_terminate_rport_io() and lpfc_dev_loss_tmo_callbk(), both of which
> obtain the lpfc node through rport->dd_data->pnode.
>
> Do not drop the association reference immediately after
> fc_remote_port_delete().  Retain it until lpfc_dev_loss_tmo_callbk()
> detaches the rport and rport private data from the node, clears
> SCSI_XPT_REGD, and drops the association reference.  This prevents the
> node from being freed while the transport still has callbacks pending.
>
> An rport can reappear while devloss processing is in progress.  Before
> calling fc_remote_port_add(), acquire a provisional node reference.
> When the transport reuses the prior fc_rport and its old SCSI association
> remains registered, transfer that old reference to the new association
> and release the provisional one.  Otherwise retain the provisional
> reference because the old association was already released by devloss.
>
> Associate a generation with each rport private-data assignment. A
> devloss callback captures the generation and, while holding ndlp->lock,
> verifies the pnode, generation, and rport before changing association
> state or dropping a reference.  Thus, an old callback cannot operate
> on a reused rport after the transport has cleared and reinitialized
> its private data.
>
> Set NLP_IN_DEV_LOSS only after these ownership checks succeed.  A stale
> callback therefore cannot leave the flag set and suppress a later
> node-reference release.
>
> This preserves one node reference for each active SCSI rport association
> and closes the devloss callback use-after-free and rport-reuse races.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>   drivers/scsi/lpfc/lpfc_disc.h    |   1 +
>   drivers/scsi/lpfc/lpfc_hbadisc.c | 139 ++++++++++++++++++++++++++-----
>   drivers/scsi/lpfc/lpfc_scsi.h    |   1 +
>   3 files changed, 121 insertions(+), 20 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.
>
> V3:
> . Redo the patch based on Paul's review.
> Delay dropping the SCSI rport node reference until
> lpfc_dev_loss_tmo_callbk() detaches the rport from the lpfc node.
>
> V4:
> . Fix the node lead reference when the remote port reconnects before
> dev_loss_tmo expires. If fc_remote_port_add() reuses the existing SCSI
> rport, transfer the reference that is still held on the rport instead
> of acquiring a new reference.
>
> V5:
> . Fix devloss callback use-after-free and rport-reuse races identified
> in saskiko review by introducing rport association generation number.
> lpfc_dev_loss_tmo_callbk checks the generation number verifies the pnode,
> generation, and rport before changing association state or dropping a
> reference.
>
> diff --git a/drivers/scsi/lpfc/lpfc_disc.h b/drivers/scsi/lpfc/lpfc_disc.h
> index 51cb8571c049..4236e2397357 100644
> --- a/drivers/scsi/lpfc/lpfc_disc.h
> +++ b/drivers/scsi/lpfc/lpfc_disc.h
> @@ -141,6 +141,7 @@ struct lpfc_nodelist {
>   	struct timer_list   nlp_delayfunc;	/* Used for delayed ELS cmds */
>   	struct lpfc_hba *phba;
>   	struct fc_rport *rport;		/* scsi_transport_fc port structure */
> +	u32 rport_gen;			/* SCSI rport association generation */
>   	struct lpfc_nvme_rport *nrport;	/* nvme transport rport struct. */
>   	struct lpfc_vport *vport;
>   	struct lpfc_work_evt els_retry_evt;
> diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
> index 4f68038789b5..c47a8ccdf2b5 100644
> --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
> +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
> @@ -157,15 +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;
> +	u32 rport_gen;
>   	bool drop_initial_node_ref = false;
> +	bool drop_scsi_node_ref = false;
> +	bool stale_rport = false;
>   
> -	ndlp = ((struct lpfc_rport_data *)rport->dd_data)->pnode;
> +	rdata = rport->dd_data;
> +	ndlp = READ_ONCE(rdata->pnode);
>   	if (!ndlp)
>   		return;
> +	rport_gen = READ_ONCE(rdata->generation);
>   
>   	vport = ndlp->vport;
>   	phba  = vport->phba;
> @@ -187,6 +193,16 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   	    !test_bit(HBA_SETUP, &phba->hba_flag))) {
>   
>   		spin_lock_irqsave(&ndlp->lock, iflags);
> +		if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
> +			spin_unlock_irqrestore(&ndlp->lock, iflags);
> +			return;
> +		}
> +		if (ndlp->rport != rport) {
> +			spin_unlock_irqrestore(&ndlp->lock, iflags);
> +			lpfc_nlp_put(ndlp);
> +			return;
> +		}
> +		rdata->pnode = NULL;
>   		ndlp->rport = NULL;
>   
>   		/* Only 1 thread can drop the initial node reference.
> @@ -202,6 +218,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   		 */
>   		if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
>   			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
> +			drop_scsi_node_ref = true;
>   
>   			/* If NLP_XPT_REGD was cleared in lpfc_nlp_unreg_node,
>   			 * unregister calls were made to the scsi and nvme
> @@ -213,9 +230,6 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   				if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
>   					ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
>   				spin_unlock_irqrestore(&ndlp->lock, iflags);
> -
> -				/* Release scsi transport reference */
> -				lpfc_nlp_put(ndlp);
>   			} else {
>   				spin_unlock_irqrestore(&ndlp->lock, iflags);
>   			}
> @@ -223,14 +237,13 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   			spin_unlock_irqrestore(&ndlp->lock, iflags);
>   		}
>   
> +		if (drop_scsi_node_ref)
> +			lpfc_nlp_put(ndlp);
>   		if (drop_initial_node_ref)
>   			lpfc_nlp_put(ndlp);
>   		return;
>   	}
>   
> -	if (ndlp->nlp_state == NLP_STE_MAPPED_NODE)
> -		return;
> -
>   	/* Ignore callback for a mismatched (stale) rport */
>   	if (ndlp->rport != rport) {
>   		lpfc_vlog_msg(vport, KERN_WARNING, LOG_NODE,
> @@ -239,6 +252,31 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   			      "refcnt %u\n",
>   			      ndlp->nlp_DID, ndlp, rport, ndlp->rport,
>   			      ndlp->nlp_state, kref_read(&ndlp->kref));
> +		/* Drop the reference held by this stale rport. */
> +		lpfc_nlp_put(ndlp);
> +		return;
> +	}
> +
> +	if (ndlp->nlp_state == NLP_STE_MAPPED_NODE) {
> +		spin_lock_irqsave(&ndlp->lock, iflags);
> +		if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
> +			spin_unlock_irqrestore(&ndlp->lock, iflags);
> +			return;
> +		}
> +		if (ndlp->rport != rport) {
> +			stale_rport = true;
> +		} else if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
> +			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
> +			rdata->pnode = NULL;
> +			ndlp->rport = NULL;
> +			drop_scsi_node_ref = true;
> +		}
> +		spin_unlock_irqrestore(&ndlp->lock, iflags);
> +
> +		if (stale_rport)
> +			lpfc_nlp_put(ndlp);
> +		if (drop_scsi_node_ref)
> +			lpfc_nlp_put(ndlp);
>   		return;
>   	}
>   
> @@ -254,15 +292,44 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   		lpfc_printf_vlog(vport, KERN_ERR, LOG_TRACE_EVENT,
>   				 "6790 rport name %llx dev_loss_evt pending\n",
>   				 rport->port_name);
> +		spin_lock_irqsave(&ndlp->lock, iflags);
> +		if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
> +			spin_unlock_irqrestore(&ndlp->lock, iflags);
> +			return;
> +		}
> +		if (ndlp->rport != rport) {
> +			stale_rport = true;
> +		} else if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
> +			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
> +			rdata->pnode = NULL;
> +			ndlp->rport = NULL;
> +			drop_scsi_node_ref = true;
> +		}
> +		spin_unlock_irqrestore(&ndlp->lock, iflags);
> +
> +		if (stale_rport || drop_scsi_node_ref)
> +			lpfc_nlp_put(ndlp);
>   		return;
>   	}
>   
> -	set_bit(NLP_IN_DEV_LOSS, &ndlp->nlp_flag);
> -
>   	spin_lock_irqsave(&ndlp->lock, iflags);
>   	/* If there is a PLOGI in progress, and we are in a
>   	 * NLP_NPR_2B_DISC state, don't turn off the flag.
>   	 */
> +	if (rdata->pnode != ndlp || rdata->generation != rport_gen) {
> +		spin_unlock_irqrestore(&ndlp->lock, iflags);
> +		return;
> +	}
> +	if (ndlp->rport != rport) {
> +		spin_unlock_irqrestore(&ndlp->lock, iflags);
> +		lpfc_nlp_put(ndlp);
> +		return;
> +	}
> +	if (!(ndlp->fc4_xpt_flags & SCSI_XPT_REGD)) {
> +		spin_unlock_irqrestore(&ndlp->lock, iflags);
> +		return;
> +	}
> +	set_bit(NLP_IN_DEV_LOSS, &ndlp->nlp_flag);
>   	if (ndlp->nlp_state != NLP_STE_PLOGI_ISSUE)
>   		clear_bit(NLP_NPR_2B_DISC, &ndlp->nlp_flag);
>   
> @@ -271,7 +338,8 @@ 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;
> +	drop_scsi_node_ref = true;
> +	rdata->pnode = NULL;
>   	ndlp->rport = NULL;
>   	spin_unlock_irqrestore(&ndlp->lock, iflags);
>   
> @@ -287,6 +355,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   			list_add_tail(&evtp->evt_listp, &phba->work_list);
>   			spin_unlock_irqrestore(&phba->hbalock, iflags);
>   			lpfc_worker_wake_up(phba);
> +			if (drop_scsi_node_ref)
> +				lpfc_nlp_put(ndlp);
>   			return;
>   		}
>   		spin_unlock_irqrestore(&phba->hbalock, iflags);
> @@ -304,6 +374,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
>   						NLP_EVT_DEVICE_RM);
>   		}
>   	}
> +	if (drop_scsi_node_ref)
> +		lpfc_nlp_put(ndlp);
>   }
>   
>   /**
> @@ -4475,11 +4547,14 @@ static void
>   lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>   {
>   	struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
> +	struct fc_rport  *old_rport;
>   	struct fc_rport  *rport;
>   	struct lpfc_rport_data *rdata;
> +	struct lpfc_nodelist *pnode;
>   	struct fc_rport_identifiers rport_ids;
>   	struct lpfc_hba  *phba = vport->phba;
>   	unsigned long flags;
> +	bool drop_new_scsi_ref = false;
>   
>   	if (vport->cfg_enable_fc4_type == LPFC_ENABLE_NVME)
>   		return;
> @@ -4499,10 +4574,27 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>   	if (test_bit(FC_UNLOADING, &vport->load_flag))
>   		return;
>   
> -	ndlp->rport = rport = fc_remote_port_add(shost, 0, &rport_ids);
> +	/*
> +	 * Keep ndlp alive while the transport cancels or completes an old
> +	 * rport's devloss work. This reference becomes the new association
> +	 * reference after fc_remote_port_add() succeeds.
> +	 */
> +	pnode = lpfc_nlp_get(ndlp);
> +	if (!pnode) {
> +		dev_warn(&phba->pcidev->dev,
> +			 "Warning - node ref failed. Unreg rport\n");
> +		return;
> +	}
> +
> +	spin_lock_irqsave(&ndlp->lock, flags);
> +	old_rport = ndlp->rport;
> +	spin_unlock_irqrestore(&ndlp->lock, flags);
> +
> +	rport = fc_remote_port_add(shost, 0, &rport_ids);
>   	if (!rport) {
>   		dev_printk(KERN_WARNING, &phba->pcidev->dev,
>   			   "Warning: fc_remote_port_add failed\n");
> +		lpfc_nlp_put(pnode);
>   		return;
>   	}
>   
> @@ -4510,19 +4602,27 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>   	rport->maxframe_size = ndlp->nlp_maxframe;
>   	rport->supported_classes = ndlp->nlp_class_sup;
>   	rdata = rport->dd_data;
> -	rdata->pnode = lpfc_nlp_get(ndlp);
> -	if (!rdata->pnode) {
> -		dev_warn(&phba->pcidev->dev,
> -			 "Warning - node ref failed. Unreg rport\n");
> -		fc_remote_port_delete(rport);
> -		ndlp->rport = NULL;
> -		return;
> -	}
>   
>   	spin_lock_irqsave(&ndlp->lock, flags);
> +	/*
> +	 * A reused rport normally retains its old association reference.
> +	 * However, devloss may have completed while fc_remote_port_add()
> +	 * was running, in which case it cleared SCSI_XPT_REGD and released
> +	 * that reference. In the former case, transfer the old reference to
> +	 * the new association and release the provisional new reference.
> +	 */
> +	if (rport == old_rport &&
> +	    (ndlp->fc4_xpt_flags & SCSI_XPT_REGD))
> +		drop_new_scsi_ref = true;
> +	rdata->pnode = pnode;
> +	rdata->generation = ++ndlp->rport_gen;
> +	ndlp->rport = rport;
>   	ndlp->fc4_xpt_flags |= SCSI_XPT_REGD;
>   	spin_unlock_irqrestore(&ndlp->lock, flags);
>   
> +	if (drop_new_scsi_ref)
> +		lpfc_nlp_put(pnode);
> +
>   	if (ndlp->nlp_type & NLP_FCP_TARGET)
>   		rport_ids.roles |= FC_PORT_ROLE_FCP_TARGET;
>   	if (ndlp->nlp_type & NLP_FCP_INITIATOR)
> @@ -4570,7 +4670,6 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
>   			 kref_read(&ndlp->kref));
>   
>   	fc_remote_port_delete(rport);
> -	lpfc_nlp_put(ndlp);
>   }
>   
>   static void
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.h b/drivers/scsi/lpfc/lpfc_scsi.h
> index a05d203e4777..6e8bb2232cba 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.h
> +++ b/drivers/scsi/lpfc/lpfc_scsi.h
> @@ -42,6 +42,7 @@ struct lpfc_hba;
>   /* per-port data that is allocated in the FC transport for us */
>   struct lpfc_rport_data {
>   	struct lpfc_nodelist *pnode;	/* Pointer to the node structure. */
> +	u32 generation;		/* rport association generation */
>   };
>   
>   struct lpfc_device_id {