| Message ID | 59db8d6b-4224-2ec9-09de-909c4338b67a@lca.pw (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CONFIG_KASAN_SW_TAGS=y not play well with kmemleak | expand |
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; > > 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)
On 2/8/19 12:15 PM, Andrey Konovalov wrote: > 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. The patch works great!
On Fri, Feb 08, 2019 at 06:15:02PM +0100, Andrey Konovalov wrote: > 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. > > Yeah, the issue is that kmemleak performs a bunch of pointer > comparisons that break when pointers are tagged. Does it mean that the kmemleak API receives pointer aliases (i.e. same object tagged with different values or tagged/untagged)?
On Mon, Feb 11, 2019 at 1:16 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Fri, Feb 08, 2019 at 06:15:02PM +0100, Andrey Konovalov wrote: > > 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. > > > > Yeah, the issue is that kmemleak performs a bunch of pointer > > comparisons that break when pointers are tagged. > > Does it mean that the kmemleak API receives pointer aliases (i.e. same > object tagged with different values or tagged/untagged)? Please disregard that comment. This is a bug in KASAN, will send a fix soon.
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;
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. 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] 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;