Hannes Reinecke 6123a0
From: James Smart <jsmart2021@gmail.com>
Hannes Reinecke 6123a0
Date: Wed, 18 Mar 2020 14:41:12 -0700
Hannes Reinecke 6123a0
Subject: [PATCH] nvme-fcloop: fix deallocation of working context
Hannes Reinecke 6123a0
Git-commit: 38803fcffb5baf40cd403c1bd980f22308aefee8
Michal Kubecek 66e035
Patch-mainline: v5.7-rc1
Hannes Reinecke 6123a0
References: bsc#1169045
Hannes Reinecke 6123a0
Hannes Reinecke 6123a0
There's been a longstanding bug of LS completions which freed ls ops,
Hannes Reinecke 6123a0
particularly the disconnect LS, while executing on a work context that
Hannes Reinecke 6123a0
is in the memory being free. Not a good thing to do.
Hannes Reinecke 6123a0
Hannes Reinecke 6123a0
Rework LS handling to make callbacks in the rport context rather than
Hannes Reinecke 6123a0
the ls_request context.
Hannes Reinecke 6123a0
Hannes Reinecke 6123a0
Signed-off-by: James Smart <jsmart2021@gmail.com>
Hannes Reinecke 6123a0
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Hannes Reinecke 6123a0
Reviewed-by: Hannes Reinecke <hare@suse.de>
Hannes Reinecke 6123a0
Signed-off-by: Christoph Hellwig <hch@lst.de>
Hannes Reinecke 6123a0
Acked-by: Hannes Reinecke <hare@suse.com>
Hannes Reinecke 6123a0
---
Hannes Reinecke 6123a0
 drivers/nvme/target/fcloop.c | 76 ++++++++++++++++++++++++++++++--------------
Hannes Reinecke 6123a0
 1 file changed, 52 insertions(+), 24 deletions(-)
