[PATCH 1/2] mm/kasan: dump alloc/free stack for page allocator

[Walter Wu]

This patch is KASAN report adds the alloc/free stacks for page allocator
in order to help programmer to see memory corruption caused by page.

By default, KASAN doesn't record alloc/free stack for page allocator.
It is difficult to fix up page use-after-free issue.

This feature depends on page owner to record the last stack of pages.
It is very helpful for solving the page use-after-free or out-of-bound.

KASAN report will show the last stack of page, it may be:
a) If page is in-use state, then it prints alloc stack.
It is useful to fix up page out-of-bound issue.

BUG: KASAN: slab-out-of-bounds in kmalloc_pagealloc_oob_right+0x88/0x90
Write of size 1 at addr ffffffc0d64ea00a by task cat/115
...
Allocation stack of page:
prep_new_page+0x1a0/0x1d8
get_page_from_freelist+0xd78/0x2748
__alloc_pages_nodemask+0x1d4/0x1978
kmalloc_order+0x28/0x58
kmalloc_order_trace+0x28/0xe0
kmalloc_pagealloc_oob_right+0x2c/0x90

b) If page is freed state, then it prints free stack.
It is useful to fix up page use-after-free issue.

BUG: KASAN: use-after-free in kmalloc_pagealloc_uaf+0x70/0x80
Write of size 1 at addr ffffffc0d651c000 by task cat/115
...
Free stack of page:
kasan_free_pages+0x68/0x70
__free_pages_ok+0x3c0/0x1328
__free_pages+0x50/0x78
kfree+0x1c4/0x250
kmalloc_pagealloc_uaf+0x38/0x80


This has been discussed, please refer below link.
https://bugzilla.kernel.org/show_bug.cgi?id=203967

Signed-off-by: Walter Wu <walter...@mediatek.com>
---
lib/Kconfig.kasan | 9 +++++++++
mm/kasan/common.c | 6 ++++++
2 files changed, 15 insertions(+)

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4fafba1a923b..ba17f706b5f8 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -135,6 +135,15 @@ config KASAN_S390_4_LEVEL_PAGING
to 3TB of RAM with KASan enabled). This options allows to force
4-level paging instead.

+config KASAN_DUMP_PAGE
+ bool "Dump the page last stack information"
+ depends on KASAN && PAGE_OWNER
+ help
+ By default, KASAN doesn't record alloc/free stack for page allocator.
+ It is difficult to fix up page use-after-free issue.
+ This feature depends on page owner to record the last stack of page.
+ It is very helpful for solving the page use-after-free or out-of-bound.
+
config TEST_KASAN
tristate "Module for testing KASAN for bug detection"
depends on m && KASAN
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 2277b82902d8..2a32474efa74 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -35,6 +35,7 @@
#include <linux/vmalloc.h>
#include <linux/bug.h>
#include <linux/uaccess.h>
+#include <linux/page_owner.h>

#include "kasan.h"
#include "../slab.h"
@@ -227,6 +228,11 @@ void kasan_alloc_pages(struct page *page, unsigned int order)

