linux-kernel - Re: [PATCH 1/1] module: check if memory leak by module.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 27 Mar 2017 08:43:25 +0200
From:   Pavel Machek <pavel@....cz>
To:     Maninder Singh <maninder1.s@...sung.com>
Cc:     jeyu@...hat.com, rusty@...tcorp.com.au, akpm@...ux-foundation.org,
        chris@...is-wilson.co.uk, aryabinin@...tuozzo.com,
        joonas.lahtinen@...ux.intel.com, mhocko@...e.com,
        keescook@...omium.org, jinb.park7@...il.com, anisse@...ier.eu,
        rafael.j.wysocki@...el.com, zijun_hu@....com, mingo@...nel.org,
        mawilcox@...rosoft.com, thgarnie@...gle.com, joelaf@...gle.com,
        kirill.shutemov@...ux.intel.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, pankaj.m@...sung.com,
        ajeet.y@...sung.com, hakbong5.lee@...sung.com,
        a.sahrawat@...sung.com, lalit.mohan@...sung.com, cpgs@...sung.com,
        Vaneet Narang <v.narang@...sung.com>
Subject: Re: [PATCH 1/1] module: check if memory leak by module.

Hi!

> This patch adds new config VMALLOC_MEMORY_LEAK to check if any
> module which is going to be unloaded is doing vmalloc memory leak.
> 
> Logs:-
> [  129.336368] Module vmalloc is getting unloaded before doing vfree
> [  129.336371] Memory still allocated: addr:0xffffc90001461000 - 0xffffc900014c7000, pages 101
> [  129.336376] Allocating function kernel_init+0x1c/0x20 [vmalloc]
> 
> Signed-off-by: Maninder Singh <maninder1.s@...sung.com>
> Signed-off-by: Vaneet Narang <v.narang@...sung.com>

Let me see...

> diff --git a/kernel/module.c b/kernel/module.c
> index 529efae..b492f34 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2082,9 +2082,37 @@ void __weak module_arch_freeing_init(struct module *mod)
>  {
>  }
>  
> +#ifdef CONFIG_VMALLOC_MEMORY_LEAK

I'd not make this optional -- the performance cost is not all that
big, right?

> +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) {

Two spaces after "+".

> +			pr_alert("Module %s is getting unloaded before doing vfree\n", mod->name);
> +			pr_alert("Memory still allocated: addr:0x%lx - 0x%lx, pages %u\n",
> +				va->va_start, va->va_end, va->vm->nr_pages);
> +			pr_alert("Allocating function %pS\n", va->vm->caller);
> +		}

Plain pr_err() would be preffered. Its just a memory leak.

Otherwise looks good to me..
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)