Re: [PATCH v2 00/11] kasan: unify kasan_arch_is_ready with kasan_enabled

[Heiko Carstens <hca@linux.ibm.com>]

> > > Another thing that needs careful consideration is whether it's
> > > possible to combine kasan_arch_is_ready() and kasan_enabled() into the
> > > same check logically at all. There's one issue mentioned in [1]:
> >
> > Hello,
> > I've removed kasan_arch_is_ready() at all in this series:
> > [PATCH v2 11/11] kasan: replace kasan_arch_is_ready with kasan_enabled
> >
> > Is it not what's expected by unification?
> 
> I guess the issue description diverged a bit from what needs to be
> done, sorry about that.
> 
> The core 2 things I wanted to address with the unification are:
> 
> 1. Avoid spraying kasan_arch_is_ready() throughout the KASAN
> implementation and move these checks into include/linux/kasan.h (and
> add __wrappers when required).
> 
> 2. Avoid architectures redefining the same kasan_enabled global
> variable/static key.
> 
> Initially, I thought that s/kasan_arch_is_ready/kasan_enabled + simply
> moving the calls into affected include/linux/kasan.h functions would
> be enough. But then, based on [1], turns out it's not that simple.
> 
> So now, I think we likely still need two separate checks/flags:
> kasan_enabled() that controls whether KASAN is enabled at all and
> kasan_arch_is_ready() that gets turned on by kasan_init() when shadow
> is initialized (should we rename it to kasan_shadow_initialized()?).
> But then we can still move kasan_arch_is_ready() into
> include/linux/kasan.h and use the proper combination of checks for
> each affected function before calling __wrappers. And we can still
> remove the duplicated flags/keys code from the arch code.

FWIW, as Alexander Gordeev already mentioned: this series breaks s390,
since the static_branch_enable() call in kasan_init_generic() is now
called way too early, and it isn't necessary at all. Which, as far as
I understand, may be the case for other architectures as well. s390
sets up the required KASAN mappings in the decompressor and can start
with KASAN enabled nearly from the beginning.

So something like below on top of this series would address
that. Given that this series is about to be reworked this is just for
illustration :)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 0c16dc443e2f..c2f51ac39a91 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -172,6 +172,7 @@ config S390
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN
+	select HAVE_ARCH_KASAN_EARLY
 	select HAVE_ARCH_KASAN_VMALLOC
 	select HAVE_ARCH_KCSAN
 	select HAVE_ARCH_KMSAN
diff --git a/include/linux/kasan-enabled.h b/include/linux/kasan-enabled.h
index 2436eb45cfee..049270a2269f 100644
--- a/include/linux/kasan-enabled.h
+++ b/include/linux/kasan-enabled.h
@@ -10,7 +10,11 @@
  * Global runtime flag. Starts ‘false’; switched to ‘true’ by
  * the appropriate kasan_init_*() once KASAN is fully initialized.
  */
+#ifdef CONFIG_HAVE_ARCH_KASAN_EARLY
+DECLARE_STATIC_KEY_TRUE(kasan_flag_enabled);
+#else
 DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
+#endif
 
 static __always_inline bool kasan_enabled(void)
 {
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index f82889a830fa..1407374e83b9 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -4,6 +4,13 @@
 config HAVE_ARCH_KASAN
 	bool
 
+config HAVE_ARCH_KASAN_EARLY
+	bool
+	help
+	  Architectures should select this if KASAN mappings are setup in
+	  the decompressor and when the kernel can run very early with
+	  KASAN enabled.
+
 config HAVE_ARCH_KASAN_SW_TAGS
 	bool
 
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 0f3648335a6b..2aae0ce659b4 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -36,7 +36,11 @@
  * Definition of the unified static key declared in kasan-enabled.h.
  * This provides consistent runtime enable/disable across all KASAN modes.
  */
+#ifdef CONFIG_HAVE_ARCH_KASAN_EARLY
+DEFINE_STATIC_KEY_TRUE(kasan_flag_enabled);
+#else
 DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
+#endif
 EXPORT_SYMBOL(kasan_flag_enabled);
 
 struct slab *kasan_addr_to_slab(const void *addr)
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index a3b112868be7..455376d5f1c3 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -42,7 +42,8 @@
  */
 void __init kasan_init_generic(void)
 {
-	static_branch_enable(&kasan_flag_enabled);
+	if (!IS_ENABLED(CONFIG_HAVE_ARCH_KASAN_EARLY))
+		static_branch_enable(&kasan_flag_enabled);
 
 	pr_info("KernelAddressSanitizer initialized (generic)\n");
 }