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

[Mark Rutland <mark.rutland@arm.com>]

On Wed, Dec 01, 2021 at 07:16:25PM +0100, Marco Elver wrote:
> 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?

If it's not used by Syzbot, that's good enough for me -- I can apply local
hacks to run with KCOV if I want to in the mean time, and I can debug my own
mess if I have to.

So FWIW, for taking that now:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.