lkml.org 
[lkml]   [2022]   [Dec]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: drivers/nvme/host/auth.c:950 nvme_auth_init_ctrl() warn: missing error code? 'ret'
From


On 12/23/22 17:47, Christoph Hellwig wrote:
> Based on the code in nvme_auth_generate_key I assume this is intentional,
> but the code looks really confusing.
>
> Hannes, Sagi, what do you think of something like this:
>
>
> diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
> index d90e4f0c08b7b9..a07eb4cd9ce173 100644
> --- a/drivers/nvme/common/auth.c
> +++ b/drivers/nvme/common/auth.c
> @@ -455,28 +455,18 @@ int nvme_auth_gen_shared_secret(struct crypto_kpp *dh_tfm,
> }
> EXPORT_SYMBOL_GPL(nvme_auth_gen_shared_secret);
>
> -int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key)
> +struct nvme_dhchap_key *nvme_auth_generate_key(u8 *secret)
> {
> - struct nvme_dhchap_key *key;
> u8 key_hash;
>
> - if (!secret) {
> - *ret_key = NULL;
> - return 0;
> - }
> + if (!secret)
> + return NULL;
>
> if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
>
> /* Pass in the secret without the 'DHHC-1:XX:' prefix */
> - key = nvme_auth_extract_key(secret + 10, key_hash);
> - if (IS_ERR(key)) {
> - *ret_key = NULL;
> - return PTR_ERR(key);
> - }
> -
> - *ret_key = key;
> - return 0;
> + return nvme_auth_extract_key(secret + 10, key_hash);
> }
> EXPORT_SYMBOL_GPL(nvme_auth_generate_key);
>
> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
> index bb0abbe4491cdc..c808652966a94f 100644
> --- a/drivers/nvme/host/auth.c
> +++ b/drivers/nvme/host/auth.c
> @@ -943,16 +943,19 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
> INIT_WORK(&ctrl->dhchap_auth_work, nvme_ctrl_auth_work);
> if (!ctrl->opts)
> return 0;
> - ret = nvme_auth_generate_key(ctrl->opts->dhchap_secret,
> - &ctrl->host_key);
> - if (ret)
> - return ret;
> - ret = nvme_auth_generate_key(ctrl->opts->dhchap_ctrl_secret,
> - &ctrl->ctrl_key);
> - if (ret)
> +
> + ctrl->host_key = nvme_auth_generate_key(ctrl->opts->dhchap_secret);
> + if (IS_ERR(ctrl->host_key)) {

nvme_auth_generate_key can return NULL, so in this case we should avoid
calling it if the secret is null here.

Other than that, this looks good.
Although I think that for this specific report, we should do a simple
fix and then make the code look better.

\
 
 \ /
  Last update: 2023-03-26 23:19    [W:0.096 / U:0.384 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site