void kasan_free_pages(struct page *page, unsigned int order)
{
+#ifdef CONFIG_KASAN_DUMP_PAGE
+ gfp_t gfp_flags = GFP_KERNEL;
+
+ set_page_owner(page, order, gfp_flags);
+#endif
if (likely(!PageHighMem(page)))
kasan_poison_shadow(page_address(page),
PAGE_SIZE << order,
--
2.18.0

[Vlastimil Babka]

On 9/4/19 8:51 AM, Walter Wu wrote:
> This patch is KASAN report adds the alloc/free stacks for page allocator
> in order to help programmer to see memory corruption caused by page.
>
> By default, KASAN doesn't record alloc/free stack for page allocator.
> It is difficult to fix up page use-after-free issue.
>
> This feature depends on page owner to record the last stack of pages.
> It is very helpful for solving the page use-after-free or out-of-bound.
>
> KASAN report will show the last stack of page, it may be:
> a) If page is in-use state, then it prints alloc stack.
> It is useful to fix up page out-of-bound issue.

I expect this will conflict both in syntax and semantics with my series [1] that
adds the freeing stack to page_owner when used together with debug_pagealloc,
and it's now in mmotm. Glad others see the need as well :) Perhaps you could
review the series, see if it fulfils your usecase (AFAICS the series should be a
superset, by storing both stacks at once), and perhaps either make KASAN enable
debug_pagealloc, or turn KASAN into an alternative enabler of the functionality
there?

Thanks, Vlastimil

[1] https://lore.kernel.org/linux-mm/20190820131828...@suse.cz/t/#u

[Andrey Konovalov]

I'm not sure if we need a separate config for this. Is there any
reason to not have this enabled by default?

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190904065133.20268-1-walter-zh.wu%40mediatek.com.

[Walter Wu]

On Wed, 2019-09-04 at 14:49 +0200, Vlastimil Babka wrote:
> On 9/4/19 8:51 AM, Walter Wu wrote:
> > This patch is KASAN report adds the alloc/free stacks for page allocator
> > in order to help programmer to see memory corruption caused by page.
> >
> > By default, KASAN doesn't record alloc/free stack for page allocator.
> > It is difficult to fix up page use-after-free issue.
> >
> > This feature depends on page owner to record the last stack of pages.
> > It is very helpful for solving the page use-after-free or out-of-bound.
> >
> > KASAN report will show the last stack of page, it may be:
> > a) If page is in-use state, then it prints alloc stack.
> > It is useful to fix up page out-of-bound issue.
>
> I expect this will conflict both in syntax and semantics with my series [1] that
> adds the freeing stack to page_owner when used together with debug_pagealloc,
> and it's now in mmotm. Glad others see the need as well :) Perhaps you could
> review the series, see if it fulfils your usecase (AFAICS the series should be a
> superset, by storing both stacks at once), and perhaps either make KASAN enable
> debug_pagealloc, or turn KASAN into an alternative enabler of the functionality
> there?
>
> Thanks, Vlastimil
>
> [1] https://lore.kernel.org/linux-mm/20190820131828...@suse.cz/t/#u
>

Thanks your information.
We focus on the smartphone, so it doesn't enable
CONFIG_TRANSPARENT_HUGEPAGE, Is it invalid for our usecase?
And It looks like something is different, because we only need last
stack of page, so it can decrease memory overhead.
I will try to enable debug_pagealloc(with your patch) and KASAN, then we
see the result.

Thanks.
Walter

[Vlastimil Babka]

On 9/4/19 4:06 PM, Walter Wu wrote:
> On Wed, 2019-09-04 at 14:49 +0200, Vlastimil Babka wrote:
>> On 9/4/19 8:51 AM, Walter Wu wrote:
>> > This patch is KASAN report adds the alloc/free stacks for page allocator
>> > in order to help programmer to see memory corruption caused by page.
>> >
>> > By default, KASAN doesn't record alloc/free stack for page allocator.
>> > It is difficult to fix up page use-after-free issue.
>> >
>> > This feature depends on page owner to record the last stack of pages.
>> > It is very helpful for solving the page use-after-free or out-of-bound.
>> >
>> > KASAN report will show the last stack of page, it may be:
>> > a) If page is in-use state, then it prints alloc stack.
>> > It is useful to fix up page out-of-bound issue.
>>
>> I expect this will conflict both in syntax and semantics with my series [1] that
>> adds the freeing stack to page_owner when used together with debug_pagealloc,
>> and it's now in mmotm. Glad others see the need as well :) Perhaps you could
>> review the series, see if it fulfils your usecase (AFAICS the series should be a
>> superset, by storing both stacks at once), and perhaps either make KASAN enable
>> debug_pagealloc, or turn KASAN into an alternative enabler of the functionality
>> there?
>>
>> Thanks, Vlastimil
>>
>> [1] https://lore.kernel.org/linux-mm/20190820131828...@suse.cz/t/#u
>>
> Thanks your information.
> We focus on the smartphone, so it doesn't enable
> CONFIG_TRANSPARENT_HUGEPAGE, Is it invalid for our usecase?

The THP fix is not required for the rest of the series, it was even merged to
mainline separately.

> And It looks like something is different, because we only need last
> stack of page, so it can decrease memory overhead.

That would save you depot_stack_handle_t (which is u32) per page. I guess that's
nothing compared to KASAN overhead?

> I will try to enable debug_pagealloc(with your patch) and KASAN, then we
> see the result.

Thanks.

> Thanks.
> Walter
>

