[PATCH 1/3] mm: rename kernel_init_free_pages to kernel_init_pages

[PATCH 1/3] mm: rename kernel_init_free_pages to kernel_init_pages

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Rename kernel_init_free_pages() to kernel_init_pages(). This function is
not only used for free pages but also for pages that were just allocated.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/page_alloc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e008a3df0485..66ef8c310dce 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1296,7 +1296,7 @@ static inline bool should_skip_kasan_poison(struct page *page, fpi_t fpi_flags)
 	       PageSkipKASanPoison(page);
 }
 
-static void kernel_init_free_pages(struct page *page, int numpages)
+static void kernel_init_pages(struct page *page, int numpages)
 {
 	int i;
 
@@ -1396,7 +1396,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
 			init = false;
 	}
 	if (init)
-		kernel_init_free_pages(page, 1 << order);
+		kernel_init_pages(page, 1 << order);
 
 	/*
 	 * arch_free_page() can make the page's contents inaccessible.  s390
@@ -2441,7 +2441,7 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
 	}
 	/* If memory is still not initialized, do it now. */
 	if (init)
-		kernel_init_free_pages(page, 1 << order);
+		kernel_init_pages(page, 1 << order);
 	/* Propagate __GFP_SKIP_KASAN_POISON to page flags. */
 	if (kasan_hw_tags_enabled() && (gfp_flags & __GFP_SKIP_KASAN_POISON))
 		SetPageSkipKASanPoison(page);
-- 
2.25.1

[PATCH 2/3] mm: introduce clear_highpage_tagged

[andrey.konovalov]

From: Andrey Konovalov <andreyknvl@google.com>

Add a clear_highpage_tagged() helper that does clear_highpage() on a
page potentially tagged by KASAN.

This helper is used by the following patch.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 include/linux/highmem.h | 11 +++++++++++
 mm/page_alloc.c         |  8 ++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 3af34de54330..df76a0db7cec 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -243,6 +243,17 @@ static inline void clear_highpage(struct page *page)
 	kunmap_local(kaddr);
 }
 
+static inline void clear_highpage_tagged(struct page *page)
+{
+	u8 tag;
+
+	tag = page_kasan_tag(page);
+	page_kasan_tag_reset(page);
+	clear_highpage(page);
+	page_kasan_tag_set(page, tag);
+
+}
+
 #ifndef __HAVE_ARCH_TAG_CLEAR_HIGHPAGE
 
 static inline void tag_clear_highpage(struct page *page)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 66ef8c310dce..d82ea983a7a3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1302,12 +1302,8 @@ static void kernel_init_pages(struct page *page, int numpages)
 
 	/* s390's use of memset() could override KASAN redzones. */
 	kasan_disable_current();
-	for (i = 0; i < numpages; i++) {
-		u8 tag = page_kasan_tag(page + i);
-		page_kasan_tag_reset(page + i);
-		clear_highpage(page + i);
-		page_kasan_tag_set(page + i, tag);
-	}
+	for (i = 0; i < numpages; i++)
+		clear_highpage_tagged(page + i);
 	kasan_enable_current();
 }
 
-- 
2.25.1

Re: [PATCH 2/3] mm: introduce clear_highpage_tagged

[Andrew Morton]

On Tue, 31 May 2022 17:43:49 +0200 andrey.konovalov@linux.dev wrote:

> From: Andrey Konovalov <andreyknvl@google.com>
> 
> Add a clear_highpage_tagged() helper that does clear_highpage() on a
> page potentially tagged by KASAN.

clear_highpage_kasan_tagged() would be a better name, no?

--- a/include/linux/highmem.h~mm-introduce-clear_highpage_tagged-fix
+++ a/include/linux/highmem.h
@@ -243,7 +243,7 @@ static inline void clear_highpage(struct
 	kunmap_local(kaddr);
 }
 
-static inline void clear_highpage_tagged(struct page *page)
+static inline void clear_highpage_kasan_tagged(struct page *page)
 {
 	u8 tag;
 
--- a/mm/page_alloc.c~mm-introduce-clear_highpage_tagged-fix
+++ a/mm/page_alloc.c
@@ -1311,7 +1311,7 @@ static void kernel_init_pages(struct pag
 	/* s390's use of memset() could override KASAN redzones. */
 	kasan_disable_current();
 	for (i = 0; i < numpages; i++)
-		clear_highpage_tagged(page + i);
+		clear_highpage_kasan_tagged(page + i);
 	kasan_enable_current();
 }
 
_

Re: [PATCH 2/3] mm: introduce clear_highpage_tagged

[Andrey Konovalov]

On Tue, May 31, 2022 at 7:52 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 31 May 2022 17:43:49 +0200 andrey.konovalov@linux.dev wrote:
>
> > From: Andrey Konovalov <andreyknvl@google.com>
> >
> > Add a clear_highpage_tagged() helper that does clear_highpage() on a
> > page potentially tagged by KASAN.
>
> clear_highpage_kasan_tagged() would be a better name, no?

Sounds good! Will include into v2.

I also noticed there's an extra empty line at the end of the function
I need to fix.

>
> --- a/include/linux/highmem.h~mm-introduce-clear_highpage_tagged-fix
> +++ a/include/linux/highmem.h
> @@ -243,7 +243,7 @@ static inline void clear_highpage(struct
>         kunmap_local(kaddr);
>  }
>
> -static inline void clear_highpage_tagged(struct page *page)
> +static inline void clear_highpage_kasan_tagged(struct page *page)
>  {
>         u8 tag;
>
> --- a/mm/page_alloc.c~mm-introduce-clear_highpage_tagged-fix
> +++ a/mm/page_alloc.c
> @@ -1311,7 +1311,7 @@ static void kernel_init_pages(struct pag
>         /* s390's use of memset() could override KASAN redzones. */
>         kasan_disable_current();
>         for (i = 0; i < numpages; i++)
> -               clear_highpage_tagged(page + i);
> +               clear_highpage_kasan_tagged(page + i);
>         kasan_enable_current();
>  }
>
> _
>

[PATCH 3/3] kasan: fix zeroing vmalloc memory with HW_TAGS

[andrey.konovalov]

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. */
+
+		if (!(flags & KASAN_VMALLOC_INIT))
+			return (void *)start;
+
+		for (addr = start; addr < start + size; addr += PAGE_SIZE) {
+			page = virt_to_page(addr);
+			clear_highpage_tagged(page);
+		}
 
-	if (!is_vmalloc_or_module_addr(start))
 		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 07db42455dd4..0adf4aa1514d 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);
-- 
2.25.1

Re: [PATCH 3/3] kasan: fix zeroing vmalloc memory with HW_TAGS

[Marco Elver]

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;
 	}
 

Re: [PATCH 3/3] kasan: fix zeroing vmalloc memory with HW_TAGS

[Andrey Konovalov]

On Wed, Jun 1, 2022 at 2:28 PM Marco Elver <elver@google.com> wrote:
>
> 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:

Sounds good, will do in v2! Thanks!