On 2021/11/24 3:44, Catalin Marinas wrote:
On Tue, Nov 23, 2021 at 10:32:20PM +0800, Kefeng Wang wrote:
Yongqiang reports a kmemleak panic when module insmod/rmmod with KASAN
enabled on x86[1].
When the module 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.
module_alloc
__vmalloc_node_range
kmemleak_vmalloc
kmemleak_scan
update_checksum
kasan_module_alloc
kmemleak_ignore
Can you share the .config and the stack trace you get on arm64?
My gcc could not support CC_HAS_KASAN_SW_TAGS, so no repoduced on ARM64
I have a suspicion there is no problem if KASAN_VMALLOC is enabled.
Yes, if KASAN_VMALLOC enabled, the memory of vmalloc shadow has been
populated in arch's kasan_init(),
there is no issue. but x86/arm64/s390 support dynamic allocation of
module area per module load by
kasan_module_alloc(), and this leads the above concurrent issue.
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_DELAY_KMEMLEAK)
+ kmemleak_vmalloc(vm, size, gfp_mask);
+
return 0;
}
This function only exists if CONFIG_KASAN_VMALLOC=n.
yes.
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d2a00ad4e1dd..23c595b15839 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_DELAY_KMEMLEAK))
+ kmemleak_vmalloc(area, size, gfp_mask);
So with KASAN_VMALLOC enabled, we'll miss the kmemleak allocation.
See the definination, if KASAN_VMALLOC enabled, VM_DELAY_KMEMLEAK is 0,
so kmemleak allocation
still works.
+#if defined(CONFIG_KASAN) && (defined(CONFIG_KASAN_GENERIC) || \
+ defined(CONFIG_KASAN_SW_TAGS)) && !defined(CONFIG_KASAN_VMALLOC)
+#define VM_DELAY_KMEMLEAK 0x00000800 /* delay kmemleak object create */
+#else
+#define VM_DELAY_KMEMLEAK 0
+#endif
+
You could add an IS_ENABLED(CONFIG_KASAN_VMALLOC) check but I'm not
particularly fond of the delay approach (also think DEFER is probably a
better name).
Will use DEFER instead.
A quick fix would be to make KMEMLEAK depend on !KASAN || KASAN_VMALLOC.
We'll miss KASAN_SW_TAGS with kmemleak but I think vmalloc support could
be enabled for this as well.
What does KASAN do with other vmalloc() allocations when !KASAN_VMALLOC?
Can we not have a similar approach. I don't fully understand why the
module vmalloc() is a special case.
Only the shadow of module area is dynamic allocation , this exists on
ARM64 too.
if no KASAN_VMALLOC, no shadow area for vmalloc, other vmalloc
allocation is
no problem. Correct me if I'm wrong.
Thanks.
