Re: [PATCH RFC 0/8] kasan: hardware tag-based mode for production use on arm64

From: Kostya Serebryany <kcc@google.com>
To: Andrey Konovalov <andreyknvl@google.com>
Cc: Serban Constantinescu <serbanc@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	 Will Deacon <will.deacon@arm.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	Alexander Potapenko <glider@google.com>,
	 Evgenii Stepanov <eugenis@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	 Elena Petrova <lenaptr@google.com>,
	Branislav Rankov <Branislav.Rankov@arm.com>,
	 Kevin Brodsky <kevin.brodsky@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 kasan-dev <kasan-dev@googlegroups.com>,
	 Linux ARM <linux-arm-kernel@lists.infradead.org>,
	 Linux Memory Management List <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	 Marco Elver <elver@google.com>
Subject: Re: [PATCH RFC 0/8] kasan: hardware tag-based mode for production use on arm64
Date: Mon, 19 Oct 2020 15:51:15 -0700	[thread overview]
Message-ID: <CAN=P9pjxptTQyvZQg7Z9XA50kFfRBc=E3iaK-KR14Fqay7Xo-Q@mail.gmail.com> (raw)
In-Reply-To: <CAAeHK+yuUJFbQBCPyp7S+hVMzBM0m=tgrWLMCskELF6SXHXimw@mail.gmail.com>

Hi,
I would like to hear opinions from others in CC on these choices:
* Production use of In-kernel MTE should be based on stripped-down
KASAN, or implemented independently?
* Should we aim at a single boot-time flag (with several values) or
for several independent flags (OFF/SYNC/ASYNC, Stack traces on/off)

Andrey, please give us some idea of the CPU and RAM overheads other
than those coming from MTE
* stack trace collection and storage
* adding redzones to every allocation - not strictly needed for MTE,
but convenient to store the stack trace IDs.

Andrey: with production MTE we should not be using quarantine, which
means storing the stack trace IDs
in the deallocated memory doesn't provide good report quality.
We may need to consider another approach, e.g. the one used in HWASAN
(separate ring buffer, per thread or per core)

--kcc

On Fri, Oct 16, 2020 at 8:52 AM Andrey Konovalov <andreyknvl@google.com> wrote:
>
> On Fri, Oct 16, 2020 at 3:31 PM Marco Elver <elver@google.com> wrote:
> >
> > On Fri, 16 Oct 2020 at 15:17, 'Andrey Konovalov' via kasan-dev
> > <kasan-dev@googlegroups.com> wrote:
> > [...]
> > > > > The intention with this kind of a high level switch is to hide the
> > > > > implementation details. Arguably, we could add multiple switches that allow
> > > > > to separately control each KASAN or MTE feature, but I'm not sure there's
> > > > > much value in that.
> > > > >
> > > > > Does this make sense? Any preference regarding the name of the parameter
> > > > > and its values?
> > > >
> > > > KASAN itself used to be a debugging tool only. So introducing an "on"
> > > > mode which no longer follows this convention may be confusing.
> > >
> > > Yeah, perhaps "on" is not the best name here.
> > >
> > > > Instead, maybe the following might be less confusing:
> > > >
> > > > "full" - current "debug", normal KASAN, all debugging help available.
> > > > "opt" - current "on", optimized mode for production.
> > >
> > > How about "prod" here?
> >
> > SGTM.
> >
> > [...]
> > >
> > > > > Should we somehow control whether to panic the kernel on a tag fault?
> > > > > Another boot time parameter perhaps?
> > > >
> > > > It already respects panic_on_warn, correct?
> > >
> > > Yes, but Android is unlikely to enable panic_on_warn as they have
> > > warnings happening all over. AFAIR Pixel 3/4 kernels actually have a
> > > custom patch that enables kernel panic for KASAN crashes specifically
> > > (even though they don't obviously use KASAN in production), and I
> > > think it's better to provide a similar facility upstream. Maybe call
> > > it panic_on_kasan or something?
> >
> > Best would be if kasan= can take another option, e.g.
> > "kasan=prod,panic". I think you can change the strcmp() to a
> > str_has_prefix() for the checks for full/prod/on/off, and then check
> > if what comes after it is ",panic".
> >
> > Thanks,
> > -- Marco
>
> CC Kostya and Serban.