nvmet: return DHCHAP status codes from nvmet_setup_auth()

A failure in nvmet_setup_auth() does not mean that the NVMe
authentication command failed, so we should rather return a protocol
error with a 'failure1' response than an NVMe status.

Also update the type used for dhchap_step and dhchap_status to u8 to
avoid confusions with nvme status. Furthermore, split dhchap_status and
nvme status so we don't accidentally mix these return values.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
[dwagner: - use u8 as type for dhchap_{step|status}
          - separate nvme status from dhcap_status]
Signed-off-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>

diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 9afc28f1ffac9562cbeb577b739dcf8cf249a296..53bf1a084469ea443f059cad6efe4a5830a16fab 100644 (file)
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -126,12 +126,11 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id)
        return ret;
 }
 
-int nvmet_setup_auth(struct nvmet_ctrl *ctrl)
+u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl)
 {
        int ret = 0;
        struct nvmet_host_link *p;
        struct nvmet_host *host = NULL;
-       const char *hash_name;
 
        down_read(&nvmet_config_sem);
        if (nvmet_is_disc_subsys(ctrl->subsys))
@@ -149,13 +148,16 @@ int nvmet_setup_auth(struct nvmet_ctrl *ctrl)
        }
        if (!host) {
                pr_debug("host %s not found\n", ctrl->hostnqn);
-               ret = -EPERM;
+               ret = NVME_AUTH_DHCHAP_FAILURE_FAILED;
                goto out_unlock;
        }
 
        ret = nvmet_setup_dhgroup(ctrl, host->dhchap_dhgroup_id);
-       if (ret < 0)
+       if (ret < 0) {
                pr_warn("Failed to setup DH group");
+               ret = NVME_AUTH_DHCHAP_FAILURE_DHGROUP_UNUSABLE;
+               goto out_unlock;
+       }
 
        if (!host->dhchap_secret) {
                pr_debug("No authentication provided\n");
@@ -166,12 +168,6 @@ int nvmet_setup_auth(struct nvmet_ctrl *ctrl)
                pr_debug("Re-use existing hash ID %d\n",
                         ctrl->shash_id);
        } else {
-               hash_name = nvme_auth_hmac_name(host->dhchap_hash_id);
-               if (!hash_name) {
-                       pr_warn("Hash ID %d invalid\n", host->dhchap_hash_id);
-                       ret = -EINVAL;
-                       goto out_unlock;
-               }
                ctrl->shash_id = host->dhchap_hash_id;
        }
 
@@ -180,7 +176,7 @@ int nvmet_setup_auth(struct nvmet_ctrl *ctrl)
        ctrl->host_key = nvme_auth_extract_key(host->dhchap_secret + 10,
                                               host->dhchap_key_hash);
        if (IS_ERR(ctrl->host_key)) {
-               ret = PTR_ERR(ctrl->host_key);
+               ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
                ctrl->host_key = NULL;
                goto out_free_hash;
        }
@@ -198,7 +194,7 @@ int nvmet_setup_auth(struct nvmet_ctrl *ctrl)
        ctrl->ctrl_key = nvme_auth_extract_key(host->dhchap_ctrl_secret + 10,
                                               host->dhchap_ctrl_key_hash);
        if (IS_ERR(ctrl->ctrl_key)) {
-               ret = PTR_ERR(ctrl->ctrl_key);
+               ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
                ctrl->ctrl_key = NULL;
                goto out_free_hash;
        }

diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index eb7785be0ca770c68c0084059f0a4f23b06f6dff..d61b8c6ff3b20ea60c39981b25a3157645c9d8ae 100644 (file)
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -31,7 +31,7 @@ void nvmet_auth_sq_init(struct nvmet_sq *sq)
        sq->dhchap_step = NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE;
 }
 
-static u16 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
+static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
 {
        struct nvmet_ctrl *ctrl = req->sq->ctrl;
        struct nvmf_auth_dhchap_negotiate_data *data = d;
@@ -109,7 +109,7 @@ static u16 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
        return 0;
 }
 
-static u16 nvmet_auth_reply(struct nvmet_req *req, void *d)
+static u8 nvmet_auth_reply(struct nvmet_req *req, void *d)
 {
        struct nvmet_ctrl *ctrl = req->sq->ctrl;
        struct nvmf_auth_dhchap_reply_data *data = d;
@@ -172,7 +172,7 @@ static u16 nvmet_auth_reply(struct nvmet_req *req, void *d)
        return 0;
 }
 
-static u16 nvmet_auth_failure2(void *d)
+static u8 nvmet_auth_failure2(void *d)
 {
        struct nvmf_auth_dhchap_failure_data *data = d;
 
@@ -186,6 +186,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
        void *d;
        u32 tl;
        u16 status = 0;
+       u8 dhchap_status;
 
        if (req->cmd->auth_send.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
                status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
@@ -237,30 +238,32 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
        if (data->auth_type == NVME_AUTH_COMMON_MESSAGES) {
                if (data->auth_id == NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE) {
                        /* Restart negotiation */
-                       pr_debug("%s: ctrl %d qid %d reset negotiation\n", __func__,
-                                ctrl->cntlid, req->sq->qid);
+                       pr_debug("%s: ctrl %d qid %d reset negotiation\n",
+                                __func__, ctrl->cntlid, req->sq->qid);
                        if (!req->sq->qid) {
-                               if (nvmet_setup_auth(ctrl) < 0) {
-                                       status = NVME_SC_INTERNAL;
-                                       pr_err("ctrl %d qid 0 failed to setup"
-                                              "re-authentication",
+                               dhchap_status = nvmet_setup_auth(ctrl);
+                               if (dhchap_status) {
+                                       pr_err("ctrl %d qid 0 failed to setup re-authentication\n",
                                               ctrl->cntlid);
-                                       goto done_failure1;
+                                       req->sq->dhchap_status = dhchap_status;
+                                       req->sq->dhchap_step =
+                                               NVME_AUTH_DHCHAP_MESSAGE_FAILURE1;
+                                       goto done_kfree;
                                }
                        }
-                       req->sq->dhchap_step = NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE;
+                       req->sq->dhchap_step =
+                               NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE;
                } else if (data->auth_id != req->sq->dhchap_step)
                        goto done_failure1;
                /* Validate negotiation parameters */
-               status = nvmet_auth_negotiate(req, d);
-               if (status == 0)
+               dhchap_status = nvmet_auth_negotiate(req, d);
+               if (dhchap_status == 0)
                        req->sq->dhchap_step =
                                NVME_AUTH_DHCHAP_MESSAGE_CHALLENGE;
                else {
                        req->sq->dhchap_step =
                                NVME_AUTH_DHCHAP_MESSAGE_FAILURE1;
-                       req->sq->dhchap_status = status;
-                       status = 0;
+                       req->sq->dhchap_status = dhchap_status;
                }
                goto done_kfree;
        }
@@ -284,15 +287,14 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
 
        switch (data->auth_id) {
        case NVME_AUTH_DHCHAP_MESSAGE_REPLY:
-               status = nvmet_auth_reply(req, d);
-               if (status == 0)
+               dhchap_status = nvmet_auth_reply(req, d);
+               if (dhchap_status == 0)
                        req->sq->dhchap_step =
                                NVME_AUTH_DHCHAP_MESSAGE_SUCCESS1;
                else {
                        req->sq->dhchap_step =
                                NVME_AUTH_DHCHAP_MESSAGE_FAILURE1;
-                       req->sq->dhchap_status = status;
-                       status = 0;
+                       req->sq->dhchap_status = dhchap_status;
                }
                goto done_kfree;
        case NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2:
@@ -301,13 +303,12 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
                         __func__, ctrl->cntlid, req->sq->qid);
                goto done_kfree;
        case NVME_AUTH_DHCHAP_MESSAGE_FAILURE2:
-               status = nvmet_auth_failure2(d);
-               if (status) {
+               dhchap_status = nvmet_auth_failure2(d);
+               if (dhchap_status) {
                        pr_warn("ctrl %d qid %d: authentication failed (%d)\n",
-                               ctrl->cntlid, req->sq->qid, status);
-                       req->sq->dhchap_status = status;
+                               ctrl->cntlid, req->sq->qid, dhchap_status);
+                       req->sq->dhchap_status = dhchap_status;
                        req->sq->authenticated = false;
-                       status = 0;
                }
                goto done_kfree;
        default:

diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index b23f4cf840bd541ca3f0215cda994549273a43b3..042b379cbb3647a1afba2446bb2a77499cb86735 100644 (file)
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -211,7 +211,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
        struct nvmf_connect_data *d;
        struct nvmet_ctrl *ctrl = NULL;
        u16 status;
-       int ret;
+       u8 dhchap_status;
 
        if (!nvmet_check_transfer_len(req, sizeof(struct nvmf_connect_data)))
                return;
@@ -254,11 +254,12 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
 
        uuid_copy(&ctrl->hostid, &d->hostid);
 
-       ret = nvmet_setup_auth(ctrl);
-       if (ret < 0) {
-               pr_err("Failed to setup authentication, error %d\n", ret);
+       dhchap_status = nvmet_setup_auth(ctrl);
+       if (dhchap_status) {
+               pr_err("Failed to setup authentication, dhchap status %u\n",
+                      dhchap_status);
                nvmet_ctrl_put(ctrl);
-               if (ret == -EPERM)
+               if (dhchap_status == NVME_AUTH_DHCHAP_FAILURE_FAILED)
                        status = (NVME_SC_CONNECT_INVALID_HOST | NVME_SC_DNR);
                else
                        status = NVME_SC_INTERNAL;

diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index f460728e1df1fd87e24969e782203c7684a23326..e4ba54b85511f2f98f0855a4984b3ec02e8dfcd7 100644 (file)
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -113,8 +113,8 @@ struct nvmet_sq {
        bool                    authenticated;
        struct delayed_work     auth_expired_work;
        u16                     dhchap_tid;
-       u16                     dhchap_status;
-       int                     dhchap_step;
+       u8                      dhchap_status;
+       u8                      dhchap_step;
        u8                      *dhchap_c1;
        u8                      *dhchap_c2;
        u32                     dhchap_s1;
@@ -713,7 +713,7 @@ void nvmet_execute_auth_receive(struct nvmet_req *req);
 int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
                       bool set_ctrl);
 int nvmet_auth_set_host_hash(struct nvmet_host *host, const char *hash);
-int nvmet_setup_auth(struct nvmet_ctrl *ctrl);
+u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl);
 void nvmet_auth_sq_init(struct nvmet_sq *sq);
 void nvmet_destroy_auth(struct nvmet_ctrl *ctrl);
 void nvmet_auth_sq_free(struct nvmet_sq *sq);
@@ -732,7 +732,7 @@ int nvmet_auth_ctrl_exponential(struct nvmet_req *req,
 int nvmet_auth_ctrl_sesskey(struct nvmet_req *req,
                            u8 *buf, int buf_size);
 #else
-static inline int nvmet_setup_auth(struct nvmet_ctrl *ctrl)
+static inline u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl)
 {
        return 0;
 }