[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <af677847-e625-43d7-8750-b2ce4ba9626c@csgroup.eu>
Date: Fri, 8 Aug 2025 19:03:54 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Sabyrzhan Tasbolatov <snovitoll@...il.com>
Cc: ryabinin.a.a@...il.com, bhe@...hat.com, hca@...ux.ibm.com,
andreyknvl@...il.com, akpm@...ux-foundation.org, zhangqing@...ngson.cn,
chenhuacai@...ngson.cn, davidgow@...gle.co, glider@...gle.com,
dvyukov@...gle.com, alex@...ti.fr, agordeev@...ux.ibm.com,
vincenzo.frascino@....com, elver@...gle.com, kasan-dev@...glegroups.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
loongarch@...ts.linux.dev, linuxppc-dev@...ts.ozlabs.org,
linux-riscv@...ts.infradead.org, linux-s390@...r.kernel.org,
linux-um@...ts.infradead.org, linux-mm@...ck.org
Subject: Re: [PATCH v5 1/2] kasan: introduce ARCH_DEFER_KASAN and unify static
key across modes
Le 08/08/2025 à 17:33, Sabyrzhan Tasbolatov a écrit :
> On Fri, Aug 8, 2025 at 10:03 AM Christophe Leroy
> <christophe.leroy@...roup.eu> wrote:
>>
>>
>>
>> Le 07/08/2025 à 21:40, Sabyrzhan Tasbolatov a écrit :
>>> Introduce CONFIG_ARCH_DEFER_KASAN to identify architectures [1] that need
>>> to defer KASAN initialization until shadow memory is properly set up,
>>> and unify the static key infrastructure across all KASAN modes.
>>
>> That probably desserves more details, maybe copy in informations from
>> the top of cover letter.
>>
>> I think there should also be some exeplanations about
>> kasan_arch_is_ready() becoming kasan_enabled(), and also why
>> kasan_arch_is_ready() completely disappear from mm/kasan/common.c
>> without being replaced by kasan_enabled().
>>
>>>
>>> [1] PowerPC, UML, LoongArch selects ARCH_DEFER_KASAN.
>>>
>>> Closes: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D217049&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cfe4f5a759ad6452b047408ddd691024a%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638902640503259176%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=UM4uvQihJdeWwcC6DIiJXbn4wGsrijjRcHc55uCMErI%3D&reserved=0
>>> Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@...il.com>
>>> ---
>>> Changes in v5:
>>> - Unified patches where arch (powerpc, UML, loongarch) selects
>>> ARCH_DEFER_KASAN in the first patch not to break
>>> bisectability
>>> - Removed kasan_arch_is_ready completely as there is no user
>>> - Removed __wrappers in v4, left only those where it's necessary
>>> due to different implementations
>>>
>>> Changes in v4:
>>> - Fixed HW_TAGS static key functionality (was broken in v3)
>>> - Merged configuration and implementation for atomicity
>>> ---
>>> arch/loongarch/Kconfig | 1 +
>>> arch/loongarch/include/asm/kasan.h | 7 ------
>>> arch/loongarch/mm/kasan_init.c | 8 +++----
>>> arch/powerpc/Kconfig | 1 +
>>> arch/powerpc/include/asm/kasan.h | 12 ----------
>>> arch/powerpc/mm/kasan/init_32.c | 2 +-
>>> arch/powerpc/mm/kasan/init_book3e_64.c | 2 +-
>>> arch/powerpc/mm/kasan/init_book3s_64.c | 6 +----
>>> arch/um/Kconfig | 1 +
>>> arch/um/include/asm/kasan.h | 5 ++--
>>> arch/um/kernel/mem.c | 10 ++++++--
>>> include/linux/kasan-enabled.h | 32 ++++++++++++++++++--------
>>> include/linux/kasan.h | 6 +++++
>>> lib/Kconfig.kasan | 8 +++++++
>>> mm/kasan/common.c | 17 ++++++++++----
>>> mm/kasan/generic.c | 19 +++++++++++----
>>> mm/kasan/hw_tags.c | 9 +-------
>>> mm/kasan/kasan.h | 8 ++++++-
>>> mm/kasan/shadow.c | 12 +++++-----
>>> mm/kasan/sw_tags.c | 1 +
>>> mm/kasan/tags.c | 2 +-
>>> 21 files changed, 100 insertions(+), 69 deletions(-)
>>>
>>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>>> index f0abc38c40a..cd64b2bc12d 100644
>>> --- a/arch/loongarch/Kconfig
>>> +++ b/arch/loongarch/Kconfig
>>> @@ -9,6 +9,7 @@ config LOONGARCH
>>> select ACPI_PPTT if ACPI
>>> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
>>> select ARCH_BINFMT_ELF_STATE
>>> + select ARCH_DEFER_KASAN if KASAN
>>
>> Instead of adding 'if KASAN' in all users, you could do in two steps:
>>
>> Add a symbol ARCH_NEEDS_DEFER_KASAN.
>>
>> +config ARCH_NEEDS_DEFER_KASAN
>> + bool
>>
>> And then:
>>
>> +config ARCH_DEFER_KASAN
>> + def_bool
>> + depends on KASAN
>> + depends on ARCH_DEFER_KASAN
>> + help
>> + Architectures should select this if they need to defer KASAN
>> + initialization until shadow memory is properly set up. This
>> + enables runtime control via static keys. Otherwise, KASAN uses
>> + compile-time constants for better performance.
>>
>
> Actually, I don't see the benefits from this option. Sorry, have just
> revisited this again.
> With the new symbol, arch (PowerPC, UML, LoongArch) still needs select
> 2 options:
>
> select ARCH_NEEDS_DEFER_KASAN
> select ARCH_DEFER_KASAN
Sorry, my mistake, ARCH_DEFER_KASAN has to be 'def_bool y'. Missing the
'y'. That way it is automatically set to 'y' as long as KASAN and
ARCH_NEEDS_DEFER_KASAN are selected. Should be:
config ARCH_DEFER_KASAN
def_bool y
depends on KASAN
depends on ARCH_NEEDS_DEFER_KASAN
>
> and the oneline with `if` condition is cleaner.
> select ARCH_DEFER_KASAN if KASAN
>
I don't think so because it requires all architectures to add 'if KASAN'
which is not convenient.
Christophe
Powered by blists - more mailing lists