Hannes Reinecke 6123a0
Hannes Reinecke 6123a0
diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
Hannes Reinecke 6123a0
index 1c50af6219f3..9861fcea39f6 100644
Hannes Reinecke 6123a0
--- a/drivers/nvme/target/fcloop.c
Hannes Reinecke 6123a0
+++ b/drivers/nvme/target/fcloop.c
Hannes Reinecke 6123a0
@@ -198,10 +198,13 @@ struct fcloop_lport_priv {
Hannes Reinecke 6123a0
 };
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 struct fcloop_rport {
Hannes Reinecke 6123a0
-	struct nvme_fc_remote_port *remoteport;
Hannes Reinecke 6123a0
-	struct nvmet_fc_target_port *targetport;
Hannes Reinecke 6123a0
-	struct fcloop_nport *nport;
Hannes Reinecke 6123a0
-	struct fcloop_lport *lport;
Hannes Reinecke 6123a0
+	struct nvme_fc_remote_port	*remoteport;
Hannes Reinecke 6123a0
+	struct nvmet_fc_target_port	*targetport;
Hannes Reinecke 6123a0
+	struct fcloop_nport		*nport;
Hannes Reinecke 6123a0
+	struct fcloop_lport		*lport;
Hannes Reinecke 6123a0
+	spinlock_t			lock;
Hannes Reinecke 6123a0
+	struct list_head		ls_list;
Hannes Reinecke 6123a0
+	struct work_struct		ls_work;
Hannes Reinecke 6123a0
 };
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 struct fcloop_tport {
Hannes Reinecke 6123a0
@@ -224,11 +227,10 @@ struct fcloop_nport {
Hannes Reinecke 6123a0
 };
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 struct fcloop_lsreq {
Hannes Reinecke 6123a0
-	struct fcloop_tport		*tport;
Hannes Reinecke 6123a0
 	struct nvmefc_ls_req		*lsreq;
Hannes Reinecke 6123a0
-	struct work_struct		work;
Hannes Reinecke 6123a0
 	struct nvmefc_tgt_ls_req	tgt_ls_req;
Hannes Reinecke 6123a0
 	int				status;
Hannes Reinecke 6123a0
+	struct list_head		ls_list; /* fcloop_rport->ls_list */
Hannes Reinecke 6123a0
 };
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 struct fcloop_rscn {
Hannes Reinecke 6123a0
@@ -292,21 +294,32 @@ fcloop_delete_queue(struct nvme_fc_local_port *localport,
Hannes Reinecke 6123a0
 {
Hannes Reinecke 6123a0
 }
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
-
Hannes Reinecke 6123a0
-/*
Hannes Reinecke 6123a0
- * Transmit of LS RSP done (e.g. buffers all set). call back up
Hannes Reinecke 6123a0
- * initiator "done" flows.
Hannes Reinecke 6123a0
- */
Hannes Reinecke 6123a0
 static void
Hannes Reinecke 6123a0
-fcloop_tgt_lsrqst_done_work(struct work_struct *work)
Hannes Reinecke 6123a0
+fcloop_rport_lsrqst_work(struct work_struct *work)
Hannes Reinecke 6123a0
 {
Hannes Reinecke 6123a0
-	struct fcloop_lsreq *tls_req =
Hannes Reinecke 6123a0
-		container_of(work, struct fcloop_lsreq, work);
Hannes Reinecke 6123a0
-	struct fcloop_tport *tport = tls_req->tport;
Hannes Reinecke 6123a0
-	struct nvmefc_ls_req *lsreq = tls_req->lsreq;
Hannes Reinecke 6123a0
+	struct fcloop_rport *rport =
Hannes Reinecke 6123a0
+		container_of(work, struct fcloop_rport, ls_work);
Hannes Reinecke 6123a0
+	struct fcloop_lsreq *tls_req;
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
-	if (!tport || tport->remoteport)
Hannes Reinecke 6123a0
-		lsreq->done(lsreq, tls_req->status);
Hannes Reinecke 6123a0
+	spin_lock(&rport->lock);
Hannes Reinecke 6123a0
+	for (;;) {
Hannes Reinecke 6123a0
+		tls_req = list_first_entry_or_null(&rport->ls_list,
Hannes Reinecke 6123a0
+				struct fcloop_lsreq, ls_list);
Hannes Reinecke 6123a0
+		if (!tls_req)
Hannes Reinecke 6123a0
+			break;
Hannes Reinecke 6123a0
+
Hannes Reinecke 6123a0
+		list_del(&tls_req->ls_list);
Hannes Reinecke 6123a0
+		spin_unlock(&rport->lock);
Hannes Reinecke 6123a0
+
Hannes Reinecke 6123a0
+		tls_req->lsreq->done(tls_req->lsreq, tls_req->status);
Hannes Reinecke 6123a0
+		/*
Hannes Reinecke 6123a0
+		 * callee may free memory containing tls_req.
Hannes Reinecke 6123a0
+		 * do not reference lsreq after this.
Hannes Reinecke 6123a0
+		 */
Hannes Reinecke 6123a0
+
Hannes Reinecke 6123a0
+		spin_lock(&rport->lock);
Hannes Reinecke 6123a0
+	}
Hannes Reinecke 6123a0
+	spin_unlock(&rport->lock);
Hannes Reinecke 6123a0
 }
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 static int
Hannes Reinecke 6123a0
@@ -319,17 +332,18 @@ fcloop_ls_req(struct nvme_fc_local_port *localport,
Hannes Reinecke 6123a0
 	int ret = 0;
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 	tls_req->lsreq = lsreq;
Hannes Reinecke 6123a0
-	INIT_WORK(&tls_req->work, fcloop_tgt_lsrqst_done_work);
Hannes Reinecke 6123a0
+	INIT_LIST_HEAD(&tls_req->ls_list);
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 	if (!rport->targetport) {
Hannes Reinecke 6123a0
 		tls_req->status = -ECONNREFUSED;
Hannes Reinecke 6123a0
-		tls_req->tport = NULL;
Hannes Reinecke 6123a0
-		schedule_work(&tls_req->work);
Hannes Reinecke 6123a0
+		spin_lock(&rport->lock);
Hannes Reinecke 6123a0
+		list_add_tail(&rport->ls_list, &tls_req->ls_list);
Hannes Reinecke 6123a0
+		spin_unlock(&rport->lock);
Hannes Reinecke 6123a0
+		schedule_work(&rport->ls_work);
Hannes Reinecke 6123a0
 		return ret;
Hannes Reinecke 6123a0
 	}
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 	tls_req->status = 0;
Hannes Reinecke 6123a0
-	tls_req->tport = rport->targetport->private;
Hannes Reinecke 6123a0
 	ret = nvmet_fc_rcv_ls_req(rport->targetport, &tls_req->tgt_ls_req,
Hannes Reinecke 6123a0
 				 lsreq->rqstaddr, lsreq->rqstlen);
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
@@ -337,18 +351,28 @@ fcloop_ls_req(struct nvme_fc_local_port *localport,
Hannes Reinecke 6123a0
 }
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 static int
Hannes Reinecke 6123a0
-fcloop_xmt_ls_rsp(struct nvmet_fc_target_port *tport,
Hannes Reinecke 6123a0
+fcloop_xmt_ls_rsp(struct nvmet_fc_target_port *targetport,
Hannes Reinecke 6123a0
 			struct nvmefc_tgt_ls_req *tgt_lsreq)
Hannes Reinecke 6123a0
 {
Hannes Reinecke 6123a0
 	struct fcloop_lsreq *tls_req = tgt_ls_req_to_lsreq(tgt_lsreq);
Hannes Reinecke 6123a0
 	struct nvmefc_ls_req *lsreq = tls_req->lsreq;
Hannes Reinecke 6123a0
+	struct fcloop_tport *tport = targetport->private;
Hannes Reinecke 6123a0
+	struct nvme_fc_remote_port *remoteport = tport->remoteport;
Hannes Reinecke 6123a0
+	struct fcloop_rport *rport;
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 	memcpy(lsreq->rspaddr, tgt_lsreq->rspbuf,
Hannes Reinecke 6123a0
 		((lsreq->rsplen < tgt_lsreq->rsplen) ?
Hannes Reinecke 6123a0
 				lsreq->rsplen : tgt_lsreq->rsplen));
Hannes Reinecke 6123a0
+
Hannes Reinecke 6123a0
 	tgt_lsreq->done(tgt_lsreq);
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
-	schedule_work(&tls_req->work);
Hannes Reinecke 6123a0
+	if (remoteport) {
Hannes Reinecke 6123a0
+		rport = remoteport->private;
Hannes Reinecke 6123a0
+		spin_lock(&rport->lock);
Hannes Reinecke 6123a0
+		list_add_tail(&rport->ls_list, &tls_req->ls_list);
Hannes Reinecke 6123a0
+		spin_unlock(&rport->lock);
Hannes Reinecke 6123a0
+		schedule_work(&rport->ls_work);
Hannes Reinecke 6123a0
+	}
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 	return 0;
Hannes Reinecke 6123a0
 }
Hannes Reinecke 6123a0
@@ -834,6 +858,7 @@ fcloop_remoteport_delete(struct nvme_fc_remote_port *remoteport)
Hannes Reinecke 6123a0
 {
Hannes Reinecke 6123a0
 	struct fcloop_rport *rport = remoteport->private;
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
+	flush_work(&rport->ls_work);
Hannes Reinecke 6123a0
 	fcloop_nport_put(rport->nport);
Hannes Reinecke 6123a0
 }
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
@@ -1136,6 +1161,9 @@ fcloop_create_remote_port(struct device *dev, struct device_attribute *attr,
Hannes Reinecke 6123a0
 	rport->nport = nport;
Hannes Reinecke 6123a0
 	rport->lport = nport->lport;
Hannes Reinecke 6123a0
 	nport->rport = rport;
Hannes Reinecke 6123a0
+	spin_lock_init(&rport->lock);
Hannes Reinecke 6123a0
+	INIT_WORK(&rport->ls_work, fcloop_rport_lsrqst_work);
Hannes Reinecke 6123a0
+	INIT_LIST_HEAD(&rport->ls_list);
Hannes Reinecke 6123a0
 
Hannes Reinecke 6123a0
 	return count;
Hannes Reinecke 6123a0
 }
Hannes Reinecke 6123a0
-- 
Hannes Reinecke 6123a0
2.16.4
Hannes Reinecke 6123a0