[PATCH v4 1/1] scsi: lpfc: defer SCSI rport node put until devloss callback

[Dai Ngo <dai.ngo@oracle.com>]

lpfc_register_remote_port() stores an lpfc_nodelist pointer in the
SCSI transport rport private data and takes a node reference for that
association.

On remote-port unregister, lpfc_unregister_remote_port() calls
fc_remote_port_delete(), which starts SCSI transport devloss handling.
The rport is not fully finished at that point.  The SCSI transport may
still call back into the driver through lpfc_terminate_rport_io() and
lpfc_dev_loss_tmo_callbk(), both of which use rport->dd_data->pnode to
find the lpfc node.

Dropping the node reference immediately after fc_remote_port_delete()
can therefore free the lpfc_nodelist while the rport still points to
it. A cable pull can hit this window and report a KASAN use-after-free
from lpfc_terminate_rport_io(), followed by stale-node access in
lpfc_dev_loss_tmo_callbk().

Keep the SCSI rport node reference until lpfc_dev_loss_tmo_callbk()
detaches the rport from the lpfc node.  Drop that reference after
clearing SCSI_XPT_REGD and removing the rport/dd_data association,
so the node remains valid for the SCSI transport devloss callbacks.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

V2:
. in lpfc_unregister_remote_port(), snapshot ndlp->rport under
ndlp->lock, clear rdata->pnode, clear ndlp->rport, and clear the
SCSI transport registration flag while holding the ndlp->lock.

. in lpfc_dev_loss_tmo_callbk, check rport's private data before
using it and and use READ_ONCE() / WRITE_ONCE() for the lockless
pnode handoff.

V3:
. Redo the patch based on Paul's review.
Delay dropping the SCSI rport node reference until
lpfc_dev_loss_tmo_callbk() detaches the rport from the lpfc node.

V4:
. Fix the node lead reference when the remote port reconnects before
dev_loss_tmo expires. If fc_remote_port_add() reuses the existing SCSI
rport, transfer the reference that is still held on the rport instead
of acquiring a new reference.

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 4f68038789b5..582efb297c74 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -162,6 +162,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	struct lpfc_work_evt *evtp;
 	unsigned long iflags;
 	bool drop_initial_node_ref = false;
+	bool drop_scsi_node_ref = false;
 
 	ndlp = ((struct lpfc_rport_data *)rport->dd_data)->pnode;
 	if (!ndlp)
@@ -202,6 +203,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 		 */
 		if (ndlp->fc4_xpt_flags & SCSI_XPT_REGD) {
 			ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+			drop_scsi_node_ref = true;
 
 			/* If NLP_XPT_REGD was cleared in lpfc_nlp_unreg_node,
 			 * unregister calls were made to the scsi and nvme
@@ -213,9 +215,6 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 				if (!(ndlp->fc4_xpt_flags & NVME_XPT_REGD))
 					ndlp->fc4_xpt_flags &= ~NLP_XPT_REGD;
 				spin_unlock_irqrestore(&ndlp->lock, iflags);
-
-				/* Release scsi transport reference */
-				lpfc_nlp_put(ndlp);
 			} else {
 				spin_unlock_irqrestore(&ndlp->lock, iflags);
 			}
@@ -223,6 +222,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 			spin_unlock_irqrestore(&ndlp->lock, iflags);
 		}
 
+		if (drop_scsi_node_ref)
+			lpfc_nlp_put(ndlp);
 		if (drop_initial_node_ref)
 			lpfc_nlp_put(ndlp);
 		return;
@@ -271,6 +272,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 	 * rport. Remove the association between rport and ndlp.
 	 */
 	ndlp->fc4_xpt_flags &= ~SCSI_XPT_REGD;
+	drop_scsi_node_ref = true;
 	((struct lpfc_rport_data *)rport->dd_data)->pnode = NULL;
 	ndlp->rport = NULL;
 	spin_unlock_irqrestore(&ndlp->lock, iflags);
@@ -287,6 +289,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 			list_add_tail(&evtp->evt_listp, &phba->work_list);
 			spin_unlock_irqrestore(&phba->hbalock, iflags);
 			lpfc_worker_wake_up(phba);
+			if (drop_scsi_node_ref)
+				lpfc_nlp_put(ndlp);
 			return;
 		}
 		spin_unlock_irqrestore(&phba->hbalock, iflags);
@@ -304,6 +308,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
 						NLP_EVT_DEVICE_RM);
 		}
 	}
+	if (drop_scsi_node_ref)
+		lpfc_nlp_put(ndlp);
 }
 
 /**
@@ -4475,6 +4481,7 @@ static void
 lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 {
 	struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
+	struct fc_rport  *old_rport = ndlp->rport;
 	struct fc_rport  *rport;
 	struct lpfc_rport_data *rdata;
 	struct fc_rport_identifiers rport_ids;
@@ -4510,7 +4517,10 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 	rport->maxframe_size = ndlp->nlp_maxframe;
 	rport->supported_classes = ndlp->nlp_class_sup;
 	rdata = rport->dd_data;
-	rdata->pnode = lpfc_nlp_get(ndlp);
+	if (rport == old_rport)
+		rdata->pnode = ndlp;
+	else
+		rdata->pnode = lpfc_nlp_get(ndlp);
 	if (!rdata->pnode) {
 		dev_warn(&phba->pcidev->dev,
 			 "Warning - node ref failed. Unreg rport\n");
@@ -4570,7 +4580,6 @@ lpfc_unregister_remote_port(struct lpfc_nodelist *ndlp)
 			 kref_read(&ndlp->kref));
 
 	fc_remote_port_delete(rport);
-	lpfc_nlp_put(ndlp);
 }
 
 static void
-- 
2.47.3