Re: [PATCH v3 3/4] string: Allow 2-argument strscpy_pad()

From: Justin Stitt
Date: Tue Feb 06 2024 - 19:52:21 EST


Hi,

On Tue, Feb 06, 2024 at 06:22:18AM -0800, Kees Cook wrote:
> Similar to strscpy(), update strscpy_pad()'s 3rd argument to be
> optional when the destination is a compile-time known size array.

This patch is diff'd against Patch 1/4 in this series, right? I wonder
why you split them up. If I hadn't literally just read that patch I
would be mildly confused.

I suppose one reason may be that 1/4 is a standalone change with a high
percentage chance of landing whilst this overloading magic may not land
as easily?

At any rate,
Reviewed-by: Justin Stitt <justinstitt@xxxxxxxxxx>

>
> Cc: Andy Shevchenko <andy@xxxxxxxxxx>
> Cc: linux-hardening@xxxxxxxxxxxxxxx
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
> include/linux/string.h | 29 ++++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 79b875de615e..9bd421ad92a4 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -79,6 +79,10 @@ ssize_t sized_strscpy(char *, const char *, size_t);
> sized_strscpy(dst, src, sizeof(dst) + __must_be_array(dst))
> #define __strscpy1(dst, src, size) sized_strscpy(dst, src, size)
>
> +#define __strscpy_pad0(dst, src, ...) \
> + sized_strscpy_pad(dst, src, sizeof(dst) + __must_be_array(dst))
> +#define __strscpy_pad1(dst, src, size) sized_strscpy_pad(dst, src, size)
> +
> /**
> * strscpy - Copy a C-string into a sized buffer
> * @dst: Where to copy the string to
> @@ -104,6 +108,18 @@ ssize_t sized_strscpy(char *, const char *, size_t);
> #define strscpy(dst, src, ...) \
> CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src, __VA_ARGS__)
>
> +#define sized_strscpy_pad(dest, src, count) ({ \
> + char *__dst = (dest); \
> + const char *__src = (src); \
> + const size_t __count = (count); \
> + ssize_t __wrote; \
> + \
> + __wrote = sized_strscpy(__dst, __src, __count); \
> + if (__wrote >= 0 && __wrote < __count) \
> + memset(__dst + __wrote + 1, 0, __count - __wrote - 1); \
> + __wrote; \
> +})
> +
> /**
> * strscpy_pad() - Copy a C-string into a sized buffer
> * @dest: Where to copy the string to
> @@ -124,17 +140,8 @@ ssize_t sized_strscpy(char *, const char *, size_t);
> * * The number of characters copied (not including the trailing %NULs)
> * * -E2BIG if count is 0 or @src was truncated.
> */
> -#define strscpy_pad(dest, src, count) ({ \
> - char *__dst = (dest); \
> - const char *__src = (src); \
> - const size_t __count = (count); \
> - ssize_t __wrote; \
> - \
> - __wrote = strscpy(__dst, __src, __count); \
> - if (__wrote >= 0 && __wrote < __count) \
> - memset(__dst + __wrote + 1, 0, __count - __wrote - 1); \
> - __wrote; \
> -})
> +#define strscpy_pad(dst, src, ...) \
> + CONCATENATE(__strscpy_pad, COUNT_ARGS(__VA_ARGS__))(dst, src, __VA_ARGS__)
>
> #ifndef __HAVE_ARCH_STRCAT
> extern char * strcat(char *, const char *);
> --
> 2.34.1
>

Thanks
Justin