lkml.org 
[lkml]   [2022]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 5.19 0767/1157] kasan: fix zeroing vmalloc memory with HW_TAGS
    From
    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

    \
     
     \ /
      Last update: 2022-08-16 13:45    [W:2.638 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site