| Message ID | 20250206214014.3567372-3-kees@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 6270f4deba3fbd77d1717fb8634f1fc612ff69e2 |
| Headers | show |
| Series | string.h: Use ARRAY_SIZE() for memtostr*()/strtomem*() | expand |
On Thu, Feb 6, 2025 at 1:40 PM Kees Cook <kees@kernel.org> wrote: > > The destination argument of memtostr*() and strtomem*() must be a > fixed-size char array at compile time, so there is no need to use > __builtin_object_size() (which is useful for when an argument is > either a pointer or unknown). Instead use ARRAY_SIZE(), which has the > benefit of working around a bug in Clang (fixed[1] in 15+) that got > __builtin_object_size() wrong sometimes. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202501310832.kiAeOt2z-lkp@intel.com/ > Suggested-by: Kent Overstreet <kent.overstreet@linux.dev> > Link: https://github.com/llvm/llvm-project/commit/d8e0a6d5e9dd2311641f9a8a5d2bf90829951ddc [1] > Signed-off-by: Kees Cook <kees@kernel.org> Tested-by: Suren Baghdasaryan <surenb@google.com> Thanks Kees! CC'ing Andrew Morton since mm-unstable contains my workaround patch [1] for the issue fixed by this patchset. I'm not sure which tree will be taking these fixes but once they are in we should drop [1]. Or maybe we just drop it now from mm-unstable? Seems like only testbot was the only one who complained... [1] 0e91345e75b2 ("alloc_tag: work around clang-14 build issue with __builtin_object_size()") > --- > Cc: Suren Baghdasaryan <surenb@google.com> > Cc: nathan@kernel.org > Cc: Andy Shevchenko <andy@kernel.org> > Cc: linux-hardening@vger.kernel.org > --- > include/linux/string.h | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/include/linux/string.h b/include/linux/string.h > index 493ac4862c77..fc5ae145bd78 100644 > --- a/include/linux/string.h > +++ b/include/linux/string.h > @@ -411,7 +411,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > * must be discoverable by the compiler. > */ > #define strtomem_pad(dest, src, pad) do { \ > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > + const size_t _dest_len = __must_be_byte_array(dest) + \ > + ARRAY_SIZE(dest); \ > const size_t _src_len = __builtin_object_size(src, 1); \ > \ > BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ > @@ -434,7 +435,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > * must be discoverable by the compiler. > */ > #define strtomem(dest, src) do { \ > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > + const size_t _dest_len = __must_be_byte_array(dest) + \ > + ARRAY_SIZE(dest); \ > const size_t _src_len = __builtin_object_size(src, 1); \ > \ > BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ > @@ -453,7 +455,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > * Note that sizes of @dest and @src must be known at compile-time. > */ > #define memtostr(dest, src) do { \ > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > + const size_t _dest_len = __must_be_byte_array(dest) + \ > + ARRAY_SIZE(dest); \ > const size_t _src_len = __builtin_object_size(src, 1); \ > const size_t _src_chars = strnlen(src, _src_len); \ > const size_t _copy_len = min(_dest_len - 1, _src_chars); \ > @@ -478,7 +481,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > * Note that sizes of @dest and @src must be known at compile-time. > */ > #define memtostr_pad(dest, src) do { \ > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > + const size_t _dest_len = __must_be_byte_array(dest) + \ > + ARRAY_SIZE(dest); \ > const size_t _src_len = __builtin_object_size(src, 1); \ > const size_t _src_chars = strnlen(src, _src_len); \ > const size_t _copy_len = min(_dest_len - 1, _src_chars); \ > -- > 2.34.1 >
On Thu, Feb 06, 2025 at 03:59:53PM -0800, Suren Baghdasaryan wrote: > On Thu, Feb 6, 2025 at 1:40 PM Kees Cook <kees@kernel.org> wrote: > > > > The destination argument of memtostr*() and strtomem*() must be a > > fixed-size char array at compile time, so there is no need to use > > __builtin_object_size() (which is useful for when an argument is > > either a pointer or unknown). Instead use ARRAY_SIZE(), which has the > > benefit of working around a bug in Clang (fixed[1] in 15+) that got > > __builtin_object_size() wrong sometimes. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202501310832.kiAeOt2z-lkp@intel.com/ > > Suggested-by: Kent Overstreet <kent.overstreet@linux.dev> > > Link: https://github.com/llvm/llvm-project/commit/d8e0a6d5e9dd2311641f9a8a5d2bf90829951ddc [1] > > Signed-off-by: Kees Cook <kees@kernel.org> > > Tested-by: Suren Baghdasaryan <surenb@google.com> > > Thanks Kees! > CC'ing Andrew Morton since mm-unstable contains my workaround patch > [1] for the issue fixed by this patchset. I'm not sure which tree will > be taking these fixes but once they are in we should drop [1]. Or > maybe we just drop it now from mm-unstable? Seems like only testbot > was the only one who complained... > > [1] 0e91345e75b2 ("alloc_tag: work around clang-14 build issue with > __builtin_object_size()") Yeah, that should be dropped. I intend to send this fix series to Linus tomorrow or Saturday before rc2. -Kees > > > --- > > Cc: Suren Baghdasaryan <surenb@google.com> > > Cc: nathan@kernel.org > > Cc: Andy Shevchenko <andy@kernel.org> > > Cc: linux-hardening@vger.kernel.org > > --- > > include/linux/string.h | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/string.h b/include/linux/string.h > > index 493ac4862c77..fc5ae145bd78 100644 > > --- a/include/linux/string.h > > +++ b/include/linux/string.h > > @@ -411,7 +411,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > > * must be discoverable by the compiler. > > */ > > #define strtomem_pad(dest, src, pad) do { \ > > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > > + const size_t _dest_len = __must_be_byte_array(dest) + \ > > + ARRAY_SIZE(dest); \ > > const size_t _src_len = __builtin_object_size(src, 1); \ > > \ > > BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ > > @@ -434,7 +435,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > > * must be discoverable by the compiler. > > */ > > #define strtomem(dest, src) do { \ > > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > > + const size_t _dest_len = __must_be_byte_array(dest) + \ > > + ARRAY_SIZE(dest); \ > > const size_t _src_len = __builtin_object_size(src, 1); \ > > \ > > BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ > > @@ -453,7 +455,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > > * Note that sizes of @dest and @src must be known at compile-time. > > */ > > #define memtostr(dest, src) do { \ > > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > > + const size_t _dest_len = __must_be_byte_array(dest) + \ > > + ARRAY_SIZE(dest); \ > > const size_t _src_len = __builtin_object_size(src, 1); \ > > const size_t _src_chars = strnlen(src, _src_len); \ > > const size_t _copy_len = min(_dest_len - 1, _src_chars); \ > > @@ -478,7 +481,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > > * Note that sizes of @dest and @src must be known at compile-time. > > */ > > #define memtostr_pad(dest, src) do { \ > > - const size_t _dest_len = __builtin_object_size(dest, 1); \ > > + const size_t _dest_len = __must_be_byte_array(dest) + \ > > + ARRAY_SIZE(dest); \ > > const size_t _src_len = __builtin_object_size(src, 1); \ > > const size_t _src_chars = strnlen(src, _src_len); \ > > const size_t _copy_len = min(_dest_len - 1, _src_chars); \ > > -- > > 2.34.1 > >
diff --git a/include/linux/string.h b/include/linux/string.h index 493ac4862c77..fc5ae145bd78 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -411,7 +411,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, * must be discoverable by the compiler. */ #define strtomem_pad(dest, src, pad) do { \ - const size_t _dest_len = __builtin_object_size(dest, 1); \ + const size_t _dest_len = __must_be_byte_array(dest) + \ + ARRAY_SIZE(dest); \ const size_t _src_len = __builtin_object_size(src, 1); \ \ BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ @@ -434,7 +435,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, * must be discoverable by the compiler. */ #define strtomem(dest, src) do { \ - const size_t _dest_len = __builtin_object_size(dest, 1); \ + const size_t _dest_len = __must_be_byte_array(dest) + \ + ARRAY_SIZE(dest); \ const size_t _src_len = __builtin_object_size(src, 1); \ \ BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ @@ -453,7 +455,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, * Note that sizes of @dest and @src must be known at compile-time. */ #define memtostr(dest, src) do { \ - const size_t _dest_len = __builtin_object_size(dest, 1); \ + const size_t _dest_len = __must_be_byte_array(dest) + \ + ARRAY_SIZE(dest); \ const size_t _src_len = __builtin_object_size(src, 1); \ const size_t _src_chars = strnlen(src, _src_len); \ const size_t _copy_len = min(_dest_len - 1, _src_chars); \ @@ -478,7 +481,8 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, * Note that sizes of @dest and @src must be known at compile-time. */ #define memtostr_pad(dest, src) do { \ - const size_t _dest_len = __builtin_object_size(dest, 1); \ + const size_t _dest_len = __must_be_byte_array(dest) + \ + ARRAY_SIZE(dest); \ const size_t _src_len = __builtin_object_size(src, 1); \ const size_t _src_chars = strnlen(src, _src_len); \ const size_t _copy_len = min(_dest_len - 1, _src_chars); \
The destination argument of memtostr*() and strtomem*() must be a fixed-size char array at compile time, so there is no need to use __builtin_object_size() (which is useful for when an argument is either a pointer or unknown). Instead use ARRAY_SIZE(), which has the benefit of working around a bug in Clang (fixed[1] in 15+) that got __builtin_object_size() wrong sometimes. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202501310832.kiAeOt2z-lkp@intel.com/ Suggested-by: Kent Overstreet <kent.overstreet@linux.dev> Link: https://github.com/llvm/llvm-project/commit/d8e0a6d5e9dd2311641f9a8a5d2bf90829951ddc [1] Signed-off-by: Kees Cook <kees@kernel.org> --- Cc: Suren Baghdasaryan <surenb@google.com> Cc: nathan@kernel.org Cc: Andy Shevchenko <andy@kernel.org> Cc: linux-hardening@vger.kernel.org --- include/linux/string.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)