Re: [PATCH] kcov: fix generic Kconfig dependencies if ARCH_WANTS_NO_INSTR

From: Marco Elver <elver@google.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	kasan-dev@googlegroups.com, Peter Zijlstra <peterz@infradead.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] kcov: fix generic Kconfig dependencies if ARCH_WANTS_NO_INSTR
Date: Wed, 1 Dec 2021 19:16:25 +0100	[thread overview]
Message-ID: <CANpmjNMW_BFnVj2Eaai76PQZqOoABLw+oYm8iGy6Vp9r_ru_iQ@mail.gmail.com> (raw)
In-Reply-To: <Yae08MUQn5SxPwZ/@FVFF77S0Q05N>

On Wed, 1 Dec 2021 at 18:46, Mark Rutland <mark.rutland@arm.com> wrote:
[...]
> > > Currently we mostly get away with disabling KCOV for while compilation units,
> > > so maybe it's worth waiting for the GCC 12.0 release, and restricting things
> > > once that's out?
> >
> > An alternative would be to express 'select ARCH_WANTS_NO_INSTR' more
> > precisely, say with an override or something. Because as-is,
> > ARCH_WANTS_NO_INSTR then doesn't quite reflect reality on arm64
> > (yet?).
>
> It's more of a pragmatic thing -- ARCH_WANTS_NO_INSTR does reflect reality, and
> we do *want* to enforce that strictly, it's just that we're just struck between
> a rock and a hard place where until GCC 12 is released we either:
>
> a) Strictly enforce noinstr, and be sure there aren't any bugs from unexpected
>    instrumentation, but we can't test GCC-built kernels under Syzkaller due to
>    the lack of KCOV.
>
> b) Don't strictly enforce noinstr, and have the same latent bugs as today (of
>    unknown severity), but we can test GCC-built kernels under Syzkaller.
>
> ... and since this (currently only affects KCOV, which people only practically
> enable for Syzkaller, I think it's ok to wait until GCC 12 is out, so that we
> can have the benefit of Sykaller in the mean time, and subsequrntly got for
> option (a) and say those people need to use GCC 12+ (and clang 13+).
>
> > But it does look simpler to wait, so I'm fine with that. I leave it to you.
>
> FWIW, for my purposes I'm happy to take this immediately and to have to apply a
> local patch to my fuzzing branches until GCC 12 is out, but I assume we'd want
> the upstream testing to work in the mean time without requiring additional
> patches.

Agree, it's not an ideal situation. :-/

syzkaller would still work, just not as efficiently. Not sure what's
worse, less efficient fuzzing, or chance of random crashes. In fact,
on syzbot we already had to disable it:
https://github.com/google/syzkaller/blob/61f862782082c777ba335aa4b4b08d4f74d7d86e/dashboard/config/linux/bits/base.yml#L110
https://lore.kernel.org/linux-arm-kernel/20210119130010.GA2338@C02TD0UTHF1T.local/T/#m78fdfcc41ae831f91c93ad5dabe63f7ccfb482f0

So if we ran into issues with KCOV on syzbot for arm64, I'm sure it's
not just us. I can't quite see what the reasons for the crashes are,
but ruling out noinstr vs. KCOV would be a first step.

So I'm inclined to suggest we take this patch now and not wait for GCC
12, given we're already crashing with KCOV and therefore have KCOV
disabled on arm64 syzbot.

I'm still fine waiting, but just wanted to point out you can fuzz
without KCOV. Preferences?

Thanks,
-- Marco