linux-kernel - Re: [PATCH] crypto: essiv - Replace memcpy() + NUL-termination with strscpy()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250323102057.79c810f1@pumpkin>
Date: Sun, 23 Mar 2025 10:20:57 +0000
From: David Laight <david.laight.linux@...il.com>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: Herbert Xu <herbert@...dor.apana.org.au>, "David S. Miller"
 <davem@...emloft.net>, linux-crypto@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] crypto: essiv - Replace memcpy() + NUL-termination with
 strscpy()

On Sun, 16 Mar 2025 22:15:04 +0100
Thorsten Blum <thorsten.blum@...ux.dev> wrote:

> Use strscpy() to copy the NUL-terminated string 'p' to the destination
> buffer instead of using memcpy() followed by a manual NUL-termination.
> 
> No functional changes intended.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.dev>
> ---
>  crypto/essiv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/crypto/essiv.c b/crypto/essiv.c
> index 1c00c3324058..ec0ec8992c2d 100644
> --- a/crypto/essiv.c
> +++ b/crypto/essiv.c
> @@ -405,8 +405,7 @@ static bool parse_cipher_name(char *essiv_cipher_name, const char *cra_name)
>  	if (len >= CRYPTO_MAX_ALG_NAME)
>  		return false;
>  
> -	memcpy(essiv_cipher_name, p, len);
> -	essiv_cipher_name[len] = '\0';
> +	strscpy(essiv_cipher_name, p, len + 1);

That is just 'so wrong'.
The 'len' argument to strscpy() is supposed to be the length of the
buffer (in order to avoid overflow) not the number of characters.

In this case the bound check is before the copy (and the buffer assumed
to be the right size!)
So memcpy() + terminate is exactly correct.

The warning gcc emits for strncpy() when the length depends on the source
string is equally applicable to strscpy().

	David

>  	return true;
>  }
>