[PATCH] scsi: lpfc: Fix ndlp reference leak in lpfc_nlp_reg_node()

[PATCH] scsi: lpfc: Fix ndlp reference leak in lpfc_nlp_reg_node()

[Wentao Liang]

In the NVMET case the driver takes a node reference in
lpfc_nlp_reg_node() because the target does not register rports, but
lpfc_nlp_unreg_node() only releases that reference when the
NVME_XPT_REGD flag is set, and the NVMET path never sets it, so the
reference is leaked until the node is torn down.

Set NVME_XPT_REGD when the reference is taken so the existing put in
lpfc_nlp_unreg_node() balances it.

Fixes: 0614568361b0 ("scsi: lpfc: Delay unregistering from transport until GIDFT or ADISC completes")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index f3a85f6c796e..9e920eaff9ef 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -4664,9 +4664,15 @@ lpfc_nlp_reg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 			}
 		} else {
 			/* Just take an NDLP ref count since the
-			 * target does not register rports.
+			 * target does not register rports. Mark it as
+			 * an NVME transport ref so that
+			 * lpfc_nlp_unreg_node() releases it.
 			 */
 			lpfc_nlp_get(ndlp);
+
+			spin_lock_irqsave(&ndlp->lock, iflags);
+			ndlp->fc4_xpt_flags |= NVME_XPT_REGD;
+			spin_unlock_irqrestore(&ndlp->lock, iflags);
 		}
 	}
 }
-- 
2.34.1

Re: [PATCH] scsi: lpfc: Fix ndlp reference leak in lpfc_nlp_reg_node()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: lpfc: missing NVME_XPT_REGD clear in NVMET unregister causes leak
- [Medium] scsi: lpfc: TOCTOU race in lpfc_nlp_reg_node() leaks reference
--

commit 8a787af86823c5598e09493f24aaa14226c3f5da
Author: Wentao Liang <vulab@iscas.ac.cn>

scsi: lpfc: Fix ndlp reference leak in lpfc_nlp_reg_node()

This commit attempts to fix a node reference leak for NVMET nodes by
setting the NVME_XPT_REGD flag when the reference is taken in
lpfc_nlp_reg_node(). This aims to ensure that lpfc_nlp_unreg_node()
balances the reference count during teardown.

Cc: stable@vger.kernel.org

> diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
> index 4c673dffa671b..f61026e3b48ea 100644
> --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
> +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
> @@ -4664,9 +4664,15 @@ lpfc_nlp_reg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>  			}
>  		} else {
>  			/* Just take an NDLP ref count since the
> -			 * target does not register rports.
> +			 * target does not register rports. Mark it as
> +			 * an NVME transport ref so that
> +			 * lpfc_nlp_unreg_node() releases it.
>  			 */
>  			lpfc_nlp_get(ndlp);
> +
> +			spin_lock_irqsave(&ndlp->lock, iflags);
> +			ndlp->fc4_xpt_flags |= NVME_XPT_REGD;
> +			spin_unlock_irqrestore(&ndlp->lock, iflags);

[Severity: Medium]
Does acquiring the lock here to set NVME_XPT_REGD introduce a time-of-check
to time-of-use race that can leak the reference?

Earlier in lpfc_nlp_reg_node(), NLP_XPT_REGD is set and ndlp->lock is
dropped to call lpfc_register_remote_port().

If a concurrent lpfc_nlp_unreg_node() call sees NLP_XPT_REGD set, it clears
it, and then locklessly evaluates NVME_XPT_REGD as 0, skipping the
lpfc_nlp_put().

When lpfc_nlp_reg_node() resumes, it increments the refcount and sets
NVME_XPT_REGD. Because NLP_XPT_REGD was already cleared, subsequent
unregisters will return early, permanently leaking this reference.

[Severity: High]
Does setting NVME_XPT_REGD for NVMET nodes here strand the node during
unregistration?

While lpfc_nlp_unreg_node() drops the reference for NVMET nodes, it fails
to clear the NVME_XPT_REGD flag:

lpfc_nlp_unreg_node() {
    ...
    /* NVMET has no upcall. */
    lpfc_nlp_put(ndlp);
    // missing: ndlp->fc4_xpt_flags &= ~NVME_XPT_REGD;
    ...
}

Leaving this flag set permanently prevents the driver from evaluating this
to true in lpfc_dev_loss_tmo_handler():

    if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
        lpfc_disc_state_machine(vport, ndlp, NULL, NLP_EVT_DEVICE_RM);

This strands the node, blocking device removal and potentially deadlocking
vport teardown.

Additionally, if the node is rediscovered as FCP-only, the sticky flag
triggers an unmatched lpfc_nlp_put(), causing an underflow and
use-after-free.

>  		}
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917144543.2157614-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] scsi: lpfc: Fix ndlp reference leak in lpfc_nlp_reg_node()

[Paul Ely]

[-- Attachment #1: Type: text/plain, Size: 2261 bytes --]

Hello Wentao,

Broadcom has already fixed this issue via an AI security scan.  We
will be pushing the patch very soon.

BTW:  using NVME_XPT_REGD is wrong for lpfc when in NVMET mode.  As
you correctly noted, When in NVMET mode, lpfc does not register NVME
rports with the nvme transport.  That flag is reserved for initiator
mode that does register nvme rports with the nvme_fc transport.

Thanks

On Thu, Sep 17, 2026 at 10:46 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
>
> In the NVMET case the driver takes a node reference in
> lpfc_nlp_reg_node() because the target does not register rports, but
> lpfc_nlp_unreg_node() only releases that reference when the
> NVME_XPT_REGD flag is set, and the NVMET path never sets it, so the
> reference is leaked until the node is torn down.
>
> Set NVME_XPT_REGD when the reference is taken so the existing put in
> lpfc_nlp_unreg_node() balances it.
>
> Fixes: 0614568361b0 ("scsi: lpfc: Delay unregistering from transport until GIDFT or ADISC completes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/scsi/lpfc/lpfc_hbadisc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
> index f3a85f6c796e..9e920eaff9ef 100644
> --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
> +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
> @@ -4664,9 +4664,15 @@ lpfc_nlp_reg_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
>                         }
>                 } else {
>                         /* Just take an NDLP ref count since the
> -                        * target does not register rports.
> +                        * target does not register rports. Mark it as
> +                        * an NVME transport ref so that
> +                        * lpfc_nlp_unreg_node() releases it.
>                          */
>                         lpfc_nlp_get(ndlp);
> +
> +                       spin_lock_irqsave(&ndlp->lock, iflags);
> +                       ndlp->fc4_xpt_flags |= NVME_XPT_REGD;
> +                       spin_unlock_irqrestore(&ndlp->lock, iflags);
>                 }
>         }
>  }
> --
> 2.34.1
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5453 bytes --]