Similar to strscpy(), update strscpy_pad()'s 3rd argument to be
optional when the destination is a compile-time known size array.
Cc: Andy Shevchenko <andy@kernel.org>
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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.1Hi,
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@google.com>
>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> 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
On Wed, Feb 07, 2024 at 12:51:51AM +0000, Justin Stitt wrote: > 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? I viewed it as a distinct logical change. I could certainly combine them, but I think it's easier to review the conversion from function to macro without needing to consider anything else. No behavioral changes are expected, etc. But if they were together, there's a little more cognitive load to keep the func/macro conversion in mind while looking at the optional arg magic, etc. I don't think it's a strict rule or anything; it just felt like the right thing to do to split them up. > At any rate, > Reviewed-by: Justin Stitt <justinstitt@google.com> Thanks! -Kees -- Kees Cook
From: Kees Cook > Sent: 07 February 2024 09:19 > > On Wed, Feb 07, 2024 at 12:51:51AM +0000, Justin Stitt wrote: > > 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? > > I viewed it as a distinct logical change. I could certainly combine > them, but I think it's easier to review the conversion from function to > macro without needing to consider anything else. No behavioral changes > are expected, etc. I wonder about the code-bloat from inlining strscpy_pad()? Especially given the code that gcc is likely to generate for string ops. I strongly suspect that the end of strscpy() knows exactly you many bytes weren't written (in the non-truncate path). So maybe implement both strscpy() and strscp_pad() in terms of an inline function that has a parameter that 'turns on' padding. That way you get a simple call site and still only one implementation. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
© 2016 - 2025 Red Hat, Inc.