[PATCH v2] nvme-auth: Include SC_C in RVAL controller hash

[PATCH v2] nvme-auth: Include SC_C in RVAL controller hash

[alistair23]

From: Alistair Francis <alistair.francis@wdc.com>

Section 8.3.4.5.5 of the NVMe Base Specification 2.1 describes what is
included in the Response Value (RVAL) hash and SC_C should be included.
Currently we are hardcoding 0 instead of using the correct SC_C value.

Update the host and target code to use the SC_C when calculating the
RVAL instead of using 0.

Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
v2:
 - Rebase using new nvme_auth_hmac_update() functions

 drivers/nvme/host/auth.c   | 3 ++-
 drivers/nvme/target/auth.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index bbedbe181c8a6..63f543e809985 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -535,11 +535,12 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
 	put_unaligned_le16(chap->transaction, buf);
 	nvme_auth_hmac_update(&hmac, buf, 2);
 
-	memset(buf, 0, 4);
+	*buf = chap->sc_c;
 	nvme_auth_hmac_update(&hmac, buf, 1);
 	nvme_auth_hmac_update(&hmac, "Controller", 10);
 	nvme_auth_hmac_update(&hmac, ctrl->opts->subsysnqn,
 			      strlen(ctrl->opts->subsysnqn));
+	memset(buf, 0, 4);
 	nvme_auth_hmac_update(&hmac, buf, 1);
 	nvme_auth_hmac_update(&hmac, ctrl->opts->host->nqn,
 			      strlen(ctrl->opts->host->nqn));
diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index b34610e2f19d4..f032855b21477 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -402,11 +402,12 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 	put_unaligned_le16(req->sq->dhchap_tid, buf);
 	nvme_auth_hmac_update(&hmac, buf, 2);
 
-	memset(buf, 0, 4);
+	*buf = req->sq->sc_c;
 	nvme_auth_hmac_update(&hmac, buf, 1);
 	nvme_auth_hmac_update(&hmac, "Controller", 10);
 	nvme_auth_hmac_update(&hmac, ctrl->subsys->subsysnqn,
 			      strlen(ctrl->subsys->subsysnqn));
+	memset(buf, 0, 4);
 	nvme_auth_hmac_update(&hmac, buf, 1);
 	nvme_auth_hmac_update(&hmac, ctrl->hostnqn, strlen(ctrl->hostnqn));
 	nvme_auth_hmac_final(&hmac, response);
-- 
2.53.0

Re: [PATCH v2] nvme-auth: Include SC_C in RVAL controller hash

[Chris Leech]

On Thu, Apr 16, 2026 at 09:08:24AM +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Section 8.3.4.5.5 of the NVMe Base Specification 2.1 describes what is
> included in the Response Value (RVAL) hash and SC_C should be included.
> Currently we are hardcoding 0 instead of using the correct SC_C value.
> 
> Update the host and target code to use the SC_C when calculating the
> RVAL instead of using 0.
> 
> Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> v2:
>  - Rebase using new nvme_auth_hmac_update() functions
> 
>  drivers/nvme/host/auth.c   | 3 ++-
>  drivers/nvme/target/auth.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

Thanks, this was a good catch.  Fix looks good to me.

Reviewed-by: Chris Leech <cleech@redhat.com>

Re: [PATCH v2] nvme-auth: Include SC_C in RVAL controller hash

[Christoph Hellwig]

On Thu, Apr 16, 2026 at 09:08:24AM +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Section 8.3.4.5.5 of the NVMe Base Specification 2.1 describes what is
> included in the Response Value (RVAL) hash and SC_C should be included.
> Currently we are hardcoding 0 instead of using the correct SC_C value.
> 
> Update the host and target code to use the SC_C when calculating the
> RVAL instead of using 0.

This looks correct.  But I guess this breaks existing implementations
in the wild now?

Re: [PATCH v2] nvme-auth: Include SC_C in RVAL controller hash

[Alistair Francis]

On Thu, Apr 16, 2026 at 3:16 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Apr 16, 2026 at 09:08:24AM +1000, alistair23@gmail.com wrote:
> > From: Alistair Francis <alistair.francis@wdc.com>
> >
> > Section 8.3.4.5.5 of the NVMe Base Specification 2.1 describes what is
> > included in the Response Value (RVAL) hash and SC_C should be included.
> > Currently we are hardcoding 0 instead of using the correct SC_C value.
> >
> > Update the host and target code to use the SC_C when calculating the
> > RVAL instead of using 0.
>
> This looks correct.  But I guess this breaks existing implementations
> in the wild now?

It would break an implementation that is using non zero sc_c and
updates one of the Linux target or Linux host but not the other.

Note that similar changes have been made recently to "HostHost" and
didn't seem to break everything

7e091add9c43 nvme-auth: update sc_c in host response
159de7a825ae nvmet-auth: update sc_c in target host hash calculation

Alistair

Re: [PATCH v2] nvme-auth: Include SC_C in RVAL controller hash

[Hannes Reinecke]

On 4/16/26 01:08, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Section 8.3.4.5.5 of the NVMe Base Specification 2.1 describes what is
> included in the Response Value (RVAL) hash and SC_C should be included.
> Currently we are hardcoding 0 instead of using the correct SC_C value.
> 
> Update the host and target code to use the SC_C when calculating the
> RVAL instead of using 0.
> 
> Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> v2:
>   - Rebase using new nvme_auth_hmac_update() functions
> 
>   drivers/nvme/host/auth.c   | 3 ++-
>   drivers/nvme/target/auth.c | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
I would argue that this is a fix to secure concatenation, as the
original implementation (ie the quoted commit) did not support
secure concatenation (which sets 'sc_c' to a non-zero value).

Can you adapt the 'Fixes' line?

> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
> index bbedbe181c8a6..63f543e809985 100644
> --- a/drivers/nvme/host/auth.c
> +++ b/drivers/nvme/host/auth.c
> @@ -535,11 +535,12 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
>   	put_unaligned_le16(chap->transaction, buf);
>   	nvme_auth_hmac_update(&hmac, buf, 2);
>   
> -	memset(buf, 0, 4);
> +	*buf = chap->sc_c;
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, "Controller", 10);
>   	nvme_auth_hmac_update(&hmac, ctrl->opts->subsysnqn,
>   			      strlen(ctrl->opts->subsysnqn));
> +	memset(buf, 0, 4);
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, ctrl->opts->host->nqn,
>   			      strlen(ctrl->opts->host->nqn));
> diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
> index b34610e2f19d4..f032855b21477 100644
> --- a/drivers/nvme/target/auth.c
> +++ b/drivers/nvme/target/auth.c
> @@ -402,11 +402,12 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
>   	put_unaligned_le16(req->sq->dhchap_tid, buf);
>   	nvme_auth_hmac_update(&hmac, buf, 2);
>   
> -	memset(buf, 0, 4);
> +	*buf = req->sq->sc_c;
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, "Controller", 10);
>   	nvme_auth_hmac_update(&hmac, ctrl->subsys->subsysnqn,
>   			      strlen(ctrl->subsys->subsysnqn));
> +	memset(buf, 0, 4);
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, ctrl->hostnqn, strlen(ctrl->hostnqn));
>   	nvme_auth_hmac_final(&hmac, response);

Otherwise looks good.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich