scsi: lpfc: Avoid potential ndlp use-after-free in dev_loss_tmo_callbk

Smatch detected a potential use-after-free of an ndlp oject in
dev_loss_tmo_callbk during driver unload or fatal error handling.

Fix by reordering code to avoid potential use-after-free if initial
nodelist reference has been previously removed.

Fixes: 4281f44ea8bf ("scsi: lpfc: Prevent NDLP reference count underflow in dev_loss_tmo callback")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-scsi/41c1d855-9eb5-416f-ac12-8b61929201a3@stanley.mountain/
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20250425194806.3585-6-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 9e585af05bec92c9f341ed869826f228ef3dc37c..3d15a964f5c9824e6c5676ba6a82556fd234b9a2 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -161,7 +161,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
        struct lpfc_hba   *phba;
        struct lpfc_work_evt *evtp;
        unsigned long iflags;
-       bool nvme_reg = false;
+       bool drop_initial_node_ref = false;
 
        ndlp = ((struct lpfc_rport_data *)rport->dd_data)->pnode;
        if (!ndlp)
@@ -188,8 +188,13 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
                spin_lock_irqsave(&ndlp->lock, iflags);
                ndlp->rport = NULL;
 
-               if (ndlp->fc4_xpt_flags & NVME_XPT_REGD)
-                       nvme_reg = true;
+               /* Only 1 thread can drop the initial node reference.
+                * If not registered for NVME and NLP_DROPPED flag is
+                * clear, remove the initial reference.
+                */
+               if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
+                       if (!test_and_set_bit(NLP_DROPPED, &ndlp->nlp_flag))
+                               drop_initial_node_ref = true;
 
                /* The scsi_transport is done with the rport so lpfc cannot
                 * call to unregister.
@@ -200,13 +205,16 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
                        /* If NLP_XPT_REGD was cleared in lpfc_nlp_unreg_node,
                         * unregister calls were made to the scsi and nvme
                         * transports and refcnt was already decremented. Clear
-                        * the NLP_XPT_REGD flag only if the NVME Rport is
+                        * the NLP_XPT_REGD flag only if the NVME nrport is
                         * confirmed unregistered.
                         */
-                       if (!nvme_reg && ndlp->fc4_xpt_flags & NLP_XPT_REGD) {
-                               ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
+                       if (ndlp->fc4_xpt_flags & NLP_XPT_REGD) {
+                               if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
+                                       ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
                                spin_unlock_irqrestore(&ndlp->lock, iflags);
-                               lpfc_nlp_put(ndlp); /* may free ndlp */
+
+                               /* Release scsi transport reference */
+                               lpfc_nlp_put(ndlp);
                        } else {
                                spin_unlock_irqrestore(&ndlp->lock, iflags);
                        }
@@ -214,14 +222,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
                        spin_unlock_irqrestore(&ndlp->lock, iflags);
                }
 
-               /* Only 1 thread can drop the initial node reference.  If
-                * another thread has set NLP_DROPPED, this thread is done.
-                */
-               if (nvme_reg || test_bit(NLP_DROPPED, &ndlp->nlp_flag))
-                       return;
-
-               set_bit(NLP_DROPPED, &ndlp->nlp_flag);
-               lpfc_nlp_put(ndlp);
+               if (drop_initial_node_ref)
+                       lpfc_nlp_put(ndlp);
                return;
        }