[Walter Wu]

On Wed, 2019-09-04 at 15:44 +0200, Andrey Konovalov wrote:
> On Wed, Sep 4, 2019 at 8:51 AM Walter Wu <walter...@mediatek.com> wrote:
> > +config KASAN_DUMP_PAGE
> > + bool "Dump the page last stack information"
> > + depends on KASAN && PAGE_OWNER
> > + help
> > + By default, KASAN doesn't record alloc/free stack for page allocator.
> > + It is difficult to fix up page use-after-free issue.
> > + This feature depends on page owner to record the last stack of page.
> > + It is very helpful for solving the page use-after-free or out-of-bound.
>
> I'm not sure if we need a separate config for this. Is there any
> reason to not have this enabled by default?

PAGE_OWNER need some memory usage, it is not allowed to enable by
default in low RAM device. so I create new feature option and the person
who wants to use it to enable it.

Thanks.
Walter

[Walter Wu]

If we can use less memory, we can achieve what we want. Why not?

Thanks.
Walter

[Qian Cai]

Or you can try to look into reducing the memory footprint of PAGE_OWNER to fit
your needs. It does not always need to be that way.

[Walter Wu]

Thanks your suggestion. We can try to think what can be slimmed.

Thanks.
Walter

[Vlastimil Babka]

In my experience to solve some UAFs, it's important to know not only the
freeing stack, but also the allocating stack. Do they make sense together,
or not? In some cases, even longer history of alloc/free would be nice :)

Also by simply recording the free stack in the existing depot handle,
you might confuse existing page_owner file consumers, who won't know
that this is a freeing stack.

All that just doesn't seem to justify saving an u32 per page.

> Thanks.
> Walter
>
>
>

[Walter Wu]

On Thu, 2019-09-05 at 10:03 +0200, Vlastimil Babka wrote:
> On 9/4/19 4:24 PM, Walter Wu wrote:
> > On Wed, 2019-09-04 at 16:13 +0200, Vlastimil Babka wrote:
> >> On 9/4/19 4:06 PM, Walter Wu wrote:
> >>
> >> The THP fix is not required for the rest of the series, it was even merged to
> >> mainline separately.
> >>
> >>> And It looks like something is different, because we only need last
> >>> stack of page, so it can decrease memory overhead.
> >>
> >> That would save you depot_stack_handle_t (which is u32) per page. I guess that's
> >> nothing compared to KASAN overhead?
> >>
> > If we can use less memory, we can achieve what we want. Why not?
>
> In my experience to solve some UAFs, it's important to know not only the
> freeing stack, but also the allocating stack. Do they make sense together,
> or not? In some cases, even longer history of alloc/free would be nice :)
>

We think it only has free stack to find out the root cause. Maybe we can
refer to other people's experience and ideas.

> Also by simply recording the free stack in the existing depot handle,
> you might confuse existing page_owner file consumers, who won't know
> that this is a freeing stack.
>

Don't worry it.
1. Our feature option has this description about last stack of page.
when consumer enable our feature, they should know the changing.
2. We add to print text message for alloc or free stack before dump the
stack of page. so consumers should know what is it.

> All that just doesn't seem to justify saving an u32 per page.

Actually, We want to slim memory usage instead of increasing the memory
usage at another mail discussion. Maybe, maintainer or reviewer can
provide some ideas. That will be great.

> >
> >
>

[walter...@mediatek.com]

From: Walter Wu <walter...@mediatek.com>

This patch is KASAN report adds the alloc/free stacks for page allocator
in order to help programmer to see memory corruption caused by page.

By default, KASAN doesn't record alloc and free stack for page allocator.
It is difficult to fix up page use-after-free or dobule-free issue.

Our patchsets will record the last stack of pages.
It is very helpful for solving the page use-after-free or double-free.

KASAN report will show the last stack of page, it may be:
a) If page is in-use state, then it prints alloc stack.
It is useful to fix up page out-of-bound issue.

BUG: KASAN: slab-out-of-bounds in kmalloc_pagealloc_oob_right+0x88/0x90
Write of size 1 at addr ffffffc0d64ea00a by task cat/115
...
Allocation stack of page:

