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

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

[Thorsten Blum]

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@linux.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);
 	return true;
 }
 
-- 
2.48.1

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

[Herbert Xu]

On Sun, Mar 16, 2025 at 10:15:04PM +0100, Thorsten Blum 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@linux.dev>
> ---
>  crypto/essiv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[David Laight]

On Sun, 16 Mar 2025 22:15:04 +0100
Thorsten Blum <thorsten.blum@linux.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@linux.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;
>  }
>  

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

[Thorsten Blum]

Hi David,

On 23. Mar 2025, at 11:20, David Laight wrote:
> On Sun, 16 Mar 2025 22:15:04 +0100 Thorsten Blum 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@linux.dev>
>> ---
> 
> The 'len' argument to strscpy() is supposed to be the length of the
> buffer (in order to avoid overflow) not the number of characters.

Not necessarily, see linux/string.h:

/**
 * strscpy - Copy a C-string into a sized buffer
 * ...
 * The size argument @... is only required when @dst is not an array, or
 * when the copy needs to be smaller than sizeof(@dst).
 * ...
 */

> 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.

Yes, this is simply a refactoring, there's nothing wrong with memcpy()
followed by a manual NUL-termination.

However, strscpy() is shorter and semantically better imo because we're
copying C-strings and not just raw bytes. strscpy() also has additional
compile-time checks regarding C-strings that memcpy() doesn't.

Thanks,
Thorsten