mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] nvme: use the parsed offset for DH-CHAP secrets
@ 2026-11-09 14:59 shao.mingyin
  2026-11-09 14:59 ` [PATCH 1/2] nvme-auth: use the parsed offset for the DH-CHAP key shao.mingyin
  2026-11-09 14:59 ` [PATCH 2/2] nvmet-auth: store the DH-CHAP key offset instead of hardcoding it shao.mingyin
  0 siblings, 2 replies; 4+ messages in thread
From: shao.mingyin @ 2026-11-09 14:59 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, linux-nvme; +Cc: hare, linux-kernel, stable

Both the host and the target parse a DH-HMAC-CHAP secret with
sscanf("DHHC-1:%hhd:...") and then skip a hardcoded ten characters to
reach the key.  The prefix is only nine characters when the hash
identifier is one digit, so a short but accepted secret makes the key
pointer point past the end of the string and the follow-up strlen() read
past the allocation.

Patch 1 changes the host-side parser to record the offset of the key with
%n and use it instead of the constant.

Patch 2 makes the same change on the target side and stores the offset
with each configured secret, so nvmet_setup_auth() no longer adds the
constant to the stored string.

The two patches touch different trees (common/host and target) and are
independent.  Both carry Fixes: and Cc: stable.

Testing: both apply to next-20261105 (`ddfe51b0a0f5`), pass checkpatch.pl
--strict with 0 errors/0 warnings/0 checks, and the affected files build
under W=1.

Both were also reproduced on a KASAN kernel (7.3.0-next-20261030,
CONFIG_NVME_HOST_AUTH/TARGET_AUTH): a 9-byte "DHHC-1:1:" secret written
through the host sysfs attribute, and a target host entry configured with
the same key, both made nvme_auth_extract_key() read one byte past the
10-byte allocation (KASAN slab-out-of-bounds in strlen/strrchr). With the
two patches the same boots produce no KASAN report.

Shao Mingyin (2):
  nvme-auth: use the parsed offset for the DH-CHAP key
  nvmet-auth: store the DH-CHAP key offset instead of hardcoding it

 drivers/nvme/common/auth.c  |  6 ++++--
 drivers/nvme/target/auth.c  | 17 ++++++++++++-----
 drivers/nvme/target/nvmet.h |  2 ++
 3 files changed, 18 insertions(+), 7 deletions(-)

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] nvme-auth: use the parsed offset for the DH-CHAP key
  2026-11-09 14:59 [PATCH 0/2] nvme: use the parsed offset for DH-CHAP secrets shao.mingyin
@ 2026-11-09 14:59 ` shao.mingyin
  2026-11-10  7:21   ` Hannes Reinecke
  2026-11-09 14:59 ` [PATCH 2/2] nvmet-auth: store the DH-CHAP key offset instead of hardcoding it shao.mingyin
  1 sibling, 1 reply; 4+ messages in thread
From: shao.mingyin @ 2026-11-09 14:59 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, linux-nvme; +Cc: hare, linux-kernel, stable

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvme_auth_parse_key() accepts the DH-HMAC-CHAP key with

	if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
		return -EINVAL;

and then skips a hardcoded ten characters:

	key = nvme_auth_extract_key(secret + 10, key_hash);

The ten characters assume that the hash identifier is exactly two
characters long.  A string such as "DHHC-1:1:" still makes sscanf()
return 1, and secret + 10 then points one byte past the NUL terminator
of the buffer.  nvme_auth_extract_key() starts with
strlen(secret), so it reads past the end of the allocation.  The sysfs
dhchap_secret and dhchap_ctrl_secret stores only require seven
characters and a "DHHC-1:" prefix, so a short value reaches this code
(even though the nvme-fabrics option parser rejects secrets shorter than
eleven characters).

Use the %n conversion to obtain the real offset after the
"DHHC-1:<hash>:" prefix and reject a string where the prefix is not
complete.  This accepts both one and two character hash identifiers and
never starts the key past the terminator.

Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest, KASAN) with a live loop
controller and host DH-CHAP.

Writing the 9-byte secret "DHHC-1:1:" to the dhchap_secret sysfs
attribute makes the driver start the key ten bytes in, one byte past
the NUL terminator:

  unpatched                            patched
  KASAN slab-out-of-bounds in          write rejected (-EINVAL)
  strlen/strrchr, nvme_auth_extract_key

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index e2e0c736540a..1fdc4e4ed0b9 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -489,17 +489,19 @@ int nvme_auth_parse_key(const char *secret, struct nvme_dhchap_key **ret_key)
 {
 	struct nvme_dhchap_key *key;
 	u8 key_hash;
+	int offset = 0;
 
 	if (!secret) {
 		*ret_key = NULL;
 		return 0;
 	}
 
-	if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
+	if (sscanf(secret, "DHHC-1:%hhd:%n", &key_hash, &offset) != 1 ||
+	    !offset)
 		return -EINVAL;
 
 	/* Pass in the secret without the 'DHHC-1:XX:' prefix */
-	key = nvme_auth_extract_key(secret + 10, key_hash);
+	key = nvme_auth_extract_key(secret + offset, key_hash);
 	if (IS_ERR(key)) {
 		*ret_key = NULL;
 		return PTR_ERR(key);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] nvmet-auth: store the DH-CHAP key offset instead of hardcoding it
  2026-11-09 14:59 [PATCH 0/2] nvme: use the parsed offset for DH-CHAP secrets shao.mingyin
  2026-11-09 14:59 ` [PATCH 1/2] nvme-auth: use the parsed offset for the DH-CHAP key shao.mingyin
