linux-kernel - Re: [PATCH v2 1/2] bpf: don't rely on GCC __attribute_

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] [day] [month] [year] [list]

Message-ID: <CAMj1kXGzrTU7-x1vcNotxy-W=buSk=3OUX=WNvwZy59SGTRAxA@mail.gmail.com>
Date:   Fri, 30 Oct 2020 08:51:27 +0100
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Nick Desaulniers <ndesaulniers@...gle.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:BPF JIT for MIPS (32-BIT AND 64-BIT)" 
        <netdev@...r.kernel.org>,
        "open list:BPF JIT for MIPS (32-BIT AND 64-BIT)" 
        <bpf@...r.kernel.org>, Arnd Bergmann <arnd@...db.de>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        Randy Dunlap <rdunlap@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Peter Zijlstra <peterz@...radead.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v2 1/2] bpf: don't rely on GCC __attribute__((optimize))
 to disable GCSE

On Fri, 30 Oct 2020 at 04:22, Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Thu, Oct 29, 2020 at 05:28:11PM -0700, Nick Desaulniers wrote:
> >
> > We already know that -fno-asynchronous-unwind-tables get dropped,
> > hence this patch.
>
> On arm64 only. Not on x86
>
> > And we know -fomit-frame-pointer or
> > -fno-omit-frame-pointer I guess gets dropped, hence your ask.
>
> yep. that one is bugged.
>
> > We might not know the full extent which other flags get dropped with the
> > optimize attribute, but I'd argue that my list above can all result in
> > pretty bad bugs when accidentally omitted (ok, maybe not -fshort-wchar
> > or -fmacro-prefix-map, idk what those do) or when mixed with code that
>
> true.
> Few month back I've checked that strict-aliasing and no-common flags
> from your list are not dropped by this attr in gcc [6789].
> I've also checked that no-red-zone and model=kernel preserved as well.
>
> > has different values those flags control.  Searching GCC's bug tracker
> > for `__attribute__((optimize` turns up plenty of reports to make me
> > think this attribute maybe doesn't work the way folks suspect or
> > intend: https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__attribute__%28%28optimize&list_id=283390.
>
> There is a risk.
> Is it a footgun? Sure.
> Yet. gcc testsuite is using __attribute__((optimize)).
> And some of these tests were added _after_ offical gcc doc said that this
> attribute is broken.
> imo it's like 'beware of the dog' sign.
>
> > There's plenty of folks arguing against the use of the optimize
> > attribute in favor of the command line flag.  I urge you to please
> > reconsider the request.
>
> ok. Applied this first patch to bpf tree and will get it to Linus soon.
> Second patch that is splitting interpreter out because of this mess
> is dropped. The effect of gcse on performance is questionable.
> iirc some interpreters used to do -fno-gcse to gain performance.

That is absolutely fine. I only included the second patch to address
Daniel's concern that -fno-gcse could affect unrelated code living in
the same source file as __bpf_prog_run(), but if you don't care about
that, nor will I.