On 8/6/25 4:15 PM, Sabyrzhan Tasbolatov wrote:
> On Wed, Aug 6, 2025 at 6:35 PM Andrey Ryabinin <ryabinin....@gmail.com> 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 <snovit...@gmail.com>
>>> ---
>>> 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=aphfonbay3nqtfhzvc...@mail.gmail.com/
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()'.
>From the top of my head, I can't say if we have such cases.
>
> Ok, we will have the **only** check kasan_enabled() then in
> kasan-enabled.h which
>
> #if defined(CONFIG_ARCH_DEFER_KASAN) || defined(CONFIG_KASAN_HW_TAGS)
> static __always_inline bool kasan_enabled(void)
> {
> return static_branch_likely(&kasan_flag_enabled);
> }
> #else
> static inline bool kasan_enabled(void)
> {
> return IS_ENABLED(CONFIG_KASAN);
> }
>
> And will remove kasan_arch_is_ready (current kasan_shadow_initialized in v4).
>
> So it is the single place to check if KASAN is enabled for all arch
> and internal KASAN code.
> Same behavior is in the current mainline code but only for HW_TAGS.
>
> Is this correct?
>
Yep, that's what I meant.
>>
>>
>>> {
>>> return static_branch_likely(&kasan_flag_enabled);
>>> }
>>>
>>> -static inline bool kasan_hw_tags_enabled(void)
>>> +static inline void kasan_enable(void)
>>> +{
>>> + static_branch_enable(&kasan_flag_enabled);
>>> +}
>>> +#else
>>> +/* For architectures that can enable KASAN early, use compile-time check.
>>> */
>>> +static __always_inline bool kasan_shadow_initialized(void)
>>> {
>>> return kasan_enabled();
>>> }
>>>
>>
>> ...
>>
>>>
>>> void kasan_populate_early_vm_area_shadow(void *start, unsigned long size);
>>> -int kasan_populate_vmalloc(unsigned long addr, unsigned long size);
>>> -void kasan_release_vmalloc(unsigned long start, unsigned long end,
>>> +
>>> +int __kasan_populate_vmalloc(unsigned long addr, unsigned long size);
>>> +static inline int kasan_populate_vmalloc(unsigned long addr, unsigned long
>>> size)
>>> +{
>>> + if (!kasan_shadow_initialized())
>>> + return 0;
>>
>>
>> What's the point of moving these checks to header?
>> Leave it in C, it's easier to grep and navigate code this way.
>
> Andrey Konovalov had comments [1] to avoid checks in C
> by moving them to headers under __wrappers.
>
> : 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).
>
> [1]
> https://lore.kernel.org/all/CA+fCnZcGyTECP15VMSPh+duLmxNe=aphfonbay3nqtfhzvc...@mail.gmail.com/
>
I think Andrey K. meant cases when we have multiple implementations of one
function for each mode.
In such case it makes sense to merge multiple kasan_arch_is_ready() checks into
one in the header.
But in case like with kasan_populate_vmalloc() we have only one implementation
so I don't see any
value in adding wrapper/moving to header.
>>
>>
>>> + return __kasan_populate_vmalloc(addr, size);
>>> +}
>>> +
>>> +void __kasan_release_vmalloc(unsigned long start, unsigned long end,
>>> unsigned long free_region_start,
>>> unsigned long free_region_end,
>>> unsigned long flags);
>>> +static inline void kasan_release_vmalloc(unsigned long start,
>>> + unsigned long end,
>>> + unsigned long free_region_start,
>>> + unsigned long free_region_end,
>>> + unsigned long flags)
>>> +{
>>> + if (kasan_shadow_initialized())
>>> + __kasan_release_vmalloc(start, end, free_region_start,
>>> + free_region_end, flags);
>>> +}
>>>
>>
>> ...> @@ -250,7 +259,7 @@ static inline void poison_slab_object(struct
>> kmem_cache *cache, void *object,
>>> bool __kasan_slab_pre_free(struct kmem_cache *cache, void *object,
>>> unsigned long ip)
>>> {
>>> - if (!kasan_arch_is_ready() || is_kfence_address(object))
>>> + if (is_kfence_address(object))
>>> return false;
>>> return check_slab_allocation(cache, object, ip);
>>> }
>>> @@ -258,7 +267,7 @@ bool __kasan_slab_pre_free(struct kmem_cache *cache,
>>> void *object,
>>> bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init,
>>> bool still_accessible)
>>> {
>>> - if (!kasan_arch_is_ready() || is_kfence_address(object))
>>> + if (is_kfence_address(object))
>>> return false;
>>>
>>> poison_slab_object(cache, object, init, still_accessible);
>>> @@ -282,9 +291,6 @@ bool __kasan_slab_free(struct kmem_cache *cache, void
>>> *object, bool init,
>>>
>>> static inline bool check_page_allocation(void *ptr, unsigned long ip)
>>> {
>>> - if (!kasan_arch_is_ready())
>>> - return false;
>>> -
>>
>>
>> Well, you can't do this yet, because no arch using ARCH_DEFER_KASAN yet, so
>> this breaks
>> bisectability.
>> Leave it, and remove with separate patch only when there are no users left.
>
> Will do in v5 at the end of patch series.
>
>>
>>> if (ptr != page_address(virt_to_head_page(ptr))) {
>>> kasan_report_invalid_free(ptr, ip, KASAN_REPORT_INVALID_FREE);
>>> return true;
>>> @@ -511,7 +517,7 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned
>>> long ip)
>>> return true;
>>> }
>>>
>>> - if (is_kfence_address(ptr) || !kasan_arch_is_ready())
>>> + if (is_kfence_address(ptr))
>>> return true;
>>>
>>> slab = folio_slab(folio);
>>
>>
