linux-kernel - Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

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:   Tue, 5 May 2020 17:19:58 +0200
From:   Marco Elver <elver@...gle.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Dmitry Vyukov <dvyukov@...gle.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Kees Cook <keescook@...omium.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Thomas Gleixner <tglx@...utronix.de>,
        kasan-dev <kasan-dev@...glegroups.com>,
        LKML <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

On Tue, 5 May 2020 at 16:59, Arnd Bergmann <arnd@...db.de> wrote:
>
> On Tue, May 5, 2020 at 4:50 PM 'Dmitry Vyukov' via Clang Built Linux
> <clang-built-linux@...glegroups.com> wrote:
> > On Tue, May 5, 2020 at 4:36 PM Marco Elver <elver@...gle.com> wrote:
> > > > Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
> > > > with -fsanitize=bounds or with ubsan:
> > > >
> > > > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
> > > > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
> > > >
> > > >  menuconfig KCSAN
> > > >         bool "KCSAN: dynamic data race detector"
> > > > -       depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN
> > > > +       depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
> > >
> > > This also disables KCOV with GCC. Why does this not work with KCSAN?
>
> My mistake, this should be kept enabled for gcc. If we can get the combination
> to work in clang, that's something that should also get enabled.

See my suggestion below how we might dynamically determine if the
combination is supported.

> > > This is a huge problem for us, since syzbot requires KCOV. In fact
> > > I've always been building KCSAN kernels with CONFIG_KCOV=y (with GCC
> > > or Clang) and cannot reproduce the problem.
>
> I have some local patches that change the way we pick the warning options
> for each compiler, and enable more of the warnings that are normally disabled.
>
> Maybe -Wunused-command-line-argument is disabled by default?
> I only started seeing this problem recently. It's also possible that there
> are some other options that interact with it so only Kcov+FOO leads to
> KCSAN being ignored.

I see. It certainly seems quite bad if one or the other option is
effectively ignored.

> > > Ditto, we really need KCOV for all sanitizers. I also just tried to
> > > reproduce the problem but can't.
> > >
> > > Which version of clang is causing this? I'm currently using Clang 9.
> > > My guess is that we should not fix this by disallowing KCOV, but
> > > rather make Clang work with these configs.
> > >
> > > Dmitry, can you comment?
> >
> > FWIW I can reproduce both with clang:
> >
> > $ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds
> > clang-11: warning: argument unused during compilation:
> > '-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]
> >
> > $ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
> > clang-11: warning: argument unused during compilation:
> > '-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]
> >
> > with both my disto's 9.0.1 and fresher 11.0.0
> > (7b80cb7cf45faf462d6193cc41c2cb7ad556600d.
> >
> > But both work with gcc
> >
> > $ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
> > $ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds
> >
> > Is it a known issue in clang?
> >
> > Can we somehow disable it only for clang and not gcc?
> >
> > This will immediately break KCSAN on syzbot as it enables KCSAN and KCOV:
> > https://syzkaller.appspot.com/upstream?manager=ci2-upstream-kcsan-gce
>
> I can respin the patch with this fixup if you like:
>
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -5,7 +5,7 @@ config HAVE_ARCH_KCSAN
>
>  menuconfig KCSAN
>         bool "KCSAN: dynamic data race detector"
> -       depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
> +       depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !(KCOV
> && CC_IS_CLANG)

I wonder if we can just add this:  depends on !(KCOV &&
!$(cc-option,-Werror -fsanitize=thread -fsanitize-coverage=trace-pc))

Similarly for UBSAN.

That way, once Clang supports this combination, we don't need another
patch to fix it.

Thanks,
-- Marco

>         select STACKTRACE
>         help
>           The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
>
> As you both say, the combination seems to be quite important, so maybe there
> is something else that can be to also enable it with clang.
>
>       Arnd