linux-kernel - Re: [m68k:master 1174/1174] arch/m68k/include/asm/string.h:72:25: warning: '__builtin_memcpy' forming offset 8 is out of the bounds [0, 7]

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Mar 2019 09:31:50 +1100 (AEDT)
From:   Finn Thain <fthain@...egraphics.com.au>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
cc:     Andreas Schwab <schwab@...ux-m68k.org>,
        kbuild test robot <lkp@...el.com>, kbuild-all@...org,
        linux-m68k <linux-m68k@...ts.linux-m68k.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [m68k:master 1174/1174] arch/m68k/include/asm/string.h:72:25:
 warning: '__builtin_memcpy' forming offset 8 is out of the bounds [0, 7]

On Mon, 11 Mar 2019, Geert Uytterhoeven wrote:

> On Mon, Mar 11, 2019 at 11:13 AM Andreas Schwab <schwab@...ux-m68k.org> wrote:
> > On M?r 11 2019, Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > > On Mon, Mar 11, 2019 at 10:56 AM Andreas Schwab <schwab@...ux-m68k.org> wrote:
> > >> On M?r 11 2019, Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > >> > On Thu, Mar 7, 2019 at 10:42 PM Finn Thain <fthain@...egraphics.com.au> wrote:
> > >> >> No, the link fails because the compiler still emits some references to
> > >> >> strlen().
> > >> >
> > >> > Despite -ffreestanding?!?
> > >>
> > >> *Because* of -ffreestanding.  Without that, strlen would be recognized
> > >> and turned into __builtin_strlen.
> > >
> > > Now I'm confused: if we have a static inline or #define for strlen(),
> >
> > Do you?
> 
> I don't, but Finn's patch has, IINM.
> 

You're mixing up two separate patches there. One uses a #define and the 
other uses a forced inline function. We were discussing the former patch 
when I answered your question about __HAVE_ARCH_STRLEN (which got 
snipped).

m68k doesn't define __HAVE_ARCH_STRLEN and relies on the strlen() 
implementation in lib/string.c. The former patch doesn't alter this but 
reduces the number of callers because some call sites get optimized away. 
That's how it avoids the warning you raised.

Anyway, I don't like pre-processor kludges. So I did another experiment 
with the latter (forced inline) approach, to see if some optimizations can 
still be used with -ffreestanding.

diff --git a/include/linux/string.h b/include/linux/string.h
index 7927b875f80c..25b5bf689018 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -436,6 +436,58 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
 	return p;
 }
 
+#else
+
+//__FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
+//{
+//	return __builtin_strncpy(p, q, size);
+//}
+
+__FORTIFY_INLINE char *strcat(char *p, const char *q)
+{
+	return __builtin_strcat(p, q);
+}
+
+__FORTIFY_INLINE __kernel_size_t strlen(const char *p)
+{
+	return __builtin_strlen(p);
+}
+
+__FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
+{
+	return __builtin_strncat(p, q, count);
+}
+
+__FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size)
+{
+	return __builtin_memset(p, c, size);
+}
+
+//__FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
+//{
+//	return __builtin_memcpy(p, q, size);
+//}
+
+__FORTIFY_INLINE void *memmove(void *p, const void *q, __kernel_size_t size)
+{
+	return __builtin_memmove(p, q, size);
+}
+
+__FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
+{
+	return __builtin_memcmp(p, q, size);
+}
+
+__FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
+{
+	return __builtin_memchr(p, c, size);
+}
+
+__FORTIFY_INLINE char *strcpy(char *p, const char *q)
+{
+	return __builtin_strcpy(p, q);
+}
+
 #endif
 
 /**


The result of this patch really is confusing. It still suppresses the 
warning you raised:

arch/m68k/include/asm/string.h:72:25: warning: '__builtin_memcpy' forming
offset 8 is out of the bounds [0, 7] [-Warray-bounds]
    #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
                            ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/string.h:456:3: note: in expansion of macro 'memcpy'
      memcpy(dest, src, dest_len);
      ^~~~~~

But it also causes new ones, because of __builtin_memset():

drivers/video/fbdev/core/fbcvt.c: In function 'fb_find_mode_cvt':
drivers/video/fbdev/core/fbcvt.c:312:16: warning: 'cvt.flags' may be used uninit                                                                     ialized in this function [-Wmaybe-uninitialized]
      cvt.flags |= FB_CVT_FLAG_MARGINS;
                ^~

Apparently the compiler doesn't understand that __builtin_memset() has 
the effect of initialization. Weird.

-- 

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
>