[PATCH 1/2] net: Disable false positive memory leak report

[PATCH 1/2] net: Disable false positive memory leak report

[Huajun Li]

Memory leak detector reports following false positive memory leak, the
patch disables it.

unreferenced object 0xffff880073a70000 (size 8192):
  comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff8124db64>] create_object+0x144/0x360
    [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
    [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
    [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
    [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
    [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
    [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
    [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff880073a74290 (size 8192):
  comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff8124db64>] create_object+0x144/0x360
    [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
    [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
    [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
    [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
    [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
    [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
    [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
    [<ffffffffffffffff>] 0xffffffffffffffff


Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
---
 net/core/flow.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/net/core/flow.c b/net/core/flow.c
index 555a456..ba3e617 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -365,6 +365,13 @@ static int __cpuinit
flow_cache_cpu_prepare(struct flow_cache *fc, int cpu)

 	if (!fcp->hash_table) {
 		fcp->hash_table = kzalloc_node(sz, GFP_KERNEL, cpu_to_node(cpu));
+		/*
+		 * Avoid a kmemleak false positive. The pointer to this block
+		 * is refferenced by per-CPU varaible, here just mark it as not
+		 * being a leak.
+		 */
+		kmemleak_not_leak(fcp->hash_table);
+
 		if (!fcp->hash_table) {
 			pr_err("NET: failed to allocate flow cache sz %zu\n", sz);
 			return -ENOMEM;
-- 
1.7.4.1

Re: [PATCH 1/2] net: Disable false positive memory leak report

[Eric Dumazet]

Le samedi 24 septembre 2011 à 23:57 +0800, Huajun Li a écrit :
> Memory leak detector reports following false positive memory leak, the
> patch disables it.
> 
> unreferenced object 0xffff880073a70000 (size 8192):
>   comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff8124db64>] create_object+0x144/0x360
>     [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
>     [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
>     [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
>     [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
>     [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
>     [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
>     [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffff880073a74290 (size 8192):
>   comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff8124db64>] create_object+0x144/0x360
>     [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
>     [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
>     [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
>     [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
>     [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
>     [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
>     [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
>     [<ffffffffffffffff>] 0xffffffffffffffff
> 
> 
> Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
> ---
>  net/core/flow.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/flow.c b/net/core/flow.c
> index 555a456..ba3e617 100644
> --- a/net/core/flow.c
> +++ b/net/core/flow.c
> @@ -365,6 +365,13 @@ static int __cpuinit
> flow_cache_cpu_prepare(struct flow_cache *fc, int cpu)
> 
>  	if (!fcp->hash_table) {
>  		fcp->hash_table = kzalloc_node(sz, GFP_KERNEL, cpu_to_node(cpu));
> +		/*
> +		 * Avoid a kmemleak false positive. The pointer to this block
> +		 * is refferenced by per-CPU varaible, here just mark it as not
> +		 * being a leak.
> +		 */
> +		kmemleak_not_leak(fcp->hash_table);
> +
>  		if (!fcp->hash_table) {
>  			pr_err("NET: failed to allocate flow cache sz %zu\n", sz);
>  			return -ENOMEM;

This makes no sense to me.

per-cpu variables are taken into account by kmemleak.

If not, you should report this problem to kmemleak maintainer.

Re: [PATCH 1/2] net: Disable false positive memory leak report

[Huajun Li]

Eric, thanks for your response. :)

2011/9/26 Eric Dumazet <eric.dumazet@gmail.com>:
> Le samedi 24 septembre 2011 à 23:57 +0800, Huajun Li a écrit :
>> Memory leak detector reports following false positive memory leak, the
>> patch disables it.
>>
>> unreferenced object 0xffff880073a70000 (size 8192):
>>   comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
>>   hex dump (first 32 bytes):
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<ffffffff8124db64>] create_object+0x144/0x360
>>     [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
>>     [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
>>     [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
>>     [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
>>     [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
>>     [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
>>     [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
>>     [<ffffffffffffffff>] 0xffffffffffffffff
>> unreferenced object 0xffff880073a74290 (size 8192):
>>   comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
>>   hex dump (first 32 bytes):
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<ffffffff8124db64>] create_object+0x144/0x360
>>     [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
>>     [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
>>     [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
>>     [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
>>     [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
>>     [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
>>     [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
>>     [<ffffffffffffffff>] 0xffffffffffffffff
>>
>>
>> Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
>> ---
>>  net/core/flow.c |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/core/flow.c b/net/core/flow.c
>> index 555a456..ba3e617 100644
>> --- a/net/core/flow.c
>> +++ b/net/core/flow.c
>> @@ -365,6 +365,13 @@ static int __cpuinit
>> flow_cache_cpu_prepare(struct flow_cache *fc, int cpu)
>>
>>       if (!fcp->hash_table) {
>>               fcp->hash_table = kzalloc_node(sz, GFP_KERNEL, cpu_to_node(cpu));
>> +             /*
>> +              * Avoid a kmemleak false positive. The pointer to this block
>> +              * is refferenced by per-CPU varaible, here just mark it as not
>> +              * being a leak.
>> +              */
>> +             kmemleak_not_leak(fcp->hash_table);
>> +
>>               if (!fcp->hash_table) {
>>                       pr_err("NET: failed to allocate flow cache sz %zu\n", sz);
>>                       return -ENOMEM;
>
> This makes no sense to me.
>
> per-cpu variables are taken into account by kmemleak.
>

Yes, I think so.

> If not, you should report this problem to kmemleak maintainer.
>

To deal with false positives/negatives, in kmemleak document, it says:
...
The false positives are objects wrongly reported as being memory leaks
(orphan). For objects known not to be leaks, kmemleak provides the
kmemleak_not_leak function. The kmemleak_ignore could also be used if
the memory block is known not to contain other pointers and it will no
longer be scanned.
...

For some scenarios, _maybe_ it's hard for kmemleak to give report
exactly, and I think it is known issue for the maintainers, so hope
there will be a powerful version soon. :)
However, before the new version of kmemleak(if there is new version)
comes, is it possible to disable this false positive report as other
components? You know, other guys maybe also check the code once they
meet the report again just like me.

Re: [PATCH 1/2] net: Disable false positive memory leak report

[Eric Dumazet]

Le lundi 26 septembre 2011 à 16:42 +0800, Huajun Li a écrit :
> Eric, thanks for your response. :)
> 
> 2011/9/26 Eric Dumazet <eric.dumazet@gmail.com>:

> > This makes no sense to me.
> >
> > per-cpu variables are taken into account by kmemleak.
> >
> 
> Yes, I think so.
> 
> > If not, you should report this problem to kmemleak maintainer.
> >
> 
> To deal with false positives/negatives, in kmemleak document, it says:
> ...
> The false positives are objects wrongly reported as being memory leaks
> (orphan). For objects known not to be leaks, kmemleak provides the
> kmemleak_not_leak function. The kmemleak_ignore could also be used if
> the memory block is known not to contain other pointers and it will no
> longer be scanned.
> ...
> 

But this interface should be only used in very specific situations, not
for transient kmemleak reports.

(When the pointer to allocated block is not stored in memory, or only
stored in the object itself : A detached area of memory)

AFAIK, its almost never used :

# git grep kmemleak_not_leak
Documentation/kmemleak.txt:kmemleak_not_leak     - mark an object as not a leak
Documentation/kmemleak.txt:kmemleak_not_leak function. The kmemleak_ignore could also be used if
arch/sparc/kernel/irq_64.c:     kmemleak_not_leak(bucket);
fs/block_dev.c: kmemleak_not_leak(bd_mnt);
fs/nfs/dir.c:   kmemleak_not_leak(string->name);
include/linux/kmemleak.h:extern void kmemleak_not_leak(const void *ptr) __ref;
include/linux/kmemleak.h:static inline void kmemleak_not_leak(const void *ptr)
kernel/module.c:        kmemleak_not_leak(ptr);
mm/kmemleak.c: * kmemleak_not_leak - mark an allocated object as false positive
mm/kmemleak.c:void __ref kmemleak_not_leak(const void *ptr)
mm/kmemleak.c:EXPORT_SYMBOL(kmemleak_not_leak);
mm/kmemleak.c:                  kmemleak_not_leak(log->ptr);
mm/page_cgroup.c:       kmemleak_not_leak(base);


percpu variables are not specific these days, and kmemleak definitely
knows about them.

> For some scenarios, _maybe_ it's hard for kmemleak to give report
> exactly, and I think it is known issue for the maintainers, so hope
> there will be a powerful version soon. :)
> However, before the new version of kmemleak(if there is new version)
> comes, is it possible to disable this false positive report as other
> components? You know, other guys maybe also check the code once they
> meet the report again just like me.

We are not going to add patches to work around a core (kmemleak) issue,
especially without kmemleak maintainer feedback.