scsi: lpfc: Prevent use-after-free during rmmod with mapped NVMe rports

During rmmod, when dev_loss_tmo callback is called, an ndlp kref count is
decremented twice.  Once for SCSI transport registration and second to
remove the initial node allocation kref.  If there is also an NVMe
transport registration, another reference count decrement is expected in
lpfc_nvme_unregister_port().

Race conditions between the NVMe transport remoteport_delete and
dev_loss_tmo callbacks sometimes results in premature ndlp object release
resulting in use-after-free issues.

Fix by not dropping the ndlp object in dev_loss_tmo callback with an
outstanding NVMe transport registration.  Inversely, mark the final
NLP_DROPPED flag in lpfc_nvme_unregister_port when rmmod flag is set.

Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20230908211923.37603-1-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 674dd07aae72a310b075316e5989331e62d5879b..5154eeaee0ec325c8f293ee52e291ea5d9b7aa2f 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -199,7 +199,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
                /* Only 1 thread can drop the initial node reference.  If
                 * another thread has set NLP_DROPPED, this thread is done.
                 */
-               if (!(ndlp->nlp_flag & NLP_DROPPED)) {
+               if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD) &&
+                   !(ndlp->nlp_flag & NLP_DROPPED)) {
                        ndlp->nlp_flag |= NLP_DROPPED;
                        spin_unlock_irqrestore(&ndlp->lock, iflags);
                        lpfc_nlp_put(ndlp);

diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 39acbcb7ec66a12e246ed3f92699fa5afe430c7e..96e11a26c297eb3949fbdd8d51932c2e703b09fe 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -228,8 +228,7 @@ lpfc_nvme_remoteport_delete(struct nvme_fc_remote_port *remoteport)
        spin_unlock_irq(&ndlp->lock);
 
        /* On a devloss timeout event, one more put is executed provided the
-        * NVME and SCSI rport unregister requests are complete.  If the vport
-        * is unloading, this extra put is executed by lpfc_drop_node.
+        * NVME and SCSI rport unregister requests are complete.
         */
        if (!(ndlp->fc4_xpt_flags & fc4_xpt_flags))
                lpfc_disc_state_machine(vport, ndlp, NULL, NLP_EVT_DEVICE_RM);
@@ -2567,11 +2566,7 @@ lpfc_nvme_rescan_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
  * nvme_transport perspective.  Loss of an rport just means IO cannot
  * be sent and recovery is completely up to the initator.
  * For now, the driver just unbinds the DID and port_role so that
- * no further IO can be issued.  Changes are planned for later.
- *
- * Notes - the ndlp reference count is not decremented here since
- * since there is no nvme_transport api for devloss.  Node ref count
- * is only adjusted in driver unload.
+ * no further IO can be issued.
  */
 void
 lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
@@ -2646,6 +2641,21 @@ lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
                                         "6167 NVME unregister failed %d "
                                         "port_state x%x\n",
                                         ret, remoteport->port_state);
+
+                       if (vport->load_flag & FC_UNLOADING) {
+                               /* Only 1 thread can drop the initial node
+                                * reference. Check if another thread has set
+                                * NLP_DROPPED.
+                                */
+                               spin_lock_irq(&ndlp->lock);
+                               if (!(ndlp->nlp_flag & NLP_DROPPED)) {
+                                       ndlp->nlp_flag |= NLP_DROPPED;
+                                       spin_unlock_irq(&ndlp->lock);
+                                       lpfc_nlp_put(ndlp);
+                                       return;
+                               }
+                               spin_unlock_irq(&ndlp->lock);
+                       }
                }
        }
        return;