Message-ID: <aMfWz4gwFNMx7x82@MiWiFi-R3L-srv>
Date: Mon, 15 Sep 2025 17:05:19 +0800
From: Baoquan He <bhe@...hat.com>
To: Andrey Konovalov <andreyknvl@...il.com>
Cc: Andrey Ryabinin <ryabinin.a.a@...il.com>, snovitoll@...il.com,
	glider@...gle.com, dvyukov@...gle.com, elver@...gle.com,
	linux-mm@...ck.org, vincenzo.frascino@....com,
	akpm@...ux-foundation.org, kasan-dev@...glegroups.com,
	linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
	sj@...nel.org, lorenzo.stoakes@...cle.com,
	christophe.leroy@...roup.eu
Subject: Re: [PATCH v3 00/12] mm/kasan: make kasan=on|off work for all three
 modes

On 09/05/25 at 10:34pm, Andrey Konovalov wrote:
> On Fri, Sep 5, 2025 at 7:12 PM Andrey Ryabinin <ryabinin.a.a@...il.com> wrote:
> >
> > > But have you tried running kasan=off + CONFIG_KASAN_STACK=y +
> > > CONFIG_VMAP_STACK=y (+ CONFIG_KASAN_VMALLOC=y)? I would expect this
> > > should causes crashes, as the early shadow is mapped as read-only and
> > > the inline stack instrumentation will try writing into it (or do the
> > > writes into the early shadow somehow get ignored?..).
> >
> > It's not read-only, otherwise we would crash very early before full shadow
> > setup and won't be able to boot at all. So writes still happen, and shadow
> > checked, but reports are disabled.
> >
> > So the patchset should work, but it's a little bit odd feature. With kasan=off we still
> > pay x2-x3 performance penalty of compiler instrumentation and get nothing in return.
> > So the usecase for this is if you don't want to compile and manage additional kernel binary
> > (with CONFIG_KASAN=n) and don't care about performance at all.

Thanks a lot for your careful reviewing, and sorry for late reply.

About kasan=off, we use static key to detect that, wondering if we will
have x2-x3 performance penalty. Not only can kdump get the benefit, but I
can think of one case where people may use kasan enabled kernel to detect
MM issues, while use kasan=off to make sure kasan code itself won't make
trouble. E.g you tested a normal kernel and it has no problem, while
kasan enabled kernel will trigger issue, sometime do we doubt kasan
code? In this case, kasan=off can prove its inonence?

This could be trivial, while I don't see much kasan=off introducing will
impact the old kasan performance and stir the current kasan implementation
code. We have got the kasan_arch_is_ready() anyway.


> 
> Ack. So kasan=off would work but it's only benefit would be to avoid
> the RAM overhead.

Right, I built kernel with below configs on, kasan=off|on works very
well.

=====
CONFIG_KASAN=y
CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX=y
CONFIG_KASAN_GENERIC=y
CONFIG_KASAN_INLINE=y
CONFIG_KASAN_STACK=y
CONFIG_KASAN_VMALLOC=y
CONFIG_KASAN_KUNIT_TEST=m
...
CONFIG_VMAP_STACK=y
=====

> 
> Baoquan, I'd be in favor of implementing kasan.vmalloc=off instead of
> kasan=off. This seems to both (almost) solve the RAM overhead problem
> you're having (AFAIU) and also seems like a useful feature on its own
> (similar to CONFIG_KASAN_VMALLOC=n but via command-line). The patches
> to support kasan.vmalloc=off should also be orthogonal to the
> Sabyrzhan's series.
> 
> If you feel strongly that the ~1/8th RAM overhead (coming from the
> physmap shadow and the slab redzones) is still unacceptable for your
> use case (noting that the performance overhead (and the constant
> silent detection of false-positive bugs) would still be there), I
> think you can proceed with your series (unless someone else is
> against).

Yeah, that would be great if we can also avoid any not needed memory
consumption for kdump.

> 
> I also now get what you meant that with your patches for the kasan=off
> support, Sabyrzhan's CONFIG_ARCH_DEFER_KASAN would not be required
> anymore: as every architecture would need a kasan_enabled() check,
> every architecture would effectively need the CONFIG_ARCH_DEFER_KASAN
> functionality (i.e. the static key to switch off KASAN).

Exactly. In this case, the code with the static key enabling or
disabling is clearer than CONFIG_ARCH_DEFER_KASAN setting or not.

> 
> Nevertheless, I still like the unification of the static keys usage
> and the KASAN initialization calls that the Sabyrzhan's series
> introduces, so I would propose to rebase your patches on top of his
> (even though you would remove CONFIG_ARCH_DEFER_KASAN, but that seems
> like a simple change) or pick out the related parts from his patches
> (but this might not be the best approach in case someone discovers a
> reason why kasan=off is a bad idea and we need to abandon the
> kasan=off series).

Here I understand your reviewing policy. While I would like to explain a
little about my posting. I planned to do this job in 2023, made draft
patches on x86 for generic kasan, I dind't go further to try sw_tags
mode on arm64 because other things interrupted me. This year, I made
plan to disable some kernel features not necessary for kdump kernel,
mainly by adding kernel parameter like ima= I made, and later the
kasan=off.

aa9bb1b32594 ima: add a knob ima= to allow disabling IMA in kdump kernel

When I made patch and posted, I didn't see Sabyrzhan's patches because I
usually don't go through mm mailing list. If I saw his patch earlier, I
would have suggested him to solve this at the same time.

About Sabyrzhan's patch sereis, I have picked up part of his patches and
credit the author to Sabyrzhan in below patchset.

[PATCH 0/4] mm/kasan: remove kasan_arch_is_ready()
https://lore.kernel.org/all/20250812130933.71593-1-bhe@redhat.com/T/#u

About reposting of this series, do you think which one is preferred:

1) Firstly merge Sabyrzhan's patch series, I reverted them and apply for
   my patchset.

2) Credit the author of patch 1,2,3 of this patch series to Sabyrzhan
   too as below, because Sabyrzhan do the unification of the static keys
   usage and the KASAN initialization calls earlier:

[PATCH v3 01/12] mm/kasan: add conditional checks in functions to return directly if kasan is disabled
[PATCH v3 02/12] mm/kasan: move kasan= code to common place
[PATCH v3 03/12] mm/kasan/sw_tags: don't initialize kasan if it's disabled

commit ac4004af0e1e8798d11c9310e500a88116d90271
Author: Baoquan He <bhe@...hat.com>
Date:   Mon Jan 2 08:58:36 2023 +0800

    x86/kasan: check if kasan is available

commit cddd343bdbf5d0331695da8100380fc4b8b47464
Author: Baoquan He <bhe@...hat.com>
Date:   Sun Jan 1 20:57:51 2023 +0800

    mm/kasan: allow generic and sw_tags to be set in kernel cmdline
    
    Signed-off-by: Baoquan He <bhe@...hat.com>

commit b149886995ecb2e464fee0cdd3a814035fc87226
Author: Baoquan He <bhe@...hat.com>
Date:   Sun Jan 1 21:07:29 2023 +0800

    x86/kasan: allow to disable kasan during boot time

Powered by blists - more mailing lists