linux-ext4 - Re: [GIT PULL] Compiler Attributes for v4.20-rc1

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]

Message-ID: <CANiq72=Cz8XiYc=FGMwrNBFq2uiO55JBmoNc+NSDXYr_awosKw@mail.gmail.com>
Date:   Thu, 1 Nov 2018 21:15:12 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Dan <dan.carpenter@...cle.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Michal Marek <michal.lkml@...kovi.net>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        Olof Johansson <olof@...m.net>,
        Konstantin Ryabitsev <konstantin@...uxfoundation.org>,
        David Miller <davem@...emloft.net>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Paul Lawrence <paullawrence@...gle.com>,
        Sandipan Das <sandipan@...ux.vnet.ibm.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Will Deacon <will.deacon@....com>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Paul Burton <paul.burton@...s.com>,
        David Rientjes <rientjes@...gle.com>, Willy Tarreau <w@....eu>,
        Martin Sebor <msebor@...il.com>,
        Christopher Li <sparse@...isli.org>,
        Jonathan Corbet <corbet@....net>, "Ted Ts'o" <tytso@....edu>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Joe Perches <joe@...ches.com>, Arnd Bergmann <arnd@...db.de>,
        Dominique Martinet <asmadeus@...ewreck.org>,
        Stefan Agner <stefan@...er.ch>,
        Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        linux-sparse@...r.kernel.org, linux-kbuild@...r.kernel.org,
        gor@...ux.ibm.com, schwidefsky@...ibm.com
Subject: Re: [GIT PULL] Compiler Attributes for v4.20-rc1

On Thu, Nov 1, 2018 at 6:06 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> I'm not sure how much that matters (maybe the original check for 4.9.2
> was just a random pick by Andrey? Added to cc), but together with the
> movement to <linux/compiler_attributes.h> that looks like it also
> wouldn't want the CONFIG_KASAN tests, I wonder what the right merge
> resolution would be.

Good catch. I don't recall any special logic when I did that change,
so most likely I simply did like for the rest of the attributes and
took a look at when it was first supported (and documentation in gcc's
docs) in order to implement __has_attribute by hand.

But indeed, it *may* be that there is an (undocumented) problem
between 4.8 <= gcc < 4.9.2 with it. If so, we should document it down
and fix it. Andrey?

> Yes, I see the resolution in linux-next, and I think that one is odd
> and dubious, and now it *mixes* that old test of gcc-4.9.2 with the
> different test in compiler-attributes.

I missed that conflict completely, my bad (I did not miss all of them,
at least; one required fixing).

Hm.... at a quick look, why is it only on compiler-gcc.h? It should
either have a corresponding #define elsewhere or just be put directly
in another common header, no? (Adding Vasily & Martin to CC.)

> But I'm also unsure whether you meant for the "__has_attribute()" test
> to be usable outside the linux/compiler_attributes.h file, in which
> case I could just do
>
>   #if defined(CONFIG_KASAN) && __has_attribute(__no_sanitize_address__)
>
> instead.

I think that (using __has_attribute() outside) may be a good idea: I
wanted to keep compiler_attributes.h as simple as possible by avoiding
#ifdefs inside that header (except for __has_attribute itself), as an
attempt to avoid going back to the mess of #ifdefs we had previously.
Basically, keeping the attributes in compiler_attributes.h that do not
depend on complex logic. So using __has_attribute *outside* the header
actually goes well with that principle, because it helps keeping stuff
out of it if they depend on other config options; without having to
rely on GCC_VERSION either.

[By the way, in case it clarifies: note that "optional" in that file
actually is a bit of a misnomer. I meant to say "optional" as in "not
supported by all compilers, so conditionally defined" ("optionally"
defined); rather than "optional" in the sense of "code still works
without the attribute". It caught Rasmus in one of his patches sent a
few days ago on top of this tree, so I want to change it or explain it
to avoid confusion.]

Cheers,
Miguel