set_page_stack.constprop.1+0x30/0xc8
kasan_alloc_pages+0x18/0x38
prep_new_page+0x5c/0x150
get_page_from_freelist+0xb8c/0x17c8
__alloc_pages_nodemask+0x1a0/0x11b0
kmalloc_order+0x28/0x58
kmalloc_order_trace+0x28/0xe0
kmalloc_pagealloc_oob_right+0x2c/0x68

b) If page is freed state, then it prints free stack.

It is useful to fix up page use-after-free or double-free issue.

BUG: KASAN: use-after-free in kmalloc_pagealloc_uaf+0x70/0x80
Write of size 1 at addr ffffffc0d651c000 by task cat/115
...
Free stack of page:
kasan_free_pages+0x68/0x70
__free_pages_ok+0x3c0/0x1328
__free_pages+0x50/0x78
kfree+0x1c4/0x250
kmalloc_pagealloc_uaf+0x38/0x80

This has been discussed, please refer below link.
https://bugzilla.kernel.org/show_bug.cgi?id=203967

Changes since v1:
- slim page_owner and move it into kasan
- enable the feature by default

Signed-off-by: Walter Wu <walter...@mediatek.com>
---

include/linux/kasan.h | 1 +
lib/Kconfig.kasan | 2 ++
mm/kasan/common.c | 32 ++++++++++++++++++++++++++++++++
mm/kasan/kasan.h | 5 +++++
mm/kasan/report.c | 27 +++++++++++++++++++++++++++
5 files changed, 67 insertions(+)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index cc8a03cc9674..97e1bcb20489 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -19,6 +19,7 @@ extern pte_t kasan_early_shadow_pte[PTRS_PER_PTE];
extern pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD];
extern pud_t kasan_early_shadow_pud[PTRS_PER_PUD];
extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
+extern struct page_ext_operations page_stack_ops;

int kasan_populate_early_shadow(const void *shadow_start,
const void *shadow_end);
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4fafba1a923b..b5a9410ba4e8 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -41,6 +41,7 @@ config KASAN_GENERIC
select SLUB_DEBUG if SLUB
select CONSTRUCTORS
select STACKDEPOT
+ select PAGE_EXTENSION
help
Enables generic KASAN mode.
Supported in both GCC and Clang. With GCC it requires version 4.9.2
@@ -63,6 +64,7 @@ config KASAN_SW_TAGS
select SLUB_DEBUG if SLUB
select CONSTRUCTORS
select STACKDEPOT
+ select PAGE_EXTENSION
help
Enables software tag-based KASAN mode.
This mode requires Top Byte Ignore support by the CPU and therefore
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 2277b82902d8..c349143d2587 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -211,10 +211,38 @@ void kasan_unpoison_stack_above_sp_to(const void *watermark)
kasan_unpoison_shadow(sp, size);
}

+static bool need_page_stack(void)
+{
+ return true;
+}
+
+struct page_ext_operations page_stack_ops = {
+ .size = sizeof(depot_stack_handle_t),
+ .need = need_page_stack,
+};
+
+static void set_page_stack(struct page *page, gfp_t gfp_mask)
+{
+ struct page_ext *page_ext = lookup_page_ext(page);
+ depot_stack_handle_t handle;
+ depot_stack_handle_t *page_stack;
+
+ if (unlikely(!page_ext))
+ return;
+
+ handle = save_stack(gfp_mask);
+
+ page_stack = get_page_stack(page_ext);
+ *page_stack = handle;
+}
+

void kasan_alloc_pages(struct page *page, unsigned int order)

