On 8/6/25 4:15 PM, Sabyrzhan Tasbolatov wrote: > On Wed, Aug 6, 2025 at 6:35 PM Andrey Ryabinin <ryabinin.a.a@xxxxxxxxx> wrote: >> >> >> >> On 8/5/25 4:26 PM, Sabyrzhan Tasbolatov wrote: >>> Introduce CONFIG_ARCH_DEFER_KASAN to identify architectures that need >>> to defer KASAN initialization until shadow memory is properly set up, >>> and unify the static key infrastructure across all KASAN modes. >>> >>> Some architectures (like PowerPC with radix MMU) need to set up their >>> shadow memory mappings before KASAN can be safely enabled, while others >>> (like s390, x86, arm) can enable KASAN much earlier or even from the >>> beginning. >>> >>> Historically, the runtime static key kasan_flag_enabled existed only for >>> CONFIG_KASAN_HW_TAGS mode. Generic and SW_TAGS modes either relied on >>> architecture-specific kasan_arch_is_ready() implementations or evaluated >>> KASAN checks unconditionally, leading to code duplication. >>> >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217049 >>> Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@xxxxxxxxx> >>> --- >>> Changes in v4: >>> - Fixed HW_TAGS static key functionality (was broken in v3) >> >> I don't think it fixed. Before you patch kasan_enabled() esentially >> worked like this: >> >> if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) >> return static_branch_likely(&kasan_flag_enabled); >> else >> return IS_ENABLED(CONFIG_KASAN); >> >> Now it's just IS_ENABLED(CONFIG_KASAN); > > In v4 it is: > > #if defined(CONFIG_ARCH_DEFER_KASAN) || defined(CONFIG_KASAN_HW_TAGS) > static __always_inline bool kasan_shadow_initialized(void) > { > return static_branch_likely(&kasan_flag_enabled); > } > #else > static __always_inline bool kasan_shadow_initialized(void) > { > return kasan_enabled(); // which is IS_ENABLED(CONFIG_KASAN); > } > #endif > > So for HW_TAGS, KASAN is enabled in kasan_init_hw_tags(). You are referring to kasan_shadow_initialized(), but I was talking about kasan_enabled() specifically. E.g. your patch changes behavior for kasan_init_slab_obj() which doesn't use kasan_shadow_initialized() (in the case of HW_TAGS=y && kasan_flag_enabled = false) : static __always_inline void * __must_check kasan_init_slab_obj( struct kmem_cache *cache, const void *object) { if (kasan_enabled()) return __kasan_init_slab_obj(cache, object); return (void *)object; } >>> +#if defined(CONFIG_ARCH_DEFER_KASAN) || defined(CONFIG_KASAN_HW_TAGS) >>> +/* >>> + * Global runtime flag for KASAN modes that need runtime control. >>> + * Used by ARCH_DEFER_KASAN architectures and HW_TAGS mode. >>> + */ >>> DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled); >>> >>> -static __always_inline bool kasan_enabled(void) >>> +/* >>> + * Runtime control for shadow memory initialization or HW_TAGS mode. >>> + * Uses static key for architectures that need deferred KASAN or HW_TAGS. >>> + */ >>> +static __always_inline bool kasan_shadow_initialized(void) >> >> Don't rename it, just leave as is - kasan_enabled(). >> It's better name, shorter and you don't need to convert call sites, so >> there is less chance of mistakes due to unchanged kasan_enabled() -> kasan_shadow_initialized(). > > I actually had the only check "kasan_enabled()" in v2, but went to > double check approach in v3 > after this comment: > https://lore.kernel.org/all/CA+fCnZcGyTECP15VMSPh+duLmxNe=ApHfOnbAY3NqtFHZvceZw@xxxxxxxxxxxxxx/ AFAIU the comment suggest that we need two checks/flags, one in kasan_enabled() which checks whether kasan was enabled via cmdline (currently only for HW_TAGS) and one in kasan_arch_is_ready()(or kasan_shadow_initialized()) which checks if arch initialized KASAN. And this not what v3/v4 does. v4 basically have one check, just under different name. Separate checks might be needed if we have code paths that need 'kasan_arch_is_ready() && !kasan_enabled()' and vise versa '!kasan_arch_is_ready() && kasan_enabled()'.