On Thu, Apr 07, 2022 at 07:13:08PM -0500, Mike Christie wrote:
> 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@xxxxxxxxxx>
> ---
> drivers/scsi/scsi_transport_iscsi.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
Reviewed-by: Chris Leech <cleech@xxxxxxxxxx>
> 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
>
