linux-kernel - 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 12:25:42 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Peter Rosin <peda@...ntia.se>,
        Andy Shevchenko <andy@...nel.org>,
        Matteo Croce <mcroce@...rosoft.com>,
        Nathan Chancellor <nathan@...nel.org>,
        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()

On Thu, Feb 03, 2022 at 11:50:34AM -0800, Nick Desaulniers wrote:
> On Thu, Feb 3, 2022 at 10:10 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> >
> > Hi Kees,
> >
> > On Thu, Feb 3, 2022 at 6:15 PM Kees Cook <keescook@...omium.org> wrote:
> > > On Thu, Feb 03, 2022 at 09:04:22AM +0100, Geert Uytterhoeven wrote:
> > > > 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.
> > >
> > > Hah, for i386:
> > >
> > > arch/x86/Makefile
> > >         # temporary until string.h is fixed
> > >         KBUILD_CFLAGS += -ffreestanding
> > >
> > > This "temporary" is from 2006. ;)\
> 
> IIRC I sent a patch removing that. Yeah, Kees even signed off on it.
> https://lore.kernel.org/lkml/20200817220212.338670-5-ndesaulniers@google.com/#t
> I still think that's the right way to go, perhaps worth a resend to
> rekick discussions.

Hah. Yay. I'll go pick this into the memcpy topic branch.  Building x86_64
and ia32 differently makes no sense (and this solves the head-scratching
compile-time test failures I was seeing on ia32 too).

> >
> > And before that, we had it in the main Makefile.
> >
> > > 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to builtin")
> > >
> > > Removing that appears to solve it, and appears to build correctly. I'll
> > > continue testing.
> > >
> > > > It works after dropping -ffreestanding.
> > >
> > > I wonder if the other architectures were just copying x86?
> >
> > At least on m68k it was added because gcc added new optimizations
> > that broke if an architecture provides some functions as inline.
> > As the kernel is not supposed to be linked with the standard C
> > library, -ffreestanding should be correct, isn't it?
> 
> The kernel does not link against a libc; but it does provide many
> symbols that libc would provide, with the same or similar enough
> semantics that I would strongly recommend we _don't_ use
> -ffreestanding in order to get such libcall optimizations (there are a
> lot; see https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
> for some examples) and simply use -fno-builtin-* when necessary, or
> fix the kernel implementations individually.

Right, so, I think for x86 it's straight forward. For the other
architectures it may need more careful checking, so I'm just going
to drop the new test from the memcpy topic branch, and maybe start a
"-ffreestanding removal" topic branch so there isn't a cross-dependency
here.

-- 
Kees Cook