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@xxxxxxxxxxxx>
---
drivers/scsi/lpfc/lpfc_hbadisc.c | 3 ++-
drivers/scsi/lpfc/lpfc_nvme.c | 24 +++++++++++++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 51afb60859eb..b7f922f31e26 100644
--- 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 39acbcb7ec66..96e11a26c297 100644
--- 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;
--
2.38.0
