[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9b1474ea-8568-cdb5-72dc-51d577497f8a@kernel.org>
Date: Tue, 16 Aug 2022 13:04:00 +0200
From: Jiri Slaby <jirislaby@...nel.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org
Cc: stable@...r.kernel.org, Andrey Konovalov <andreyknvl@...gle.com>,
Marco Elver <elver@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Andrey Ryabinin <ryabinin.a.a@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 5.19 0767/1157] kasan: fix zeroing vmalloc memory with
HW_TAGS
On 15. 08. 22, 20:02, Greg Kroah-Hartman wrote:
> From: Andrey Konovalov <andreyknvl@...gle.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@...gle.com>
> Cc: Marco Elver <elver@...gle.com>
> Cc: Alexander Potapenko <glider@...gle.com>
> Cc: Dmitry Vyukov <dvyukov@...gle.com>
> Cc: Andrey Ryabinin <ryabinin.a.a@...il.com>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Sasha Levin <sashal@...nel.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]
> + }
> +}
> +
> 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
Powered by blists - more mailing lists