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

Chris Leech
2026-04-01T18:37:09Z

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