From: Andrey Konovalov <andreyknvl@google.com> To: Qian Cai <cai@lca.pw> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>, Alexander Potapenko <glider@google.com>, Dmitry Vyukov <dvyukov@google.com>, kasan-dev <kasan-dev@googlegroups.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Linux-MM <linux-mm@kvack.org>, Catalin Marinas <catalin.marinas@arm.com> Subject: Re: CONFIG_KASAN_SW_TAGS=y not play well with kmemleak Date: Fri, 8 Feb 2019 18:15:02 +0100 [thread overview] Message-ID: <CAAeHK+wsULxYXnGJnQXx9HjZMiU-5jb5ZKC+TuGQihc9L386Xg@mail.gmail.com> (raw) In-Reply-To: <59db8d6b-4224-2ec9-09de-909c4338b67a@lca.pw> [-- Attachment #1: Type: text/plain, Size: 2904 bytes --] On Fri, Feb 8, 2019 at 5:16 AM Qian Cai <cai@lca.pw> wrote: > > Kmemleak is totally busted with CONFIG_KASAN_SW_TAGS=y because most of tracking > object pointers passed to create_object() have the upper bits set by KASAN. Hi Qian, Yeah, the issue is that kmemleak performs a bunch of pointer comparisons that break when pointers are tagged. Try the attached patch, it should fix the issue. I don't like the way this patch does it though, I'll see if I can come up with something better. Thanks for the report! > However, even after applied this patch [1] to fix a few things, it still has > many errors during boot. > > https://git.sr.ht/~cai/linux-debug/tree/master/dmesg > > What I don't understand is that even the patch did call kasan_reset_tag() in > paint_ptr(), it still complained on objects with upper bits set which indicates > that this line did not run. > > return (__s64)(value << shift) >> shift; > > [ 42.462799] kmemleak: Trying to color unknown object at 0xffff80082df80000 as > Grey > [ 42.470524] CPU: 128 PID: 1 Comm: swapper/0 Not tainted 5.0.0-rc5+ #17 > [ 42.477153] Call trace: > [ 42.479639] dump_backtrace+0x0/0x450 > [ 42.483362] show_stack+0x20/0x2c > [ 42.486733] __dump_stack+0x20/0x28 > [ 42.490276] dump_stack+0xa0/0xfc > [ 42.493649] paint_ptr+0xa8/0xf4 > [ 42.496934] kmemleak_not_leak+0xa4/0x15c > [ 42.501013] init_section_page_ext+0x1bc/0x328 > [ 42.505528] page_ext_init+0x4dc/0x75c > [ 42.509336] kernel_init_freeable+0x684/0x1104 > [ 42.513857] kernel_init+0x18/0x2a4 > [ 42.517407] ret_from_fork+0x10/0x18 > > [1] > diff --git a/mm/kmemleak.c b/mm/kmemleak.c > index f9d9dc250428..70343d887f34 100644 > --- a/mm/kmemleak.c > +++ b/mm/kmemleak.c > @@ -588,7 +588,7 @@ static struct kmemleak_object *create_object(unsigned long > ptr, size_t size, > spin_lock_init(&object->lock); > atomic_set(&object->use_count, 1); > object->flags = OBJECT_ALLOCATED; > - object->pointer = ptr; > + object->pointer = (unsigned long)kasan_reset_tag((void *)ptr); > object->size = size; > object->excess_ref = 0; > object->min_count = min_count; > @@ -748,11 +748,12 @@ static void paint_it(struct kmemleak_object *object, int > color) > static void paint_ptr(unsigned long ptr, int color) > { > struct kmemleak_object *object; > + unsigned long addr = (unsigned long)kasan_reset_tag((void *)ptr); > > - object = find_and_get_object(ptr, 0); > + object = find_and_get_object(addr, 0); > if (!object) { > kmemleak_warn("Trying to color unknown object at 0x%08lx as %s\n", > - ptr, > + addr, > (color == KMEMLEAK_GREY) ? "Grey" : > (color == KMEMLEAK_BLACK) ? "Black" : "Unknown"); > return; > > [-- Attachment #2: kasan-kmemleak-fix.patch --] [-- Type: text/x-patch, Size: 2079 bytes --] diff --git a/mm/kmemleak.c b/mm/kmemleak.c index f9d9dc250428..5354e74f0d19 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -437,6 +437,8 @@ static struct kmemleak_object *lookup_object(unsigned long ptr, int alias) { struct rb_node *rb = object_tree_root.rb_node; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + while (rb) { struct kmemleak_object *object = rb_entry(rb, struct kmemleak_object, rb_node); @@ -575,6 +577,8 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size, struct kmemleak_object *object, *parent; struct rb_node **link, *rb_parent; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp)); if (!object) { pr_warn("Cannot allocate a kmemleak_object structure\n"); @@ -701,6 +705,8 @@ static void delete_object_part(unsigned long ptr, size_t size) struct kmemleak_object *object; unsigned long start, end; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + object = find_and_remove_object(ptr, 1); if (!object) { #ifdef DEBUG @@ -789,6 +795,8 @@ static void add_scan_area(unsigned long ptr, size_t size, gfp_t gfp) struct kmemleak_object *object; struct kmemleak_scan_area *area; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + object = find_and_get_object(ptr, 1); if (!object) { kmemleak_warn("Adding scan area to unknown object at 0x%08lx\n", @@ -1334,6 +1342,9 @@ static void scan_block(void *_start, void *_end, unsigned long *end = _end - (BYTES_PER_POINTER - 1); unsigned long flags; + start = (unsigned long *)kasan_reset_tag((void *)start); + end = (unsigned long *)kasan_reset_tag((void *)end); + read_lock_irqsave(&kmemleak_lock, flags); for (ptr = start; ptr < end; ptr++) { struct kmemleak_object *object; @@ -1344,7 +1355,7 @@ static void scan_block(void *_start, void *_end, break; kasan_disable_current(); - pointer = *ptr; + pointer = (unsigned long)kasan_reset_tag((void *)*ptr); kasan_enable_current(); if (pointer < min_addr || pointer >= max_addr)
WARNING: multiple messages have this Message-ID (diff)
From: Andrey Konovalov <andreyknvl@google.com> To: Qian Cai <cai@lca.pw> Cc: Catalin Marinas <catalin.marinas@arm.com>, kasan-dev <kasan-dev@googlegroups.com>, Linux-MM <linux-mm@kvack.org>, Alexander Potapenko <glider@google.com>, Dmitry Vyukov <dvyukov@google.com>, Andrey Ryabinin <aryabinin@virtuozzo.com>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: CONFIG_KASAN_SW_TAGS=y not play well with kmemleak Date: Fri, 8 Feb 2019 18:15:02 +0100 [thread overview] Message-ID: <CAAeHK+wsULxYXnGJnQXx9HjZMiU-5jb5ZKC+TuGQihc9L386Xg@mail.gmail.com> (raw) In-Reply-To: <59db8d6b-4224-2ec9-09de-909c4338b67a@lca.pw> [-- Attachment #1: Type: text/plain, Size: 2904 bytes --] On Fri, Feb 8, 2019 at 5:16 AM Qian Cai <cai@lca.pw> wrote: > > Kmemleak is totally busted with CONFIG_KASAN_SW_TAGS=y because most of tracking > object pointers passed to create_object() have the upper bits set by KASAN. Hi Qian, Yeah, the issue is that kmemleak performs a bunch of pointer comparisons that break when pointers are tagged. Try the attached patch, it should fix the issue. I don't like the way this patch does it though, I'll see if I can come up with something better. Thanks for the report! > However, even after applied this patch [1] to fix a few things, it still has > many errors during boot. > > https://git.sr.ht/~cai/linux-debug/tree/master/dmesg > > What I don't understand is that even the patch did call kasan_reset_tag() in > paint_ptr(), it still complained on objects with upper bits set which indicates > that this line did not run. > > return (__s64)(value << shift) >> shift; > > [ 42.462799] kmemleak: Trying to color unknown object at 0xffff80082df80000 as > Grey > [ 42.470524] CPU: 128 PID: 1 Comm: swapper/0 Not tainted 5.0.0-rc5+ #17 > [ 42.477153] Call trace: > [ 42.479639] dump_backtrace+0x0/0x450 > [ 42.483362] show_stack+0x20/0x2c > [ 42.486733] __dump_stack+0x20/0x28 > [ 42.490276] dump_stack+0xa0/0xfc > [ 42.493649] paint_ptr+0xa8/0xf4 > [ 42.496934] kmemleak_not_leak+0xa4/0x15c > [ 42.501013] init_section_page_ext+0x1bc/0x328 > [ 42.505528] page_ext_init+0x4dc/0x75c > [ 42.509336] kernel_init_freeable+0x684/0x1104 > [ 42.513857] kernel_init+0x18/0x2a4 > [ 42.517407] ret_from_fork+0x10/0x18 > > [1] > diff --git a/mm/kmemleak.c b/mm/kmemleak.c > index f9d9dc250428..70343d887f34 100644 > --- a/mm/kmemleak.c > +++ b/mm/kmemleak.c > @@ -588,7 +588,7 @@ static struct kmemleak_object *create_object(unsigned long > ptr, size_t size, > spin_lock_init(&object->lock); > atomic_set(&object->use_count, 1); > object->flags = OBJECT_ALLOCATED; > - object->pointer = ptr; > + object->pointer = (unsigned long)kasan_reset_tag((void *)ptr); > object->size = size; > object->excess_ref = 0; > object->min_count = min_count; > @@ -748,11 +748,12 @@ static void paint_it(struct kmemleak_object *object, int > color) > static void paint_ptr(unsigned long ptr, int color) > { > struct kmemleak_object *object; > + unsigned long addr = (unsigned long)kasan_reset_tag((void *)ptr); > > - object = find_and_get_object(ptr, 0); > + object = find_and_get_object(addr, 0); > if (!object) { > kmemleak_warn("Trying to color unknown object at 0x%08lx as %s\n", > - ptr, > + addr, > (color == KMEMLEAK_GREY) ? "Grey" : > (color == KMEMLEAK_BLACK) ? "Black" : "Unknown"); > return; > > [-- Attachment #2: kasan-kmemleak-fix.patch --] [-- Type: text/x-patch, Size: 2079 bytes --] diff --git a/mm/kmemleak.c b/mm/kmemleak.c index f9d9dc250428..5354e74f0d19 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -437,6 +437,8 @@ static struct kmemleak_object *lookup_object(unsigned long ptr, int alias) { struct rb_node *rb = object_tree_root.rb_node; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + while (rb) { struct kmemleak_object *object = rb_entry(rb, struct kmemleak_object, rb_node); @@ -575,6 +577,8 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size, struct kmemleak_object *object, *parent; struct rb_node **link, *rb_parent; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp)); if (!object) { pr_warn("Cannot allocate a kmemleak_object structure\n"); @@ -701,6 +705,8 @@ static void delete_object_part(unsigned long ptr, size_t size) struct kmemleak_object *object; unsigned long start, end; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + object = find_and_remove_object(ptr, 1); if (!object) { #ifdef DEBUG @@ -789,6 +795,8 @@ static void add_scan_area(unsigned long ptr, size_t size, gfp_t gfp) struct kmemleak_object *object; struct kmemleak_scan_area *area; + ptr = (unsigned long)kasan_reset_tag((void *)ptr); + object = find_and_get_object(ptr, 1); if (!object) { kmemleak_warn("Adding scan area to unknown object at 0x%08lx\n", @@ -1334,6 +1342,9 @@ static void scan_block(void *_start, void *_end, unsigned long *end = _end - (BYTES_PER_POINTER - 1); unsigned long flags; + start = (unsigned long *)kasan_reset_tag((void *)start); + end = (unsigned long *)kasan_reset_tag((void *)end); + read_lock_irqsave(&kmemleak_lock, flags); for (ptr = start; ptr < end; ptr++) { struct kmemleak_object *object; @@ -1344,7 +1355,7 @@ static void scan_block(void *_start, void *_end, break; kasan_disable_current(); - pointer = *ptr; + pointer = (unsigned long)kasan_reset_tag((void *)*ptr); kasan_enable_current(); if (pointer < min_addr || pointer >= max_addr) [-- Attachment #3: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-02-08 17:15 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-07 4:04 CONFIG_KASAN_SW_TAGS=y NULL pointer dereference at freelist_dereference() Qian Cai 2019-02-07 4:04 ` Qian Cai 2019-02-07 12:58 ` Andrey Konovalov 2019-02-07 12:58 ` Andrey Konovalov 2019-02-07 13:27 ` Qian Cai 2019-02-07 13:27 ` Qian Cai 2019-02-07 15:34 ` Andrey Konovalov 2019-02-07 15:34 ` Andrey Konovalov 2019-02-08 0:30 ` Qian Cai 2019-02-08 0:30 ` Qian Cai 2019-02-08 4:16 ` CONFIG_KASAN_SW_TAGS=y not play well with kmemleak Qian Cai 2019-02-08 17:15 ` Andrey Konovalov [this message] 2019-02-08 17:15 ` Andrey Konovalov 2019-02-09 2:17 ` Qian Cai 2019-02-09 2:17 ` Qian Cai 2019-02-11 12:15 ` Catalin Marinas 2019-02-11 12:15 ` Catalin Marinas 2019-02-11 18:52 ` Andrey Konovalov 2019-02-11 18:52 ` Andrey Konovalov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAAeHK+wsULxYXnGJnQXx9HjZMiU-5jb5ZKC+TuGQihc9L386Xg@mail.gmail.com \ --to=andreyknvl@google.com \ --cc=aryabinin@virtuozzo.com \ --cc=cai@lca.pw \ --cc=catalin.marinas@arm.com \ --cc=dvyukov@google.com \ --cc=glider@google.com \ --cc=kasan-dev@googlegroups.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-mm@kvack.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.