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

[sashiko-bot@kernel.org]

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