linux-hardening - Re: [PATCH v2 3/3] string.h: Use ARRAY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <202502061701.187F080@keescook>
Date: Thu, 6 Feb 2025 17:01:55 -0800
From: Kees Cook <kees@...nel.org>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: kernel test robot <lkp@...el.com>,
	Kent Overstreet <kent.overstreet@...ux.dev>, nathan@...nel.org,
	Andy Shevchenko <andy@...nel.org>, linux-hardening@...r.kernel.org,
	Miguel Ojeda <ojeda@...nel.org>,
	Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
	Nick Desaulniers <ndesaulniers@...gle.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>,
	Philipp Reisner <philipp.reisner@...bit.com>,
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 3/3] string.h: Use ARRAY_SIZE() for
 memtostr*()/strtomem*()

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@...nel.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@...el.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202501310832.kiAeOt2z-lkp@intel.com/
> > Suggested-by: Kent Overstreet <kent.overstreet@...ux.dev>
> > Link: https://github.com/llvm/llvm-project/commit/d8e0a6d5e9dd2311641f9a8a5d2bf90829951ddc [1]
> > Signed-off-by: Kees Cook <kees@...nel.org>
> 
> Tested-by: Suren Baghdasaryan <surenb@...gle.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@...gle.com>
> > Cc: nathan@...nel.org
> > Cc: Andy Shevchenko <andy@...nel.org>
> > Cc: linux-hardening@...r.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
> >

-- 
Kees Cook