On Thu, Nov 25, 2021 at 8:52 AM 'Kefeng Wang' via kasan-dev <kasan-dev@xxxxxxxxxxxxxxxx> wrote: > > Yongqiang reports a kmemleak panic when module insmod/rmmod > with KASAN enabled(without KASAN_VMALLOC) on x86[1]. > > When the module area allocates memory, it's kmemleak_object > is created successfully, but the KASAN shadow memory of module > allocation is not ready, so when kmemleak scan the module's > pointer, it will panic due to no shadow memory with KASAN check. > > module_alloc > __vmalloc_node_range > kmemleak_vmalloc > kmemleak_scan > update_checksum > kasan_module_alloc > kmemleak_ignore > > Note, there is no problem if KASAN_VMALLOC enabled, the modules > area entire shadow memory is preallocated. Thus, the bug only > exits on ARCH which supports dynamic allocation of module area > per module load, for now, only x86/arm64/s390 are involved. > > > Add a VM_DEFER_KMEMLEAK flags, defer vmalloc'ed object register > of kmemleak in module_alloc() to fix this issue. > > [1] https://lore.kernel.org/all/6d41e2b9-4692-5ec4-b1cd-cbe29ae89739@xxxxxxxxxx/ > > Fixes: 793213a82de4 ("s390/kasan: dynamic shadow mem allocation for modules") > Fixes: 39d114ddc682 ("arm64: add KASAN support") > Fixes: bebf56a1b176 ("kasan: enable instrumentation of global variables") > Reported-by: Yongqiang Liu <liuyongqiang13@xxxxxxxxxx> > Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> > --- > V4: > - add fix tag > - fix missing change about VM_DELAY_KMEMLEAK > v3: > - update changelog to add more explanation > - use DEFER instead of DELAY sugguested by Catalin. > v2: > - fix type error on changelog and kasan_module_alloc() > > arch/arm64/kernel/module.c | 4 ++-- > arch/s390/kernel/module.c | 5 +++-- > arch/x86/kernel/module.c | 7 ++++--- > include/linux/kasan.h | 4 ++-- > include/linux/vmalloc.h | 7 +++++++ > mm/kasan/shadow.c | 9 +++++++-- > mm/vmalloc.c | 3 ++- > 7 files changed, 27 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c > index b5ec010c481f..309a27553c87 100644 > --- a/arch/arm64/kernel/module.c > +++ b/arch/arm64/kernel/module.c > @@ -36,7 +36,7 @@ void *module_alloc(unsigned long size) > module_alloc_end = MODULES_END; > > p = __vmalloc_node_range(size, MODULE_ALIGN, module_alloc_base, > - module_alloc_end, gfp_mask, PAGE_KERNEL, 0, > + module_alloc_end, gfp_mask, PAGE_KERNEL, VM_DEFER_KMEMLEAK, > NUMA_NO_NODE, __builtin_return_address(0)); > > if (!p && IS_ENABLED(CONFIG_ARM64_MODULE_PLTS) && > @@ -58,7 +58,7 @@ void *module_alloc(unsigned long size) > PAGE_KERNEL, 0, NUMA_NO_NODE, > __builtin_return_address(0)); > > - if (p && (kasan_module_alloc(p, size) < 0)) { > + if (p && (kasan_module_alloc(p, size, gfp_mask) < 0)) { > vfree(p); > return NULL; > } > diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c > index b01ba460b7ca..d52d85367bf7 100644 > --- a/arch/s390/kernel/module.c > +++ b/arch/s390/kernel/module.c > @@ -37,14 +37,15 @@ > > void *module_alloc(unsigned long size) > { > + gfp_t gfp_mask = GFP_KERNEL; > void *p; > > if (PAGE_ALIGN(size) > MODULES_LEN) > return NULL; > p = __vmalloc_node_range(size, MODULE_ALIGN, MODULES_VADDR, MODULES_END, > - GFP_KERNEL, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE, > + gfp_mask, PAGE_KERNEL_EXEC, VM_DEFER_KMEMLEAK, NUMA_NO_NODE, > __builtin_return_address(0)); > - if (p && (kasan_module_alloc(p, size) < 0)) { > + if (p && (kasan_module_alloc(p, size, gfp_mask) < 0)) { > vfree(p); > return NULL; > } > diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c > index 169fb6f4cd2e..95fa745e310a 100644 > --- a/arch/x86/kernel/module.c > +++ b/arch/x86/kernel/module.c > @@ -67,6 +67,7 @@ static unsigned long int get_module_load_offset(void) > > void *module_alloc(unsigned long size) > { > + gfp_t gfp_mask = GFP_KERNEL; > void *p; > > if (PAGE_ALIGN(size) > MODULES_LEN) > @@ -74,10 +75,10 @@ void *module_alloc(unsigned long size) > > p = __vmalloc_node_range(size, MODULE_ALIGN, > MODULES_VADDR + get_module_load_offset(), > - MODULES_END, GFP_KERNEL, > - PAGE_KERNEL, 0, NUMA_NO_NODE, > + MODULES_END, gfp_mask, > + PAGE_KERNEL, VM_DEFER_KMEMLEAK, NUMA_NO_NODE, > __builtin_return_address(0)); > - if (p && (kasan_module_alloc(p, size) < 0)) { > + if (p && (kasan_module_alloc(p, size, gfp_mask) < 0)) { > vfree(p); > return NULL; > } > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > index d8783b682669..89c99e5e67de 100644 > --- a/include/linux/kasan.h > +++ b/include/linux/kasan.h > @@ -474,12 +474,12 @@ static inline void kasan_populate_early_vm_area_shadow(void *start, > * allocations with real shadow memory. With KASAN vmalloc, the special > * case is unnecessary, as the work is handled in the generic case. > */ > -int kasan_module_alloc(void *addr, size_t size); > +int kasan_module_alloc(void *addr, size_t size, gfp_t gfp_mask); > void kasan_free_shadow(const struct vm_struct *vm); > > #else /* (CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS) && !CONFIG_KASAN_VMALLOC */ > > -static inline int kasan_module_alloc(void *addr, size_t size) { return 0; } > +static inline int kasan_module_alloc(void *addr, size_t size, gfp_t gfp_mask) { return 0; } > static inline void kasan_free_shadow(const struct vm_struct *vm) {} > > #endif /* (CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS) && !CONFIG_KASAN_VMALLOC */ > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h > index 6e022cc712e6..506fc6e6a126 100644 > --- a/include/linux/vmalloc.h > +++ b/include/linux/vmalloc.h > @@ -28,6 +28,13 @@ struct notifier_block; /* in notifier.h */ > #define VM_MAP_PUT_PAGES 0x00000200 /* put pages and free array in vfree */ > #define VM_NO_HUGE_VMAP 0x00000400 /* force PAGE_SIZE pte mapping */ > > +#if defined(CONFIG_KASAN) && (defined(CONFIG_KASAN_GENERIC) || \ > + defined(CONFIG_KASAN_SW_TAGS)) && !defined(CONFIG_KASAN_VMALLOC) > +#define VM_DEFER_KMEMLEAK 0x00000800 /* defer kmemleak object creation */ > +#else > +#define VM_DEFER_KMEMLEAK 0 > +#endif No need for CONFIG_KASAN check: CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS implies CONFIG_KASAN. > + > /* > * VM_KASAN is used slightly differently depending on CONFIG_KASAN_VMALLOC. > * > diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c > index 4a4929b29a23..2ade2f484562 100644 > --- a/mm/kasan/shadow.c > +++ b/mm/kasan/shadow.c > @@ -498,7 +498,7 @@ void kasan_release_vmalloc(unsigned long start, unsigned long end, > > #else /* CONFIG_KASAN_VMALLOC */ > > -int kasan_module_alloc(void *addr, size_t size) > +int kasan_module_alloc(void *addr, size_t size, gfp_t gfp_mask) > { > void *ret; > size_t scaled_size; > @@ -520,9 +520,14 @@ int kasan_module_alloc(void *addr, size_t size) > __builtin_return_address(0)); > > if (ret) { > + struct vm_struct *vm = find_vm_area(addr); > __memset(ret, KASAN_SHADOW_INIT, shadow_size); > - find_vm_area(addr)->flags |= VM_KASAN; > + vm->flags |= VM_KASAN; > kmemleak_ignore(ret); > + > + if (vm->flags & VM_DEFER_KMEMLEAK) > + kmemleak_vmalloc(vm, size, gfp_mask); > + > return 0; > } > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index d2a00ad4e1dd..bf3c2fe8f528 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -3074,7 +3074,8 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align, > clear_vm_uninitialized_flag(area); > > size = PAGE_ALIGN(size); > - kmemleak_vmalloc(area, size, gfp_mask); > + if (!(vm_flags & VM_DEFER_KMEMLEAK)) > + kmemleak_vmalloc(area, size, gfp_mask); > > return addr; > > -- > 2.26.2 > > -- > 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@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20211125080307.27225-1-wangkefeng.wang%40huawei.com.
