From: James Smart <jsmart2021@gmail.com>

Date: Thu, 21 Nov 2019 09:59:37 -0800

Subject: [PATCH] nvme-fc: fix double-free scenarios on hw queues

Git-commit: c869e494ef8b5846d9ba91f1e922c23cd444f0c1

Patch-mainline: v5.5-rc2

References: bsc#1169045

If an error occurs on one of the ios used for creating an

association, the creating routine has error paths that are

invoked by the command failure and the error paths will free

up the controller resources created to that point.

But... the io was ultimately determined by an asynchronous

completion routine that detected the error and which

unconditionally invokes the error_recovery path which calls

delete_association. Delete association deletes all outstanding

io then tears down the controller resources. So the

create_association thread can be running in parallel with

the error_recovery thread. What was seen was the LLDD received

a call to delete a queue, causing the LLDD to do a free of a

resource, then the transport called the delete queue again

causing the driver to repeat the free call. The second free

routine corrupted the allocator. The transport shouldn't be

making the duplicate call, and the delete queue is just one

of the resources being freed.

To fix, it is realized that the create_association path is

completely serialized with one command at a time. So the

failed io completion will always be seen by the create_association

path and as of the failure, there are no ios to terminate and there

is no reason to be manipulating queue freeze states, etc.

The serialized condition stays true until the controller is

transitioned to the LIVE state. Thus the fix is to change the

error recovery path to check the controller state and only

invoke the teardown path if not already in the CONNECTING state.

Reviewed-by: Himanshu Madhani <hmadhani@marvell.com>

Reviewed-by: Ewan D. Milne <emilne@redhat.com>

Signed-off-by: James Smart <jsmart2021@gmail.com>

Signed-off-by: Keith Busch <kbusch@kernel.org>

Acked-by: Hannes Reinecke <hare@suse.com>

---

 drivers/nvme/host/fc.c | 18 +++++++++++++++---

 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c

index d61439f8f5a9..5a70ac395d53 100644

--- a/drivers/nvme/host/fc.c

+++ b/drivers/nvme/host/fc.c

@@ -2913,10 +2913,22 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status)

 static void

 __nvme_fc_terminate_io(struct nvme_fc_ctrl *ctrl)

 {

-	nvme_stop_keep_alive(&ctrl->ctrl);

+	/*

+	 * if state is connecting - the error occurred as part of a

+	 * reconnect attempt. The create_association error paths will

+	 * clean up any outstanding io.

+	 *

+	 * if it's a different state - ensure all pending io is

+	 * terminated. Given this can delay while waiting for the

+	 * aborted io to return, we recheck adapter state below

+	 * before changing state.

+	 */

+	if (ctrl->ctrl.state != NVME_CTRL_CONNECTING) {

+		nvme_stop_keep_alive(&ctrl->ctrl);

-	/* will block will waiting for io to terminate */

-	nvme_fc_delete_association(ctrl);

+		/* will block will waiting for io to terminate */

+		nvme_fc_delete_association(ctrl);

+	}

 	if (ctrl->ctrl.state != NVME_CTRL_CONNECTING &&

 	    !nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING))

-- 

2.16.4