{
u8 tag;
unsigned long i;

+ gfp_t gfp_flags = GFP_KERNEL;
+

+ set_page_stack(page, gfp_flags);

if (unlikely(PageHighMem(page)))
return;
@@ -227,6 +255,10 @@ void kasan_alloc_pages(struct page *page, unsigned int order)

void kasan_free_pages(struct page *page, unsigned int order)
{

+ gfp_t gfp_flags = GFP_KERNEL;
+

+ set_page_stack(page, gfp_flags);
+

if (likely(!PageHighMem(page)))
kasan_poison_shadow(page_address(page),
PAGE_SIZE << order,

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 014f19e76247..95b3b510d04f 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -126,6 +126,11 @@ static inline bool addr_has_shadow(const void *addr)
return (addr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
}

+static inline depot_stack_handle_t *get_page_stack(struct page_ext *page_ext)
+{
+ return (void *)page_ext + page_stack_ops.offset;
+}
+
void kasan_poison_shadow(const void *address, size_t size, u8 value);

/**
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 0e5f965f1882..2e26bc192114 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -344,6 +344,32 @@ static void print_address_stack_frame(const void *addr)
print_decoded_frame_descr(frame_descr);
}

+static void dump_page_stack(struct page *page)
+{
+ struct page_ext *page_ext = lookup_page_ext(page);
+ depot_stack_handle_t handle;
+ unsigned long *entries;
+ unsigned int nr_entries;
+ depot_stack_handle_t *page_stack;
+
+ if (unlikely(!page_ext))
+ return;
+
+ page_stack = get_page_stack(page_ext);
+
+ handle = READ_ONCE(*page_stack);
+ if (!handle)
+ return;
+
+ if ((unsigned long)page->flags & PAGE_FLAGS_CHECK_AT_PREP)
+ pr_info("Allocation stack of page:\n");
+ else
+ pr_info("Free stack of page:\n");
+
+ nr_entries = stack_depot_fetch(handle, &entries);
+ stack_trace_print(entries, nr_entries, 0);
+}
+
static void print_address_description(void *addr)
{
struct page *page = addr_to_page(addr);
@@ -366,6 +392,7 @@ static void print_address_description(void *addr)
if (page) {
pr_err("The buggy address belongs to the page:\n");
dump_page(page, "kasan: bad access detected");
+ dump_page_stack(page);
}

print_address_stack_frame(addr);
--
2.18.0

[Vlastimil Babka]

On 9/9/19 10:24 AM, walter...@mediatek.com wrote:
> From: Walter Wu <walter...@mediatek.com>
>
> This patch is KASAN report adds the alloc/free stacks for page allocator
> in order to help programmer to see memory corruption caused by page.
>
> By default, KASAN doesn't record alloc and free stack for page allocator.
> It is difficult to fix up page use-after-free or dobule-free issue.
>
> Our patchsets will record the last stack of pages.
> It is very helpful for solving the page use-after-free or double-free.
>
> KASAN report will show the last stack of page, it may be:
> a) If page is in-use state, then it prints alloc stack.
> It is useful to fix up page out-of-bound issue.

I still disagree with duplicating most of page_owner functionality for
the sake of using a single stack handle for both alloc and free (while
page_owner + debug_pagealloc with patches in mmotm uses two handles). It
reduces the amount of potentially important debugging information, and I
really doubt the u32-per-page savings are significant, given the rest of
KASAN overhead.

That's not a discussion, but a single comment from Dmitry, which btw
contains "provide alloc *and* free stacks for it" ("it" refers to page,
emphasis mine). It would be nice if he or other KASAN guys could clarify.

[Walter Wu]

Hi All,

We implement another version, it is different with v1. We hope that you
can give an ideas and make the KASAN report better. If it is possible,
we can use the less memory to show the corruption information that is
enough to help programmer to fix up memory corruption.

Thanks.
Walter

[Andrey Ryabinin]

For slab objects we memorize both alloc and free stacks. You'll never know in advance what information will be usefull
to fix an issue, so it usually better to provide more information. I don't think we should do anything different for pages.

Given that we already have the page_owner responsible for providing alloc/free stacks for pages, all that we should in KASAN do is to
enable the feature by default. Free stack saving should be decoupled from debug_pagealloc into separate option so that it can be enabled
by KASAN and/or debug_pagealloc.

[Vlastimil Babka]

On 9/10/19 12:50 PM, Andrey Ryabinin wrote:
>
>
> For slab objects we memorize both alloc and free stacks. You'll never know in advance what information will be usefull
> to fix an issue, so it usually better to provide more information. I don't think we should do anything different for pages.

Exactly, thanks.

> Given that we already have the page_owner responsible for providing alloc/free stacks for pages, all that we should in KASAN do is to
> enable the feature by default. Free stack saving should be decoupled from debug_pagealloc into separate option so that it can be enabled
> by KASAN and/or debug_pagealloc.

Right. Walter, can you do it that way, or should I?

Thanks,
Vlastimil

[Walter Wu]

I will send new patch v3.

[Walter Wu]

Thanks your suggestion.
We will send the patch v3 as described above.