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

[PATCH v3 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.

On remote-port unregister, lpfc_unregister_remote_port() calls
fc_remote_port_delete(), which starts SCSI transport devloss handling.
The rport is not fully finished at that point.  The SCSI transport may
still call back into the driver through lpfc_terminate_rport_io() and
lpfc_dev_loss_tmo_callbk(), both of which use rport->dd_data->pnode to
find the lpfc node.

Dropping the node reference immediately after fc_remote_port_delete()
can therefore free the lpfc_nodelist while the rport still points to
it. A cable pull can hit this window and report a KASAN use-after-free
from lpfc_terminate_rport_io(), followed by stale-node access in
lpfc_dev_loss_tmo_callbk().

Keep the SCSI rport node reference until lpfc_dev_loss_tmo_callbk()
detaches the rport from the lpfc node.  Drop that reference after
clearing SCSI_XPT_REGD and removing the rport/dd_data association,
so the node remains valid for the SCSI transport devloss callbacks.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 13 +++++++++----
 1 file changed, 9 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.

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.

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 4f68038789b5..04f56c2f69dc 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -162,6 +162,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	struct lpfc_work_evt *evtp;
 	unsigned long iflags;
 	bool drop_initial_node_ref = false;
+	bool drop_scsi_node_ref = false;
 
 	ndlp = ((struct lpfc_rport_data *)rport->dd_data)->pnode;
 	if (!ndlp)
@@ -202,6 +203,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 +215,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,6 +222,8 @@ 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;
@@ -271,6 +272,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;
+	drop_scsi_node_ref = true;
 	((struct lpfc_rport_data *)rport->dd_data)->pnode = NULL;
 	ndlp->rport = NULL;
 	spin_unlock_irqrestore(&ndlp->lock, iflags);
@@ -287,6 +289,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 +308,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 						NLP_EVT_DEVICE_RM);
 		}
 	}
+	if (drop_scsi_node_ref)
+		lpfc_nlp_put(ndlp);
 }
 
 /**
@@ -4570,7 +4576,6 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
 			 kref_read(&ndlp->kref));
 
 	fc_remote_port_delete(rport);
-	lpfc_nlp_put(ndlp);
 }
 
 static void
-- 
2.47.3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] lpfc: memory leak of lpfc_nodelist on rport reconnect or early return
--

commit 55116da6dbf444daa1282aba1d53431f3bfd853f
Author: Dai Ngo <dai.ngo@oracle.com>
Date:   Mon Sep 28 16:31:48 2026 +0000

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

This commit defers dropping the SCSI rport node reference in the lpfc driver
until the devloss callback detaches it. It removes the immediate reference drop
after fc_remote_port_delete() to prevent use-after-free conditions.

> diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
> index 4c673dffa671b..7a6f80ca92746 100644
> --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
> +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c

[ ... ]

> @@ -271,6 +272,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;
> +	drop_scsi_node_ref = true;
>  	((struct lpfc_rport_data *)rport->dd_data)->pnode = NULL;
>  	ndlp->rport = NULL;
>  	spin_unlock_irqrestore(&ndlp->lock, iflags);

[Severity: High]
This is a pre-existing issue, but by not reaching this cleanup block on early
returns, could we leave a dangling pointer?

If lpfc_dev_loss_tmo_callbk() exits early, such as when ndlp->nlp_state is
NLP_STE_MAPPED_NODE or a dev_loss_evt is already pending, it bypasses clearing
the SCSI_XPT_REGD flag and ndlp->rport.

When the SCSI transport later deletes and frees the fc_rport, wouldn't
ndlp->rport be left dangling? Then, if lpfc_nlp_unreg_node() is called:

lpfc_nlp_unreg_node() {
    ...
    if (ndlp->rport &&
        ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
        vport->phba->nport_event_cnt++;
        lpfc_unregister_remote_port(ndlp);
    ...
}

Will it pass the freed rport to fc_remote_port_delete() and cause a
use-after-free?

[ ... ]

> @@ -4577,7 +4583,6 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
>  			 kref_read(&ndlp->kref));
>  
>  	fc_remote_port_delete(rport);
> -	lpfc_nlp_put(ndlp);
>  }
>  
>  static void

[Severity: High]
By deferring this cleanup to lpfc_dev_loss_tmo_callbk(), could we leak the
node reference?

If the remote port reconnects before the dev_loss_tmo expires, the SCSI
transport calls fc_remote_port_add(), which cancels the pending
dev_loss_work timer and bypasses the devloss callback. It then clears
rport->dd_data.

When lpfc_register_remote_port() executes, it will overwrite rdata->pnode
with a new lpfc_nlp_get(ndlp) reference, permanently leaking the original
node reference.

Also, what happens if lpfc_dev_loss_tmo_callbk() does run but takes an early
return?

lpfc_dev_loss_tmo_callbk() {
    ...
    if (ndlp->nlp_state == NLP_STE_MAPPED_NODE)
        return;
    ...
}

Does this skip the newly deferred lpfc_nlp_put() cleanup, resulting in the
same leak of the node reference?

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