lkml.org 
[lkml]   [2022]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 3/3] kasan: fix zeroing vmalloc memory with HW_TAGS
    On Tue, May 31, 2022 at 05:43PM +0200, andrey.konovalov@linux.dev wrote:
    > From: Andrey Konovalov <andreyknvl@google.com>
    >
    > 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.
    >
    > Fixes: 23689e91fb22 ("kasan, vmalloc: add vmalloc tagging for HW_TAGS")
    > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
    > ---
    > mm/kasan/hw_tags.c | 30 ++++++++++++++++++++++--------
    > mm/vmalloc.c | 10 +++++-----
    > 2 files changed, 27 insertions(+), 13 deletions(-)
    >
    > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
    > index 9e1b6544bfa8..c0ec01eadf20 100644
    > --- a/mm/kasan/hw_tags.c
    > +++ b/mm/kasan/hw_tags.c
    > @@ -263,21 +263,31 @@ void *__kasan_unpoison_vmalloc(const void *start, unsigned long size,
    > u8 tag;
    > unsigned long redzone_start, redzone_size;
    >
    > - if (!kasan_vmalloc_enabled())
    > - return (void *)start;
    > + if (!kasan_vmalloc_enabled() || !is_vmalloc_or_module_addr(start)) {
    > + struct page *page;
    > + const void *addr;
    > +
    > + /* Initialize memory if required. */
    > +

    This whole block of code looks out-of-place in this function, since it's
    not at all related to unpoisoning but a fallback if KASAN-vmalloc is off
    but we still want to initialize the memory.

    Maybe to ease readability here I'd change it to look like:


    diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
    index 11f661a2494b..227c20d09258 100644
    --- a/mm/kasan/hw_tags.c
    +++ b/mm/kasan/hw_tags.c
    @@ -257,6 +257,21 @@ static void unpoison_vmalloc_pages(const void *addr, u8 tag)
    }
    }

    +/*
    + * Explicit initialization of pages if KASAN does not handle VM_ALLOC
    + * allocations.
    + */
    +static void init_vmalloc_pages_explicit(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);
    + }
    +}
    +
    void *__kasan_unpoison_vmalloc(const void *start, unsigned long size,
    kasan_vmalloc_flags_t flags)
    {
    @@ -264,19 +279,8 @@ void *__kasan_unpoison_vmalloc(const void *start, unsigned long size,
    unsigned long redzone_start, redzone_size;

    if (!kasan_vmalloc_enabled() || !is_vmalloc_or_module_addr(start)) {
    - struct page *page;
    - const void *addr;
    -
    - /* Initialize memory if required. */
    -
    - if (!(flags & KASAN_VMALLOC_INIT))
    - return (void *)start;
    -
    - for (addr = start; addr < start + size; addr += PAGE_SIZE) {
    - page = virt_to_page(addr);
    - clear_highpage_kasan_tagged(page);
    - }
    -
    + if (flags & KASAN_VMALLOC_INIT)
    + init_vmalloc_pages_explicit(start, size);
    return (void *)start;
    }

    \
     
     \ /
      Last update: 2022-06-01 14:30    [W:3.868 / U:0.832 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site