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]

Date:   Fri, 2 Nov 2018 12:44:01 +0300
From:   Andrey Ryabinin <aryabinin@...tuozzo.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     dan.carpenter@...cle.com, adilger.kernel@...ger.ca,
        yamada.masahiro@...ionext.com, michal.lkml@...kovi.net,
        rostedt@...dmis.org, mchehab+samsung@...nel.org, olof@...m.net,
        Konstantin Ryabitsev <konstantin@...uxfoundation.org>,
        David Miller <davem@...emloft.net>,
        Kees Cook <keescook@...omium.org>, tglx@...utronix.de,
        Ingo Molnar <mingo@...nel.org>, paullawrence@...gle.com,
        sandipan@...ux.vnet.ibm.com, andreyknvl@...gle.com,
        dwmw2@...radead.org, will.deacon@....com, pombredanne@...b.com,
        paul.burton@...s.com, rientjes@...gle.com, w@....eu,
        msebor@...il.com, sparse@...isli.org,
        Jonathan Corbet <corbet@....net>,
        Theodore Ts'o <tytso@....edu>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>, joe@...ches.com,
        Arnd Bergmann <arnd@...db.de>, asmadeus@...ewreck.org,
        stefan@...er.ch, luc.vanoostenryck@...il.com,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        linux-doc@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-sparse@...r.kernel.org, linux-kbuild@...r.kernel.org,
        Vasily Gorbik <gor@...ux.ibm.com>
Subject: Re: [GIT PULL] Compiler Attributes for v4.20-rc1

On 11/01/2018 08:06 PM, Linus Torvalds wrote:
> On Mon, Oct 22, 2018 at 3:59 AM Miguel Ojeda
> <miguel.ojeda.sandonis@...il.com> wrote:
>>
>> Here it is the Compiler Attributes series/tree, which tries to disentangle
>> the include/linux/compiler*.h headers and bring them up to date.
> 
> I've finally emptied the "normal" pull queues, and am looking at the
> odd ones that I felt I needed to review more in-depth.
> 
> This looked fine to me, and I already pulled, but when looking at the
> conflict for __no_sanitize_address_or_inline, I also noticed that you
> actually changed the gcc version logic.
> 
> The logic for using __no_sanitize_address *used* to be
> 
>     #if GCC_VERSION >= 40902
> 
> but you have changed it to
> 
>   # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> 
> so now it's gcc-4.8+ rather than gcc-4.9.2+.

As you said in other email - "this has less to do with the attribute
being recognized, and simply just being because KASAN itself wants
gcc-4.9.2."

gcc < 4.9.2 simply doesn't have -fsanitize=kernel-address.
But no_sanitize attribute originally comes with user space ASAN (-fsanitize=address)
which exist in earlier GCCs like 4.8.
Checking against 4.8 gcc should be fine. If we compile the kernel with gcc 4.8
it will be compiled without instrumentation, so the attribute won't have any effect.

> 
> 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.
> 
> 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'm _inclined_ to just do
> 
> #ifdef CONFIG_KASAN
>  #define __no_sanitize_address_or_inline \
>   __no_sanitize_address __maybe_unused notrace
> #else
>  #define __no_sanitize_address_or_inline inline
> #endif
> 
> without any compiler versions at all, because I don't think it matters
> (maybe we won't get address sanitation, and we will not get inlining
> either, but do we care?).

You're right, version checks shouldn't matter here. But __no_sanitize_address_or_inline
shouldn't have been added in the first place, because we already have almost the same __no_kasan_or_inline:

#ifdef CONFIG_KASAN
/*
 * We can't declare function 'inline' because __no_sanitize_address confilcts
 * with inlining. Attempt to inline it may cause a build failure.
 * 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
 * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
 */
# define __no_kasan_or_inline __no_sanitize_address __maybe_unused
#else
# define __no_kasan_or_inline __always_inline
#endif

There are small differences like absence of notrace, but notrace could be added
to the function together with __no_kasan_or_inline.
And inline is *not* redefined to __always_inline  only on x86 when CONFIG_OPTIMIZE_INLINING=y

So there is absolutely no reason to have both __no_kasan_or_inline and __no_sanitize_address_or_inline.