@ 2026-11-09 14:59 ` shao.mingyin
  1 sibling, 0 replies; 4+ messages in thread
From: shao.mingyin @ 2026-11-09 14:59 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, linux-nvme; +Cc: hare, linux-kernel, stable

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvmet_auth_set_key() accepts variable-width hash identifiers, but
nvmet_setup_auth() always skips ten bytes before extracting the key.
For "DHHC-1:1:" this starts one byte past the allocated string's NUL
terminator, causing an out-of-bounds read in nvme_auth_extract_key().
Other nonstandard prefix widths also select the wrong key bytes.

Use %n to validate the complete prefix and record its actual length
for both host and controller secrets.  Keep the offsets as int, matching
the scan result, so whitespace or leading zeroes in the hash identifier
do not truncate the saved offset.

Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest, KASAN) with host and
target DH-CHAP enabled.

Configuring the same 9-byte secret for the host entry on the target and
connecting makes nvmet_setup_auth() read past the kstrdup'ed string:

  unpatched                            patched
  KASAN slab-out-of-bounds in          malformed key rejected,
  strlen/strrchr, nvmet_setup_auth     connect fails cleanly

diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 574025730834..e0fc257ab90d 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -24,20 +24,24 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 {
 	unsigned char key_hash;
 	char *dhchap_secret;
+	int offset = 0;
 
 	if (!strlen(secret)) {
 		if (set_ctrl) {
 			kfree(host->dhchap_ctrl_secret);
 			host->dhchap_ctrl_secret = NULL;
 			host->dhchap_ctrl_key_hash = 0;
+			host->dhchap_ctrl_key_offset = 0;
 		} else {
 			kfree(host->dhchap_secret);
 			host->dhchap_secret = NULL;
 			host->dhchap_key_hash = 0;
+			host->dhchap_key_offset = 0;
 		}
 		return 0;
 	}
-	if (sscanf(secret, "DHHC-1:%hhd:%*s", &key_hash) != 1)
+	if (sscanf(secret, "DHHC-1:%hhd:%n", &key_hash, &offset) != 1 ||
+	    !offset)
 		return -EINVAL;
 	if (key_hash > 3) {
 		pr_warn("Invalid DH-HMAC-CHAP hash id %d\n",
@@ -52,10 +56,12 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 		kfree(host->dhchap_ctrl_secret);
 		host->dhchap_ctrl_secret = strim(dhchap_secret);
 		host->dhchap_ctrl_key_hash = key_hash;
+		host->dhchap_ctrl_key_offset = offset;
 	} else {
 		kfree(host->dhchap_secret);
 		host->dhchap_secret = strim(dhchap_secret);
 		host->dhchap_key_hash = key_hash;
+		host->dhchap_key_offset = offset;
 	}
 	up_write(&nvmet_config_sem);
 	return 0;
@@ -135,6 +141,7 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq, bool reset)
 	int ret = 0;
 	struct nvmet_host_link *p;
 	struct nvmet_host *host = NULL;
+	const u8 *key;
 
 	down_read(&nvmet_config_sem);
 	if (nvmet_is_disc_subsys(ctrl->subsys))
@@ -181,8 +188,8 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq, bool reset)
 
 	/* 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 = host->dhchap_secret + host->dhchap_key_offset;
+	ctrl->host_key = nvme_auth_extract_key(key, host->dhchap_key_hash);
 	if (IS_ERR(ctrl->host_key)) {
 		ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
 		ctrl->host_key = NULL;
@@ -200,8 +207,8 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq, bool reset)
 		goto out_unlock;
 	}
 
-	ctrl->ctrl_key = nvme_auth_extract_key(host->dhchap_ctrl_secret + 10,
-					       host->dhchap_ctrl_key_hash);
+	key = host->dhchap_ctrl_secret + host->dhchap_ctrl_key_offset;
+	ctrl->ctrl_key = nvme_auth_extract_key(key, host->dhchap_ctrl_key_hash);
 	if (IS_ERR(ctrl->ctrl_key)) {
 		ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
 		ctrl->ctrl_key = NULL;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index d7804bf91aaa..6a17cd9ee0a9 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -400,6 +400,8 @@ struct nvmet_host {
 	struct config_group	group;
 	u8			*dhchap_secret;
 	u8			*dhchap_ctrl_secret;
+	int			dhchap_key_offset;
+	int			dhchap_ctrl_key_offset;
 	u8			dhchap_key_hash;
 	u8			dhchap_ctrl_key_hash;
 	u8			dhchap_hash_id;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] nvme-auth: use the parsed offset for the DH-CHAP key
  2026-11-09 14:59 ` [PATCH 1/2] nvme-auth: use the parsed offset for the DH-CHAP key shao.mingyin
