[PATCH 1/9] nvme-auth: modify nvme_auth_transform_key() to return status
Sagi Grimberg
sagi at grimberg.me
Tue Nov 25 23:39:36 PST 2025
Patch title is misleading. The addition is the transformed secret output...
On 28/05/2025 17:05, Hannes Reinecke wrote:
> Modify nvme_auth_transform_key() to return a status and provide
> the transformed data as argument on the command line as raw data.
The patch is missing the why explanation. I mean it looks fine, its unclear
why we need this change.
>
> Signed-off-by: Hannes Reinecke <hare at kernel.org>
> ---
> drivers/nvme/common/auth.c | 72 +++++++++++++++++++++++---------------
> drivers/nvme/host/auth.c | 40 +++++++++++----------
> drivers/nvme/target/auth.c | 36 +++++++++----------
> include/linux/nvme-auth.h | 4 +--
> 4 files changed, 85 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
> index 3b6d759bcdf2..918c92cbd8c5 100644
> --- a/drivers/nvme/common/auth.c
> +++ b/drivers/nvme/common/auth.c
> @@ -237,70 +237,86 @@ void nvme_auth_free_key(struct nvme_dhchap_key *key)
> }
> EXPORT_SYMBOL_GPL(nvme_auth_free_key);
>
> -struct nvme_dhchap_key *nvme_auth_transform_key(
> - struct nvme_dhchap_key *key, char *nqn)
> +int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
> + u8 **transformed_secret)
> {
> const char *hmac_name;
> struct crypto_shash *key_tfm;
> SHASH_DESC_ON_STACK(shash, key_tfm);
> - struct nvme_dhchap_key *transformed_key;
> - int ret, key_len;
> + u8 *transformed_data;
> + u8 *key_data;
> + size_t transformed_len;
> + int ret;
>
> if (!key) {
> pr_warn("No key specified\n");
> - return ERR_PTR(-ENOKEY);
> + return -ENOKEY;
> }
> + key_data = kzalloc(key->len, GFP_KERNEL);
> + if (!key_data)
> + return -ENOMEM;
> + memcpy(key_data, key->key, key->len);
> if (key->hash == 0) {
> - key_len = nvme_auth_key_struct_size(key->len);
> - transformed_key = kmemdup(key, key_len, GFP_KERNEL);
> - if (!transformed_key)
> - return ERR_PTR(-ENOMEM);
> - return transformed_key;
> + *transformed_secret = key_data;
> + return key->len;
> }
> hmac_name = nvme_auth_hmac_name(key->hash);
> if (!hmac_name) {
> pr_warn("Invalid key hash id %d\n", key->hash);
> - return ERR_PTR(-EINVAL);
> + ret = -EINVAL;
> + goto out_free_data;
> }
>
> key_tfm = crypto_alloc_shash(hmac_name, 0, 0);
> - if (IS_ERR(key_tfm))
> - return ERR_CAST(key_tfm);
> + if (IS_ERR(key_tfm)) {
> + ret = PTR_ERR(key_tfm);
> + goto out_free_data;
> + }
>
> - key_len = crypto_shash_digestsize(key_tfm);
> - transformed_key = nvme_auth_alloc_key(key_len, key->hash);
> - if (!transformed_key) {
> + transformed_len = crypto_shash_digestsize(key_tfm);
> + if (transformed_len != key->len) {
> + pr_warn("incompatible digest size %ld for key (hash %s, len %ld)\n",
> + transformed_len, hmac_name, key->len);
> + ret = -EINVAL;
> + goto out_free_tfm;
> + }
> +
> + transformed_data = kzalloc(transformed_len, GFP_KERNEL);
> + if (!transformed_data) {
> ret = -ENOMEM;
> - goto out_free_key;
> + goto out_free_tfm;
> }
>
> shash->tfm = key_tfm;
> ret = crypto_shash_setkey(key_tfm, key->key, key->len);
> if (ret < 0)
> - goto out_free_transformed_key;
> + goto out_free_transformed_data;
> ret = crypto_shash_init(shash);
> if (ret < 0)
> - goto out_free_transformed_key;
> + goto out_free_transformed_data;
> ret = crypto_shash_update(shash, nqn, strlen(nqn));
> if (ret < 0)
> - goto out_free_transformed_key;
> + goto out_free_transformed_data;
> ret = crypto_shash_update(shash, "NVMe-over-Fabrics", 17);
> if (ret < 0)
> - goto out_free_transformed_key;
> - ret = crypto_shash_final(shash, transformed_key->key);
> + goto out_free_transformed_data;
> + ret = crypto_shash_final(shash, transformed_data);
> if (ret < 0)
> - goto out_free_transformed_key;
> + goto out_free_transformed_data;
>
> crypto_free_shash(key_tfm);
> + *transformed_secret = transformed_data;
>
> - return transformed_key;
> + return transformed_len;
>
> -out_free_transformed_key:
> - nvme_auth_free_key(transformed_key);
> -out_free_key:
> +out_free_transformed_data:
> + kfree(transformed_data);
> +out_free_tfm:
> crypto_free_shash(key_tfm);
> +out_free_data:
> + kfree(key_data);
>
> - return ERR_PTR(ret);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(nvme_auth_transform_key);
>
> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
> index f6ddbe553289..9e7c2e889ee0 100644
> --- a/drivers/nvme/host/auth.c
> +++ b/drivers/nvme/host/auth.c
> @@ -24,7 +24,8 @@ struct nvme_dhchap_queue_context {
> struct nvme_ctrl *ctrl;
> struct crypto_shash *shash_tfm;
> struct crypto_kpp *dh_tfm;
> - struct nvme_dhchap_key *transformed_key;
> + u8 *transformed_secret;
> + size_t transformed_len;
> void *buf;
> int qid;
> int error;
> @@ -437,21 +438,20 @@ static int nvme_auth_dhchap_setup_host_response(struct nvme_ctrl *ctrl,
> dev_dbg(ctrl->device, "%s: qid %d host response seq %u transaction %d\n",
> __func__, chap->qid, chap->s1, chap->transaction);
>
> - if (!chap->transformed_key) {
> - chap->transformed_key = nvme_auth_transform_key(ctrl->host_key,
> - ctrl->opts->host->nqn);
> - if (IS_ERR(chap->transformed_key)) {
> - ret = PTR_ERR(chap->transformed_key);
> - chap->transformed_key = NULL;
> + if (!chap->transformed_secret) {
> + ret = nvme_auth_transform_key(ctrl->host_key,
> + ctrl->opts->host->nqn,
> + &chap->transformed_secret);
> + if (ret < 0)
> return ret;
> - }
> + chap->transformed_len = ret;
> } else {
> dev_dbg(ctrl->device, "%s: qid %d re-using host response\n",
> __func__, chap->qid);
> }
>
> ret = crypto_shash_setkey(chap->shash_tfm,
> - chap->transformed_key->key, chap->transformed_key->len);
> + chap->transformed_secret, chap->transformed_len);
> if (ret) {
> dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
> chap->qid, ret);
> @@ -517,19 +517,20 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
> struct nvme_dhchap_queue_context *chap)
> {
> SHASH_DESC_ON_STACK(shash, chap->shash_tfm);
> - struct nvme_dhchap_key *transformed_key;
> + u8 *transformed_secret;
> + size_t transformed_len;
> u8 buf[4], *challenge = chap->c2;
> int ret;
>
> - transformed_key = nvme_auth_transform_key(ctrl->ctrl_key,
> - ctrl->opts->subsysnqn);
> - if (IS_ERR(transformed_key)) {
> - ret = PTR_ERR(transformed_key);
> + ret = nvme_auth_transform_key(ctrl->ctrl_key,
> + ctrl->opts->subsysnqn,
> + &transformed_secret);
> + if (ret < 0)
> return ret;
> - }
> + transformed_len = ret;
>
> ret = crypto_shash_setkey(chap->shash_tfm,
> - transformed_key->key, transformed_key->len);
> + transformed_secret, transformed_len);
> if (ret) {
> dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
> chap->qid, ret);
> @@ -595,7 +596,7 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
> out:
> if (challenge != chap->c2)
> kfree(challenge);
> - nvme_auth_free_key(transformed_key);
> + kfree(transformed_secret);
> return ret;
> }
>
> @@ -657,8 +658,9 @@ static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl,
>
> static void nvme_auth_reset_dhchap(struct nvme_dhchap_queue_context *chap)
> {
> - nvme_auth_free_key(chap->transformed_key);
> - chap->transformed_key = NULL;
> + kfree(chap->transformed_secret);
> + chap->transformed_secret = NULL;
> + chap->transformed_len = 0;
> kfree_sensitive(chap->host_key);
> chap->host_key = NULL;
> chap->host_key_len = 0;
> diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
> index b340380f3892..c70d9d259a7f 100644
> --- a/drivers/nvme/target/auth.c
> +++ b/drivers/nvme/target/auth.c
> @@ -297,7 +297,8 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
> struct nvmet_ctrl *ctrl = req->sq->ctrl;
> const char *hash_name;
> u8 *challenge = req->sq->dhchap_c1;
> - struct nvme_dhchap_key *transformed_key;
> + u8 *transformed_secret;
> + size_t transformed_len;
> u8 buf[4];
> int ret;
>
> @@ -321,15 +322,14 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
> goto out_free_tfm;
> }
>
> - transformed_key = nvme_auth_transform_key(ctrl->host_key,
> - ctrl->hostnqn);
> - if (IS_ERR(transformed_key)) {
> - ret = PTR_ERR(transformed_key);
> + ret = nvme_auth_transform_key(ctrl->host_key, ctrl->hostnqn,
> + &transformed_secret);
> + if (ret < 0)
> goto out_free_tfm;
> - }
>
> - ret = crypto_shash_setkey(shash_tfm, transformed_key->key,
> - transformed_key->len);
> + transformed_len = ret;
> + ret = crypto_shash_setkey(shash_tfm, transformed_secret,
> + transformed_len);
> if (ret)
> goto out_free_response;
>
> @@ -389,7 +389,7 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
> if (challenge != req->sq->dhchap_c1)
> kfree(challenge);
> out_free_response:
> - nvme_auth_free_key(transformed_key);
> + kfree(transformed_secret);
> out_free_tfm:
> crypto_free_shash(shash_tfm);
> return ret;
> @@ -403,7 +403,8 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
> struct nvmet_ctrl *ctrl = req->sq->ctrl;
> const char *hash_name;
> u8 *challenge = req->sq->dhchap_c2;
> - struct nvme_dhchap_key *transformed_key;
> + u8 *transformed_secret;
> + size_t transformed_len;
> u8 buf[4];
> int ret;
>
> @@ -427,15 +428,14 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
> goto out_free_tfm;
> }
>
> - transformed_key = nvme_auth_transform_key(ctrl->ctrl_key,
> - ctrl->subsysnqn);
> - if (IS_ERR(transformed_key)) {
> - ret = PTR_ERR(transformed_key);
> + ret = nvme_auth_transform_key(ctrl->ctrl_key, ctrl->subsysnqn,
> + &transformed_secret);
> + if (ret < 0)
> goto out_free_tfm;
> - }
> + transformed_len = ret;
>
> - ret = crypto_shash_setkey(shash_tfm, transformed_key->key,
> - transformed_key->len);
> + ret = crypto_shash_setkey(shash_tfm, transformed_secret,
> + transformed_len);
> if (ret)
> goto out_free_response;
>
> @@ -500,7 +500,7 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
> if (challenge != req->sq->dhchap_c2)
> kfree(challenge);
> out_free_response:
> - nvme_auth_free_key(transformed_key);
> + kfree(transformed_secret);
> out_free_tfm:
> crypto_free_shash(shash_tfm);
> return ret;
> diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
> index 60e069a6757f..fd43fa042c88 100644
> --- a/include/linux/nvme-auth.h
> +++ b/include/linux/nvme-auth.h
> @@ -29,8 +29,8 @@ struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
> u8 key_hash);
> void nvme_auth_free_key(struct nvme_dhchap_key *key);
> struct nvme_dhchap_key *nvme_auth_alloc_key(u32 len, u8 hash);
> -struct nvme_dhchap_key *nvme_auth_transform_key(
> - struct nvme_dhchap_key *key, char *nqn);
> +int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
> + u8 **transformed_secret) ;
> int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key);
> int nvme_auth_augmented_challenge(u8 hmac_id, u8 *skey, size_t skey_len,
> u8 *challenge, u8 *aug, size_t hlen);
More information about the Linux-nvme
mailing list