| Message ID | 20190904065133.20268-1-walter-zh.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] mm/kasan: dump alloc/free stack for page allocator | expand |
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.22684-1-vbabka@suse.cz/t/#u > 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-zh.wu@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, >
On Wed, Sep 4, 2019 at 8:51 AM Walter Wu <walter-zh.wu@mediatek.com> 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. > > 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-zh.wu@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. I'm not sure if we need a separate config for this. Is there any reason to not have this enabled by default? > + > 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 > > -- > 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+unsubscribe@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.
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.22684-1-vbabka@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
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.22684-1-vbabka@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 >
On Wed, 2019-09-04 at 15:44 +0200, Andrey Konovalov wrote: > On Wed, Sep 4, 2019 at 8:51 AM Walter Wu <walter-zh.wu@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
On Wed, 2019-09-04 at 16:13 +0200, Vlastimil Babka wrote: > 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.22684-1-vbabka@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? > If we can use less memory, we can achieve what we want. Why not? Thanks. Walter
On Wed, 2019-09-04 at 22:16 +0800, Walter Wu wrote: > On Wed, 2019-09-04 at 15:44 +0200, Andrey Konovalov wrote: > > On Wed, Sep 4, 2019 at 8:51 AM Walter Wu <walter-zh.wu@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. 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.
On Wed, 2019-09-04 at 10:37 -0400, Qian Cai wrote: > On Wed, 2019-09-04 at 22:16 +0800, Walter Wu wrote: > > On Wed, 2019-09-04 at 15:44 +0200, Andrey Konovalov wrote: > > > On Wed, Sep 4, 2019 at 8:51 AM Walter Wu <walter-zh.wu@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. > > 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. Thanks your suggestion. We can try to think what can be slimmed. Thanks. Walter
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 :) 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 > > >
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. > > > > >
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,
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-zh.wu@mediatek.com> --- lib/Kconfig.kasan | 9 +++++++++ mm/kasan/common.c | 6 ++++++ 2 files changed, 15 insertions(+)