Re: [PATCH 3/8] nvme-auth: switch to use 'struct key'

[Chris Leech <cleech@redhat.com>]

On Tue, Mar 17, 2026 at 02:00:58PM +0100, Hannes Reinecke wrote:
> Use the new key type 'dhchap' to store the DH-HMAC-CHAP keys and modify
> handling function to use 'struct key'. With that we can drop the now
> unused 'struct nvme_dhchap_key' definitions.
> 
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
>  drivers/nvme/common/Kconfig   |   1 +
>  drivers/nvme/common/auth.c    | 191 +++++++++++++---------------------
>  drivers/nvme/common/keyring.c |  98 +++++++++++++++++
>  drivers/nvme/host/Kconfig     |   1 -
>  drivers/nvme/host/auth.c      |  28 +++--
>  drivers/nvme/host/nvme.h      |   4 +-
>  drivers/nvme/host/sysfs.c     |  26 +++--
>  drivers/nvme/target/Kconfig   |   1 -
>  drivers/nvme/target/auth.c    |  40 +++----
>  drivers/nvme/target/nvmet.h   |   4 +-
>  include/linux/nvme-auth.h     |  17 +--
>  include/linux/nvme-keyring.h  |  22 +++-
>  12 files changed, 256 insertions(+), 177 deletions(-)
... 
> @@ -180,42 +181,43 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq, bool reset)
>  		ctrl->shash_id = host->dhchap_hash_id;
>  	}
>  
> -	/* Skip the 'DHHC-1:XX:' prefix */
> -	nvme_auth_free_key(ctrl->host_key);
> -	ctrl->host_key = nvme_auth_extract_key(host->dhchap_secret + 10,
> -					       host->dhchap_key_hash);
> +	key_put(ctrl->host_key);
> +	ctrl->host_key = nvme_auth_extract_key(NULL, host->dhchap_secret,
> +					       strlen(host->dhchap_secret));
>  	if (IS_ERR(ctrl->host_key)) {
>  		ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
>  		ctrl->host_key = NULL;
>  		goto out_free_hash;
>  	}
> -	pr_debug("%s: using hash %s key %*ph\n", __func__,
> -		 ctrl->host_key->hash > 0 ?
> -		 nvme_auth_hmac_name(ctrl->host_key->hash) : "none",
> -		 (int)ctrl->host_key->len, ctrl->host_key->key);
> +	host_hash = nvme_dhchap_psk_hash(ctrl->host_key);
> +	pr_debug("%s: using hash %s key %u\n", __func__,
> +		 ctrl_hash > 0 ?
> +		 nvme_auth_hmac_name(ctrl_hash) : "none",

Use of uninitialized ctrl_hash, should this be host_hash?

> --- a/include/linux/nvme-keyring.h
> +++ b/include/linux/nvme-keyring.h
> @@ -18,9 +18,14 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
>  
>  key_serial_t nvme_keyring_id(void);
>  struct key *nvme_tls_key_lookup(key_serial_t key_id);
> +
> +struct key *nvme_dhchap_psk_create(struct key *keyring,
> +		const u8 *data, size_t data_len);
> +struct key *nvme_dhchap_psk_lookup(struct key *keyring, const char *identity);
> +u8 nvme_dhchap_psk_hash(struct key *key);
> +
>  #else
>  static inline struct key *nvme_tls_psk_refresh(struct key *keyring,
> -		const char *hostnqn, char *subnqn, u8 hmac_id,
>  		u8 *data, size_t data_len, const char *digest)

This looks like a mistake, it changes the signature of the stub but
there is no change to the actual implemenataion.

>  {
>  	return ERR_PTR(-ENOTSUPP);
> @@ -38,5 +43,20 @@ static inline struct key *nvme_tls_key_lookup(key_serial_t key_id)
>  {
>  	return ERR_PTR(-ENOTSUPP);
>  }
> +static inline struct key *nvme_dhchap_psk_refresh(struct key *keyring,
> +		const char *hostnqn, const char *subnqn,
> +		u8 *data, size_t data_len)

This looks like a stub for something that doesn't exist, it's unused code.

> +{
> +	return ERR_PTR(-ENOTSUPP);
> +}
> +static inline struct key *nvme_dhchap_psk_lookup(struct key *keyring,
> +		const char *hostnqn, const char *subnqn, u8 hmac);

This stub function signature does not match the real one.

- Chris