linux-hardening - Re: [PATCH] lib/test_string.c: Add test for strlen()

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:   Thu, 3 Feb 2022 09:04:22 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Kees Cook <keescook@...omium.org>, Peter Rosin <peda@...ntia.se>,
        Andy Shevchenko <andy@...nel.org>,
        Matteo Croce <mcroce@...rosoft.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        llvm@...ts.linux.dev, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] lib/test_string.c: Add test for strlen()

Hi Günter,

On Thu, Feb 3, 2022 at 12:12 AM Guenter Roeck <linux@...ck-us.net> wrote:
> On 2/2/22 12:52, Kees Cook wrote:
> > On Wed, Feb 02, 2022 at 08:01:49AM -0800, Guenter Roeck wrote:
> >> On Sun, Jan 30, 2022 at 10:36:53AM -0800, Kees Cook wrote:
> >>> Add a simple test for strlen() functionality, including using it as a
> >>> constant expression.
> >>>
> >>> Cc: Geert Uytterhoeven <geert@...ux-m68k.org>
> >>> Cc: Peter Rosin <peda@...ntia.se>
> >>> Signed-off-by: Kees Cook <keescook@...omium.org>
> >>> ---
> >>> I'll be taking this as part of my Clang FORTIFY_SOURCE series.
> >>> ---
> >>>   lib/test_string.c | 37 +++++++++++++++++++++++++++++++++++++
> >>>   1 file changed, 37 insertions(+)
> >>>
> >>> diff --git a/lib/test_string.c b/lib/test_string.c
> >>> index 9dfd6f52de92..59994f552c48 100644
> >>> --- a/lib/test_string.c
> >>> +++ b/lib/test_string.c
> >>> @@ -179,6 +179,38 @@ static __init int strnchr_selftest(void)
> >>>     return 0;
> >>>   }
> >>>
> >>> +/*
> >>> + * Unlike many other string functions, strlen() can be used in
> >>> + * static initializers when string lengths are known at compile
> >>> + * time. (i.e. Under these conditions, strlen() is a constant
> >>> + * expression.) Make sure it can be used this way.
> >>> + */
> >>> +static const int strlen_ce = strlen("tada, a constant expression");
> >>> +
> >>
> >> This results in:
> >>
> >> lib/test_string.c:188:30: error: initializer element is not constant
> >>    188 | static const int strlen_ce = strlen("tada, a constant expression");
> >>
> >> for several of my tests. I don't think you can mandate that a compiler
> >> implements this.
> >
> > Which tests?
> >
>
> Some examples:
>
> Build reference: next-20220202
> Compiler version: m68k-linux-gcc (GCC) 11.2.0
>
> Building m68k:defconfig ... failed
> --------------
> Error log:
> lib/test_string.c:188:30: error: initializer element is not constant
>    188 | static const int strlen_ce = strlen("tada, a constant expression");
>
> Building mips:malta_defconfig:nocd:smp:net,e1000:initrd ... failed
> ------------
> Error log:
> lib/test_string.c:188:30: error: initializer element is not constant
>   static const int strlen_ce = strlen("tada, a constant expression");
>
> Building i386:q35:Broadwell:defconfig:smp:ata:net,rtl8139:hd ... failed
> ------------
> Error log:
> lib/test_string.c:188:30: error: initializer element is not constant
>    188 | static const int strlen_ce = strlen("tada, a constant expression");
>
> i386 and is defconfig + CONFIG_STRING_SELFTEST=y; mips is
> malta_defconfig + CONFIG_STRING_SELFTEST=y. All use gcc 11.2.
>
> There may be more, but there are so many failures in -next right now
> that I may be missing some.
>
> > This property of strlen() is already required by our builds (this is how
> > I tripped over it). For example:
> >
> > drivers/firmware/xilinx/zynqmp-debug.c:
> >
> > #define PM_API(id)               {id, #id, strlen(#id)}
> > static struct pm_api_info pm_api_list[] = {
> >          PM_API(PM_GET_API_VERSION),
> >          PM_API(PM_QUERY_DATA),
> > };
>
> I do not think that it is a C standard that strlen() on a constant string
> must be compile-time evaluated and result in a constant.

Not if -ffreestanding, which is what several architectures are
using nowadays, to a.o. prevent gcc from replacing calls to stdlib
functions to other stdlib functions (e.g. strncat() -> strlen() +
store, strncmp() -> strcmp()), which breaks linking if the latter is
only provided inline.

> Anyway, key difference, I think, is the presence of an architecture-specific
> version of strlen(), or the maybe non-presence of __HAVE_ARCH_STRLEN,
> or the definition of strlen() in include/linux/fortify-string.h.

It works after dropping -ffreestanding.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds