| Message ID | 20250316211504.39327-2-thorsten.blum@linux.dev (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: essiv - Replace memcpy() + NUL-termination with strscpy() | expand |
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.
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; > } >
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
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; }
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(-)