@ 2026-11-10  7:21   ` Hannes Reinecke
  0 siblings, 0 replies; 4+ messages in thread
From: Hannes Reinecke @ 2026-11-10  7:21 UTC (permalink / raw)
  To: shao.mingyin, kbusch, axboe, hch, sagi, linux-nvme; +Cc: linux-kernel, stable

On 11/9/26 3:59 PM, shao.mingyin@zte.com.cn wrote:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> nvme_auth_parse_key() accepts the DH-HMAC-CHAP key with
> 
> 	if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
> 		return -EINVAL;
> 
> and then skips a hardcoded ten characters:
> 
> 	key = nvme_auth_extract_key(secret + 10, key_hash);
> 
> The ten characters assume that the hash identifier is exactly two
> characters long.  A string such as "DHHC-1:1:" still makes sscanf()
> return 1, and secret + 10 then points one byte past the NUL terminator
> of the buffer.  nvme_auth_extract_key() starts with
> strlen(secret), so it reads past the end of the allocation.  The sysfs
> dhchap_secret and dhchap_ctrl_secret stores only require seven
> characters and a "DHHC-1:" prefix, so a short value reaches this code
> (even though the nvme-fabrics option parser rejects secrets shorter than
> eleven characters).
> 
> Use the %n conversion to obtain the real offset after the
> "DHHC-1:<hash>:" prefix and reject a string where the prefix is not
> complete.  This accepts both one and two character hash identifiers and
> never starts the key past the terminator.
> 
> Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> Tested on 7.3.0-next-20261030 (QEMU/TCG guest, KASAN) with a live loop
> controller and host DH-CHAP.
> 
> Writing the 9-byte secret "DHHC-1:1:" to the dhchap_secret sysfs
> attribute makes the driver start the key ten bytes in, one byte past
> the NUL terminator:
> 
>    unpatched                            patched
>    KASAN slab-out-of-bounds in          write rejected (-EINVAL)
>    strlen/strrchr, nvme_auth_extract_key
> 
> diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
> index e2e0c736540a..1fdc4e4ed0b9 100644
> --- a/drivers/nvme/common/auth.c
> +++ b/drivers/nvme/common/auth.c
> @@ -489,17 +489,19 @@ int nvme_auth_parse_key(const char *secret, struct nvme_dhchap_key **ret_key)
>   {
>   	struct nvme_dhchap_key *key;
>   	u8 key_hash;
> +	int offset = 0;
>   
>   	if (!secret) {
>   		*ret_key = NULL;
>   		return 0;
>   	}
>   
> -	if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
> +	if (sscanf(secret, "DHHC-1:%hhd:%n", &key_hash, &offset) != 1 ||
> +	    !offset)
>   		return -EINVAL;
>   
>   	/* Pass in the secret without the 'DHHC-1:XX:' prefix */
> -	key = nvme_auth_extract_key(secret + 10, key_hash);
> +	key = nvme_auth_extract_key(secret + offset, key_hash);
>   	if (IS_ERR(key)) {
>   		*ret_key = NULL;
>   		return PTR_ERR(key);

An easier fix would've been to check that the string is longer
than 10 bytes. But anyway.

Reviewed-by: Hannes Reinecke <hare@kernel.org>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-10  7:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-09 14:59 [PATCH 0/2] nvme: use the parsed offset for DH-CHAP secrets shao.mingyin
2026-11-09 14:59 ` [PATCH 1/2] nvme-auth: use the parsed offset for the DH-CHAP key shao.mingyin
2026-11-10  7:21   ` Hannes Reinecke
2026-11-09 14:59 ` [PATCH 2/2] nvmet-auth: store the DH-CHAP key offset instead of hardcoding it shao.mingyin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®