[PATCH 04/10] scsi: iscsi: Fix endpoint reuse regression

From: Mike Christie <michael.christie@oracle.com>
To: skashyap@marvell.com, lduncan@suse.com, cleech@redhat.com,
	njavali@marvell.com, mrangankar@marvell.com,
	GR-QLogic-Storage-Upstream@marvell.com,
	martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
	jejb@linux.ibm.com
Cc: Mike Christie <michael.christie@oracle.com>
Subject: [PATCH 04/10] scsi: iscsi: Fix endpoint reuse regression
Date: Thu,  7 Apr 2022 19:13:08 -0500	[thread overview]
Message-ID: <20220408001314.5014-5-michael.christie@oracle.com> (raw)
In-Reply-To: <20220408001314.5014-1-michael.christie@oracle.com>

This patch fixes a bug where when using iscsi offload we can free a
endpoint while userspace still thinks it's active. That then causes the
endpoint ID to be reused for a new connection's endpoint while userspace
still thinks the ID is for the original connection. Userspace will then
end up disconnecting a running connection's endpoint or trying to bind
to another connection's endpoint.

This bug is a regression added in:

Commit 23d6fefbb3f6 ("scsi: iscsi: Fix in-kernel conn failure handling")

where we added a in kernel ep_disconnect call to fix a bug in:

Commit 0ab710458da1 ("scsi: iscsi: Perform connection failure entirely in
kernel space")

where we would call stop_conn without having done ep_disconnect. This
early ep_disconnect call will then free the endpoint and it's ID while
userspace still thinks the ID is valid.

This patch fixes the early release of the ID by having the in kernel
recovery code keep a reference to the endpoint until userspace has called
into the kernel to finish cleaning up the endpoint/connection. It requires
the previous patch "scsi: iscsi: Release endpoint ID when its freed."
which moved the freeing of the ID until when the endpoint is released.

Fixes: 23d6fefbb3f6 ("scsi: iscsi: Fix in-kernel conn failure handling")
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/scsi_transport_iscsi.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 1fc7c6bfbd67..f200da049f3b 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2247,7 +2247,11 @@ static void iscsi_if_disconnect_bound_ep(struct iscsi_cls_conn *conn,
 		mutex_unlock(&conn->ep_mutex);
 
 		flush_work(&conn->cleanup_work);
-
+		/*
+		 * Userspace is now done with the EP so we can release the ref
+		 * iscsi_cleanup_conn_work_fn took.
+		 */
+		iscsi_put_endpoint(ep);
 		mutex_lock(&conn->ep_mutex);
 	}
 }
@@ -2322,6 +2326,12 @@ static void iscsi_cleanup_conn_work_fn(struct work_struct *work)
 		return;
 	}
 
+	/*
+	 * Get a ref to the ep, so we don't release its ID until after
+	 * userspace is done referencing it in iscsi_if_disconnect_bound_ep.
+	 */
+	if (conn->ep)
+		get_device(&conn->ep->dev);
 	iscsi_ep_disconnect(conn, false);
 
 	if (system_state != SYSTEM_RUNNING) {
-- 
2.25.1

