Re: [PATCH v2] module: check if memory leak by module.

[Michal Hocko]

On Wed 29-03-17 11:32:02, Maninder Singh wrote:
> This patch checks if any module which is going to be unloaded
> is doing vmalloc memory leak or not.

Hmm, how can you track _all_ vmalloc allocations done on behalf of the
module? It is quite some time since I've checked kernel/module.c but
from my vague understading your check is basically only about statically
vmalloced areas by module loader. Is that correct? If yes then is this
actually useful? Were there any bugs in the loader code recently? What
led you to prepare this patch? All this should be part of the changelog!
 
> Logs:-
> [  129.336368] Module [test_module] is getting unloaded before doing vfree
> [  129.336371] Memory still allocated: addr:0xffffc90001461000 - 0xffffc900014c7000, pages 101
> [  129.336376] Allocating function kernel_init+0x1c/0x20 [test_module]
> 
> Signed-off-by: Vaneet Narang <v.narang@xxxxxxxxxxx>
> Signed-off-by: Maninder Singh <maninder1.s@xxxxxxxxxxx>
> ---
> v1->v2: made code generic rather than dependent on config.
> 	changed pr_alert to pr_err.
> 
>  include/linux/vmalloc.h |  2 ++
>  kernel/module.c         | 22 ++++++++++++++++++++++
>  mm/vmalloc.c            |  2 --
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 46991ad..5531af3 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -29,6 +29,8 @@
>  #define IOREMAP_MAX_ORDER	(7 + PAGE_SHIFT)	/* 128 pages */
>  #endif
>  
> +#define VM_VM_AREA  0x04
> +
>  struct vm_struct {
>  	struct vm_struct	*next;
>  	void			*addr;
> diff --git a/kernel/module.c b/kernel/module.c
> index f953df9..98a8018 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2117,9 +2117,31 @@ void __weak module_arch_freeing_init(struct module *mod)
>  {
>  }
>  
> +static void check_memory_leak(struct module *mod)
> +{
> +	struct vmap_area *va;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(va, &vmap_area_list, list) {
> +		if (!(va->flags & VM_VM_AREA))
> +			continue;
> +		if ((mod->core_layout.base < va->vm->caller) &&
> +			(mod->core_layout.base + mod->core_layout.size) > va->vm->caller) {
> +			pr_err("Module [%s] is getting unloaded before doing vfree\n", mod->name);
> +			pr_err("Memory still allocated: addr:0x%lx - 0x%lx, pages %u\n",
> +				va->va_start, va->va_end, va->vm->nr_pages);
> +			pr_err("Allocating function %pS\n", va->vm->caller);
> +		}
> +
> +	}
> +	rcu_read_unlock();
> +}
> +
>  /* Free a module, remove from lists, etc. */
>  static void free_module(struct module *mod)
>  {
> +	check_memory_leak(mod);
> +
>  	trace_module_free(mod);
>  
>  	mod_sysfs_teardown(mod);
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 68eb002..0166a0a 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -314,8 +314,6 @@ unsigned long vmalloc_to_pfn(const void *vmalloc_addr)
>  
>  /*** Global kva allocator ***/
>  
> -#define VM_VM_AREA	0x04
> -
>  static DEFINE_SPINLOCK(vmap_area_lock);
>  /* Export for kexec only */
>  LIST_HEAD(vmap_area_list);
> -- 
> 1.9.1
> 

-- 
Michal Hocko
SUSE Labs