[PATCH v2] scsi: lpfc: Add rport validation in lpfc_dev_loss_tmo_callbk

[PATCH v2] scsi: lpfc: Add rport validation in lpfc_dev_loss_tmo_callbk

[Vaibhav Nagare]

  Fix a kernel NULL pointer dereference in lpfc_dev_loss_tmo_callbk()
  when ndlp->vport is NULL during FC remote port deletion.

  The crash occurs during fc_rport_final_delete() when the vport has
  already been cleared on the ndlp structure, but the dev_loss_tmo
  callback is still invoked.

  The driver already has lpfc_rport_invalid() which validates rport,
  rdata, ndlp, and vport. The function lpfc_terminate_rport_io() uses
  this validation, but lpfc_dev_loss_tmo_callbk() does not, leading
  to a NULL pointer dereference when accessing vport->phba.

  Crash signature observed on RHEL 8.10 (4.18.0-553.125.1.el8_10.x86_64):
    BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
    RIP: lpfc_dev_loss_tmo_callbk+0x54
    Call Trace:
     fc_rport_final_delete+0xea/0x1d0 [scsi_transport_fc]
     process_one_work+0x1d3/0x390
     worker_thread+0x30/0x390

    Preceding kernel log message:
     lpfc_rport_invalid: Null vport on ndlp xffff9c36d412bc00

  Add a call to lpfc_rport_invalid() at the start of
  lpfc_dev_loss_tmo_callbk() to validate all required pointers before
  dereferencing them, consistent with lpfc_terminate_rport_io().

Signed-off-by: Vaibhav Nagare <vnagare@redhat.com>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index b8649c40b537..af56c32708c6 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -163,27 +163,15 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	unsigned long iflags;
 	bool drop_initial_node_ref = false;
 
+	/* Validate rport, rdata, ndlp, and vport before proceeding */
+	if (lpfc_rport_invalid(rport))
+		return;
+
 	ndlp = ((struct lpfc_rport_data *)rport->dd_data)->pnode;
 	if (!ndlp)
 		return;
 
 	vport = ndlp->vport;
-	if (!vport) {
-		/*
-		 * Vport is NULL - this can happen during teardown when the
-		 * vport has been destroyed but the rport final delete is
-		 * still processing. Clear the association and return.
-		 */
-		pr_err("lpfc: Null vport on ndlp %p, DID x%06x rport %p\n",
-		       ndlp, ndlp->nlp_DID, rport);
-
-		spin_lock_irqsave(&ndlp->lock, iflags);
-		((struct lpfc_rport_data *)rport->dd_data)->pnode = NULL;
-		ndlp->rport = NULL;
-		spin_unlock_irqrestore(&ndlp->lock, iflags);
-		return;
-	}
-
 	phba  = vport->phba;
 
 	lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_RPORT,
-- 
2.54.0

Re: [PATCH v2] scsi: lpfc: Add rport validation in lpfc_dev_loss_tmo_callbk

[Justin Tee]

Hi Vaibhav,

1.) This patch looks like it was rebased on top of the first version of 
this patch instead of off the tree.

2.) lpfc_dev_loss_tmo_callbk is the .dev_loss_tmo_callbk routine for 
scsi transport layer to call into our driver when dev_loss_tmo event is 
detected.  A null ndlp->vport in lpfc_dev_loss_tmo_callbk is somewhat 
unexpected.  Rather than checking for a stale pointer, it is more likely 
that an ndlp kref accounting race caused the vport ptr to be freed 
before lpfc_dev_loss_tmo_callbk was called.

Is it possible to share the vmcore’s dmesg.txt file so that we may 
examine the root cause of the stale ndlp->vport ptr?

Regards,
Justin

Re: [PATCH v2] scsi: lpfc: Add rport validation in lpfc_dev_loss_tmo_callbk

[Ewan Milne]

On Mon, Jun 29, 2026 at 3:01 PM Vaibhav Nagare <nagarevaibhav@gmail.com> wrote:
>
>   Fix a kernel NULL pointer dereference in lpfc_dev_loss_tmo_callbk()
>   when ndlp->vport is NULL during FC remote port deletion.
>
>   The crash occurs during fc_rport_final_delete() when the vport has
>   already been cleared on the ndlp structure, but the dev_loss_tmo
>   callback is still invoked.
>
>   The driver already has lpfc_rport_invalid() which validates rport,
>   rdata, ndlp, and vport. The function lpfc_terminate_rport_io() uses
>   this validation, but lpfc_dev_loss_tmo_callbk() does not, leading
>   to a NULL pointer dereference when accessing vport->phba.
>

Yeah but this doesn't really fix the problem.  It may avoid a crash in
this particular place in lpfc_dev_loss_tmo_callbk() but the cause of the
null ndlp->vport pointer is that the lpfc_nodelist object has already
been freed due to an incorrect refcount.  If we return prematurely
the driver is not cleaning up the rport state properly.

Unfortunately it can be difficult to pinpoint the cause of the prematurely
decremented refcount and we have been chasing these problems for
years.

Prior to c6adba150191 ("scsi: lpfc: Rework remote port lock handling")
lpfc_nlp_release() did not set ndlp->vport = NULL, among other things
and so the driver typically did not crash but operated with a use-after-free
and probably did not behave correctly,

-Ewan