On Mon, 2023-01-09 at 07:51 +0100, Dmitry Vyukov wrote: > On Tue, 3 Jan 2023 at 08:56, 'Kuan-Ying Lee' via kasan-dev > <kasan-dev@xxxxxxxxxxxxxxxx> wrote: > > > > We scan the shadow memory to infer the requested size instead of > > printing cache->object_size directly. > > > > This patch will fix the confusing generic kasan report like below. > > [1] > > Report shows "cache kmalloc-192 of size 192", but user > > actually kmalloc(184). > > > > ================================================================== > > BUG: KASAN: slab-out-of-bounds in _find_next_bit+0x143/0x160 > > lib/find_bit.c:109 > > Read of size 8 at addr ffff8880175766b8 by task kworker/1:1/26 > > ... > > The buggy address belongs to the object at ffff888017576600 > > which belongs to the cache kmalloc-192 of size 192 > > The buggy address is located 184 bytes inside of > > 192-byte region [ffff888017576600, ffff8880175766c0) > > ... > > Memory state around the buggy address: > > ffff888017576580: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc > > ffff888017576600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > ffff888017576680: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc > > > > ^ > > ffff888017576700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > ffff888017576780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > ================================================================== > > > > After this patch, report will show "cache kmalloc-192 of size 184". > > > > Link: > > https://urldefense.com/v3/__https://bugzilla.kernel.org/show_bug.cgi?id=216457__;!!CTRNKA9wMg0ARbw!mLNcuZ83c39d0Xkut-WMY3CcvZcAYDuLCmv4mu7IAldw4_n4i6XvX8GORBfjOadWxOa6d-ODQdx6ZCSvB2g13Q$ > > $ [1] > > > > Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@xxxxxxxxxxxx> > > --- > > mm/kasan/kasan.h | 5 +++++ > > mm/kasan/report.c | 3 ++- > > mm/kasan/report_generic.c | 18 ++++++++++++++++++ > > 3 files changed, 25 insertions(+), 1 deletion(-) > > > > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > > index 32413f22aa82..7bb627d21580 100644 > > --- a/mm/kasan/kasan.h > > +++ b/mm/kasan/kasan.h > > @@ -340,8 +340,13 @@ static inline void > > kasan_print_address_stack_frame(const void *addr) { } > > > > #ifdef CONFIG_KASAN_GENERIC > > void kasan_print_aux_stacks(struct kmem_cache *cache, const void > > *object); > > +int kasan_get_alloc_size(void *object_addr, struct kmem_cache > > *cache); > > #else > > static inline void kasan_print_aux_stacks(struct kmem_cache > > *cache, const void *object) { } > > +static inline int kasan_get_alloc_size(void *object_addr, struct > > kmem_cache *cache) > > +{ > > + return cache->object_size; > > +} > > #endif > > > > bool kasan_report(unsigned long addr, size_t size, > > diff --git a/mm/kasan/report.c b/mm/kasan/report.c > > index 1d02757e90a3..6de454bb2cad 100644 > > --- a/mm/kasan/report.c > > +++ b/mm/kasan/report.c > > @@ -236,12 +236,13 @@ static void describe_object_addr(const void > > *addr, struct kmem_cache *cache, > > { > > unsigned long access_addr = (unsigned long)addr; > > unsigned long object_addr = (unsigned long)object; > > + int real_size = kasan_get_alloc_size((void *)object_addr, > > cache); > > const char *rel_type; > > int rel_bytes; > > > > pr_err("The buggy address belongs to the object at %px\n" > > " which belongs to the cache %s of size %d\n", > > - object, cache->name, cache->object_size); > > + object, cache->name, real_size); > > > > if (access_addr < object_addr) { > > rel_type = "to the left"; > > diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c > > index 043c94b04605..01b38e459352 100644 > > --- a/mm/kasan/report_generic.c > > +++ b/mm/kasan/report_generic.c > > @@ -43,6 +43,24 @@ void *kasan_find_first_bad_addr(void *addr, > > size_t size) > > return p; > > } > > > > +int kasan_get_alloc_size(void *addr, struct kmem_cache *cache) > > +{ > > + int size = 0; > > + u8 *shadow = (u8 *)kasan_mem_to_shadow(addr); > > + > > + while (size < cache->object_size) { > > + if (*shadow == 0) > > + size += KASAN_GRANULE_SIZE; > > + else if (*shadow >= 1 && *shadow <= > > KASAN_GRANULE_SIZE - 1) > > + size += *shadow; > > + else > > + return size; > > + shadow++; > > This only works for out-of-bounds reports, but I don't see any checks > for report type. Won't this break reporting for all other report > types? > I think it won't break reporting for other report types. This function is only called by slab OOB and UAF. > I would also print the cache name anyway. Sometimes reports are > perplexing and/or this logic may return a wrong result for some > reason. The total object size may be useful to understand harder > cases. > Ok. I will keep the cache name and the total object_size. > > + } > > + > > + return cache->object_size; > > +} > > + > > static const char *get_shadow_bug_type(struct kasan_report_info > > *info) > > { > > const char *bug_type = "unknown-crash";
