On 16. 08. 22, 13:04, Jiri Slaby wrote:
> On 15. 08. 22, 20:02, Greg Kroah-Hartman wrote:
>> From: Andrey Konovalov <andreyknvl@google.com>
>>
>> [ Upstream commit 6c2f761dad7851d8088b91063ccaea3c970efe78 ]
>>
>> HW_TAGS KASAN skips zeroing page_alloc allocations backing vmalloc
>> mappings via __GFP_SKIP_ZERO. Instead, these pages are zeroed via
>> kasan_unpoison_vmalloc() by passing the KASAN_VMALLOC_INIT flag.
>>
>> The problem is that __kasan_unpoison_vmalloc() does not zero pages when
>> either kasan_vmalloc_enabled() or is_vmalloc_or_module_addr() fail.
>>
>> Thus:
>>
>> 1. Change __vmalloc_node_range() to only set KASAN_VMALLOC_INIT when
>> __GFP_SKIP_ZERO is set.
>>
>> 2. Change __kasan_unpoison_vmalloc() to always zero pages when the
>> KASAN_VMALLOC_INIT flag is set.
>>
>> 3. Add WARN_ON() asserts to check that KASAN_VMALLOC_INIT cannot be set
>> in other early return paths of __kasan_unpoison_vmalloc().
>>
>> Also clean up the comment in __kasan_unpoison_vmalloc.
>>
>> Link:
>> https://lkml.kernel.org/r/4bc503537efdc539ffc3f461c1b70162eea31cf6.1654798516.git.andreyknvl@google.com
>> Fixes: 23689e91fb22 ("kasan, vmalloc: add vmalloc tagging for HW_TAGS")
>> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>> Cc: Marco Elver <elver@google.com>
>> Cc: Alexander Potapenko <glider@google.com>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>> mm/kasan/hw_tags.c | 32 +++++++++++++++++++++++---------
>> mm/vmalloc.c | 10 +++++-----
>> 2 files changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
>> index 9e1b6544bfa8..9ad8eff71b28 100644
>> --- a/mm/kasan/hw_tags.c
>> +++ b/mm/kasan/hw_tags.c
>> @@ -257,27 +257,37 @@ static void unpoison_vmalloc_pages(const void
>> *addr, u8 tag)
>> }
>> }
>> +static void init_vmalloc_pages(const void *start, unsigned long size)
>> +{
>> + const void *addr;
>> +
>> + for (addr = start; addr < start + size; addr += PAGE_SIZE) {
>> + struct page *page = virt_to_page(addr);
>> +
>> + clear_highpage_kasan_tagged(page);
>
> This breaks build on aarch64:
>> mm/kasan/hw_tags.c: In function 'init_vmalloc_pages':
>> mm/kasan/hw_tags.c:267:17: error: implicit declaration of function
>> 'clear_highpage_kasan_tagged' [-Werror=implicit-function-declaration]
Which translates into: this is missing:
commit d9da8f6cf55eeca642c021912af1890002464c64
Author: Andrey Konovalov <andreyknvl@gmail.com>
Date: Thu Jun 9 20:18:46 2022 +0200
mm: introduce clear_highpage_kasan_tagged
>> + }
>> +}
>> +
>> void *__kasan_unpoison_vmalloc(const void *start, unsigned long size,
>> kasan_vmalloc_flags_t flags)
>> {
>> u8 tag;
>> unsigned long redzone_start, redzone_size;
>> - if (!kasan_vmalloc_enabled())
>> - return (void *)start;
>> -
>> - if (!is_vmalloc_or_module_addr(start))
>> + if (!kasan_vmalloc_enabled() || !is_vmalloc_or_module_addr(start)) {
>> + if (flags & KASAN_VMALLOC_INIT)
>> + init_vmalloc_pages(start, size);
>> return (void *)start;
>> + }
>> /*
>> - * Skip unpoisoning and assigning a pointer tag for non-VM_ALLOC
>> - * mappings as:
>> + * Don't tag non-VM_ALLOC mappings, as:
>> *
>> * 1. Unlike the software KASAN modes, hardware tag-based KASAN
>> only
>> * supports tagging physical memory. Therefore, it can only
>> tag a
>> * single mapping of normal physical pages.
>> * 2. Hardware tag-based KASAN can only tag memory mapped with
>> special
>> - * mapping protection bits, see arch_vmalloc_pgprot_modify().
>> + * mapping protection bits, see arch_vmap_pgprot_tagged().
>> * As non-VM_ALLOC mappings can be mapped outside of vmalloc
>> code,
>> * providing these bits would require tracking all non-VM_ALLOC
>> * mappers.
>> @@ -289,15 +299,19 @@ void *__kasan_unpoison_vmalloc(const void
>> *start, unsigned long size,
>> *
>> * For non-VM_ALLOC allocations, page_alloc memory is tagged as
>> usual.
>> */
>> - if (!(flags & KASAN_VMALLOC_VM_ALLOC))
>> + if (!(flags & KASAN_VMALLOC_VM_ALLOC)) {
>> + WARN_ON(flags & KASAN_VMALLOC_INIT);
>> return (void *)start;
>> + }
>> /*
>> * Don't tag executable memory.
>> * The kernel doesn't tolerate having the PC register tagged.
>> */
>> - if (!(flags & KASAN_VMALLOC_PROT_NORMAL))
>> + if (!(flags & KASAN_VMALLOC_PROT_NORMAL)) {
>> + WARN_ON(flags & KASAN_VMALLOC_INIT);
>> return (void *)start;
>> + }
>> tag = kasan_random_tag();
>> start = set_tag(start, tag);
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index effd1ff6a4b4..a1ab9b472571 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -3168,15 +3168,15 @@ void *__vmalloc_node_range(unsigned long size,
>> unsigned long align,
>> /*
>> * Mark the pages as accessible, now that they are mapped.
>> - * The init condition should match the one in post_alloc_hook()
>> - * (except for the should_skip_init() check) to make sure that
>> memory
>> - * is initialized under the same conditions regardless of the
>> enabled
>> - * KASAN mode.
>> + * The condition for setting KASAN_VMALLOC_INIT should complement
>> the
>> + * one in post_alloc_hook() with regards to the __GFP_SKIP_ZERO
>> check
>> + * to make sure that memory is initialized under the same
>> conditions.
>> * Tag-based KASAN modes only assign tags to normal non-executable
>> * allocations, see __kasan_unpoison_vmalloc().
>> */
>> kasan_flags |= KASAN_VMALLOC_VM_ALLOC;
>> - if (!want_init_on_free() && want_init_on_alloc(gfp_mask))
>> + if (!want_init_on_free() && want_init_on_alloc(gfp_mask) &&
>> + (gfp_mask & __GFP_SKIP_ZERO))
>> kasan_flags |= KASAN_VMALLOC_INIT;
>> /* KASAN_VMALLOC_PROT_NORMAL already set if required. */
>> area->addr = kasan_unpoison_vmalloc(area->addr, real_size,
>> kasan_flags);
>
--
js
suse labs