Re: [PATCH 1/2] nvme-auth: use the parsed offset for the DH-CHAP key

[Hannes Reinecke <hare@suse.de>]

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