Skip to content

Commit

Permalink
scsi: lpfc: Prevent use-after-free during rmmod with mapped NVMe rports
Browse files Browse the repository at this point in the history
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Martin K. Petersen <[email protected]>
  • Loading branch information
Justin Tee authored and martinkpetersen committed Sep 14, 2023
1 parent 9c30349 commit dae40be
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
3 changes: 2 additions & 1 deletion drivers/scsi/lpfc/lpfc_hbadisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
24 changes: 17 additions & 7 deletions drivers/scsi/lpfc/lpfc_nvme.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit dae40be

Please sign in to comment.