Re: [PATCH v2] mm: Delay kmemleak object creation of module_alloc()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux