linux-kernel - [PATCH v2 4/5] nvme-fc: Make initial connect attempt synchronous

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230620133711.22840-5-dwagner@suse.de>
Date:   Tue, 20 Jun 2023 15:37:10 +0200
From:   Daniel Wagner <dwagner@...e.de>
To:     linux-nvme@...ts.infradead.org
Cc:     linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
        Chaitanya Kulkarni <kch@...dia.com>,
        Shin'ichiro Kawasaki <shinichiro@...tmail.com>,
        Sagi Grimberg <sagi@...mberg.me>,
        Hannes Reinecke <hare@...e.de>,
        James Smart <jsmart2021@...il.com>,
        Daniel Wagner <dwagner@...e.de>
Subject: [PATCH v2 4/5] nvme-fc: Make initial connect attempt synchronous

Commit 4c984154efa1 ("nvme-fc: change controllers first connect to use
reconnect path") made the connection attempt asynchronous in order to
make the connection attempt from autoconnect/boot via udev/systemd up
case a bit more reliable.

Unfortunately, one side effect of this is that any wrong parameters
provided from userspace will not be directly reported as invalid, e.g.
auth keys.

So instead having the policy code inside the kernel it's better to
address this in userspace, for example in nvme-cli or nvme-stas.

This aligns the fc transport with tcp and rdma.

Signed-off-by: Daniel Wagner <dwagner@...e.de>
---
 drivers/nvme/host/fc.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 472ed285fd45..aa2911f07c6c 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2943,6 +2943,8 @@ nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl)
 	/* force put free routine to ignore io queues */
 	ctrl->ctrl.tagset = NULL;
 
+	if (ret > 0)
+		ret = -EIO;
 	return ret;
 }
 
@@ -3545,21 +3547,15 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 	list_add_tail(&ctrl->ctrl_list, &rport->ctrl_list);
 	spin_unlock_irqrestore(&rport->lock, flags);
 
-	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_RESETTING) ||
-	    !nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING)) {
+	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING)) {
 		dev_err(ctrl->ctrl.device,
 			"NVME-FC{%d}: failed to init ctrl state\n", ctrl->cnum);
 		goto fail_ctrl;
 	}
 
-	if (!queue_delayed_work(nvme_wq, &ctrl->connect_work, 0)) {
-		dev_err(ctrl->ctrl.device,
-			"NVME-FC{%d}: failed to schedule initial connect\n",
-			ctrl->cnum);
+	ret = nvme_fc_create_association(ctrl);
+	if (ret)
 		goto fail_ctrl;
-	}
-
-	flush_delayed_work(&ctrl->connect_work);
 
 	dev_info(ctrl->ctrl.device,
 		"NVME-FC{%d}: new ctrl: NQN \"%s\"\n",
@@ -3568,7 +3564,6 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 	return &ctrl->ctrl;
 
 fail_ctrl:
-	nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_DELETING);
 	cancel_work_sync(&ctrl->ioerr_work);
 	cancel_work_sync(&ctrl->ctrl.reset_work);
 	cancel_delayed_work_sync(&ctrl->connect_work);
@@ -3590,7 +3585,9 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 	 */
 	nvme_fc_rport_get(rport);
 
-	return ERR_PTR(-EIO);
+	if (ret > 0)
+		ret = -EIO;
+	return ERR_PTR(ret);
 
 out_free_queues:
 	kfree(ctrl->queues);
-- 
2.41.0