linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net/core/neighbour: tell kmemleak about hash tables
@ 2019-01-08  9:30 Konstantin Khlebnikov
  2019-01-08 11:59 ` Eric Dumazet
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Konstantin Khlebnikov @ 2019-01-08  9:30 UTC (permalink / raw)
  To: netdev, David S. Miller; +Cc: Catalin Marinas, linux-kernel

This fixes false-positive kmemleak reports about leaked neighbour entries:

unreferenced object 0xffff8885c6e4d0a8 (size 1024):
  comm "softirq", pid 0, jiffies 4294922664 (age 167640.804s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 20 2c f3 83 ff ff ff ff  ........ ,......
    08 c0 ef 5f 84 88 ff ff 01 8c 7d 02 01 00 00 00  ..._......}.....
  backtrace:
    [<00000000748509fe>] ip6_finish_output2+0x887/0x1e40
    [<0000000036d7a0d8>] ip6_output+0x1ba/0x600
    [<0000000027ea7dba>] ip6_send_skb+0x92/0x2f0
    [<00000000d6e2111d>] udp_v6_send_skb.isra.24+0x680/0x15e0
    [<000000000668a8be>] udpv6_sendmsg+0x18c9/0x27a0
    [<000000004bd5fa90>] sock_sendmsg+0xb3/0xf0
    [<000000008227b29f>] ___sys_sendmsg+0x745/0x8f0
    [<000000008698009d>] __sys_sendmsg+0xde/0x170
    [<00000000889dacf1>] do_syscall_64+0x9b/0x400
    [<0000000081cdb353>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
    [<000000005767ed39>] 0xffffffffffffffff

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 net/core/neighbour.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 763a7b08df67..3e27a779f288 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -18,6 +18,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/slab.h>
+#include <linux/kmemleak.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -443,12 +444,14 @@ static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
 	ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
 	if (!ret)
 		return NULL;
-	if (size <= PAGE_SIZE)
+	if (size <= PAGE_SIZE) {
 		buckets = kzalloc(size, GFP_ATOMIC);
-	else
+	} else {
 		buckets = (struct neighbour __rcu **)
 			  __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
 					   get_order(size));
+		kmemleak_alloc(buckets, size, 0, GFP_ATOMIC);
+	}
 	if (!buckets) {
 		kfree(ret);
 		return NULL;
@@ -468,10 +471,12 @@ static void neigh_hash_free_rcu(struct rcu_head *head)
 	size_t size = (1 << nht->hash_shift) * sizeof(struct neighbour *);
 	struct neighbour __rcu **buckets = nht->hash_buckets;
 
-	if (size <= PAGE_SIZE)
+	if (size <= PAGE_SIZE) {
 		kfree(buckets);
-	else
+	} else {
+		kmemleak_free(buckets);
 		free_pages((unsigned long)buckets, get_order(size));
+	}
 	kfree(nht);
 }
 


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] net/core/neighbour: tell kmemleak about hash tables
  2019-01-08  9:30 [PATCH] net/core/neighbour: tell kmemleak about hash tables Konstantin Khlebnikov
@ 2019-01-08 11:59 ` Eric Dumazet
  2019-01-08 12:56   ` Konstantin Khlebnikov
  2019-01-10 14:35 ` David Miller
  2019-01-10 18:09 ` Cong Wang
  2 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2019-01-08 11:59 UTC (permalink / raw)
  To: Konstantin Khlebnikov, netdev, David S. Miller
  Cc: Catalin Marinas, linux-kernel



On 01/08/2019 01:30 AM, Konstantin Khlebnikov wrote:
> This fixes false-positive kmemleak reports about leaked neighbour entries:
> 
> unreferenced object 0xffff8885c6e4d0a8 (size 1024):


size 1024 object : should have been allocated by kzalloc(), right ?

>   comm "softirq", pid 0, jiffies 4294922664 (age 167640.804s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 20 2c f3 83 ff ff ff ff  ........ ,......
>     08 c0 ef 5f 84 88 ff ff 01 8c 7d 02 01 00 00 00  ..._......}.....
>   backtrace:
>     [<00000000748509fe>] ip6_finish_output2+0x887/0x1e40
>     [<0000000036d7a0d8>] ip6_output+0x1ba/0x600
>     [<0000000027ea7dba>] ip6_send_skb+0x92/0x2f0
>     [<00000000d6e2111d>] udp_v6_send_skb.isra.24+0x680/0x15e0
>     [<000000000668a8be>] udpv6_sendmsg+0x18c9/0x27a0
>     [<000000004bd5fa90>] sock_sendmsg+0xb3/0xf0
>     [<000000008227b29f>] ___sys_sendmsg+0x745/0x8f0
>     [<000000008698009d>] __sys_sendmsg+0xde/0x170
>     [<00000000889dacf1>] do_syscall_64+0x9b/0x400
>     [<0000000081cdb353>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>     [<000000005767ed39>] 0xffffffffffffffff
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
>  net/core/neighbour.c |   13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 763a7b08df67..3e27a779f288 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -18,6 +18,7 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/slab.h>
> +#include <linux/kmemleak.h>
>  #include <linux/types.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> @@ -443,12 +444,14 @@ static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
>  	ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
>  	if (!ret)
>  		return NULL;
> -	if (size <= PAGE_SIZE)
> +	if (size <= PAGE_SIZE) {
>  		buckets = kzalloc(size, GFP_ATOMIC);
> -	else
> +	} else {
>  		buckets = (struct neighbour __rcu **)
>  			  __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
>  					   get_order(size));
> +		kmemleak_alloc(buckets, size, 0, GFP_ATOMIC);
> +	}
>  	if (!buckets) {
>  		kfree(ret);
>  		return NULL;
> @@ -468,10 +471,12 @@ static void neigh_hash_free_rcu(struct rcu_head *head)
>  	size_t size = (1 << nht->hash_shift) * sizeof(struct neighbour *);
>  	struct neighbour __rcu **buckets = nht->hash_buckets;
>  
> -	if (size <= PAGE_SIZE)
> +	if (size <= PAGE_SIZE) {
>  		kfree(buckets);
> -	else
> +	} else {
> +		kmemleak_free(buckets);
>  		free_pages((unsigned long)buckets, get_order(size));
> +	}
>  	kfree(nht);
>  }
>  
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net/core/neighbour: tell kmemleak about hash tables
  2019-01-08 11:59 ` Eric Dumazet
@ 2019-01-08 12:56   ` Konstantin Khlebnikov
  0 siblings, 0 replies; 7+ messages in thread
From: Konstantin Khlebnikov @ 2019-01-08 12:56 UTC (permalink / raw)
  To: Eric Dumazet, netdev, David S. Miller; +Cc: Catalin Marinas, linux-kernel



On 08.01.2019 14:59, Eric Dumazet wrote:
> 
> 
> On 01/08/2019 01:30 AM, Konstantin Khlebnikov wrote:
>> This fixes false-positive kmemleak reports about leaked neighbour entries:
>>
>> unreferenced object 0xffff8885c6e4d0a8 (size 1024):
> 
> 
> size 1024 object : should have been allocated by kzalloc(), right ?

Yep, neigh_alloc calls kzalloc.

Size might be bloated by lockdep.

> 
>>    comm "softirq", pid 0, jiffies 4294922664 (age 167640.804s)
>>    hex dump (first 32 bytes):
>>      00 00 00 00 00 00 00 00 20 2c f3 83 ff ff ff ff  ........ ,......
>>      08 c0 ef 5f 84 88 ff ff 01 8c 7d 02 01 00 00 00  ..._......}.....
>>    backtrace:
>>      [<00000000748509fe>] ip6_finish_output2+0x887/0x1e40
>>      [<0000000036d7a0d8>] ip6_output+0x1ba/0x600
>>      [<0000000027ea7dba>] ip6_send_skb+0x92/0x2f0
>>      [<00000000d6e2111d>] udp_v6_send_skb.isra.24+0x680/0x15e0
>>      [<000000000668a8be>] udpv6_sendmsg+0x18c9/0x27a0
>>      [<000000004bd5fa90>] sock_sendmsg+0xb3/0xf0
>>      [<000000008227b29f>] ___sys_sendmsg+0x745/0x8f0
>>      [<000000008698009d>] __sys_sendmsg+0xde/0x170
>>      [<00000000889dacf1>] do_syscall_64+0x9b/0x400
>>      [<0000000081cdb353>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>      [<000000005767ed39>] 0xffffffffffffffff
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> ---
>>   net/core/neighbour.c |   13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
>> index 763a7b08df67..3e27a779f288 100644
>> --- a/net/core/neighbour.c
>> +++ b/net/core/neighbour.c
>> @@ -18,6 +18,7 @@
>>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>   
>>   #include <linux/slab.h>
>> +#include <linux/kmemleak.h>
>>   #include <linux/types.h>
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>> @@ -443,12 +444,14 @@ static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
>>   	ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
>>   	if (!ret)
>>   		return NULL;
>> -	if (size <= PAGE_SIZE)
>> +	if (size <= PAGE_SIZE) {
>>   		buckets = kzalloc(size, GFP_ATOMIC);
>> -	else
>> +	} else {
>>   		buckets = (struct neighbour __rcu **)
>>   			  __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
>>   					   get_order(size));
>> +		kmemleak_alloc(buckets, size, 0, GFP_ATOMIC);
>> +	}
>>   	if (!buckets) {
>>   		kfree(ret);
>>   		return NULL;
>> @@ -468,10 +471,12 @@ static void neigh_hash_free_rcu(struct rcu_head *head)
>>   	size_t size = (1 << nht->hash_shift) * sizeof(struct neighbour *);
>>   	struct neighbour __rcu **buckets = nht->hash_buckets;
>>   
>> -	if (size <= PAGE_SIZE)
>> +	if (size <= PAGE_SIZE) {
>>   		kfree(buckets);
>> -	else
>> +	} else {
>> +		kmemleak_free(buckets);
>>   		free_pages((unsigned long)buckets, get_order(size));
>> +	}
>>   	kfree(nht);
>>   }
>>   
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net/core/neighbour: tell kmemleak about hash tables
  2019-01-08  9:30 [PATCH] net/core/neighbour: tell kmemleak about hash tables Konstantin Khlebnikov
  2019-01-08 11:59 ` Eric Dumazet
@ 2019-01-10 14:35 ` David Miller
  2019-01-10 18:09 ` Cong Wang
  2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2019-01-10 14:35 UTC (permalink / raw)
  To: khlebnikov; +Cc: netdev, catalin.marinas, linux-kernel

From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Tue, 08 Jan 2019 12:30:00 +0300

> This fixes false-positive kmemleak reports about leaked neighbour entries:
 ...
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

Applied.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net/core/neighbour: tell kmemleak about hash tables
  2019-01-08  9:30 [PATCH] net/core/neighbour: tell kmemleak about hash tables Konstantin Khlebnikov
  2019-01-08 11:59 ` Eric Dumazet
  2019-01-10 14:35 ` David Miller
@ 2019-01-10 18:09 ` Cong Wang
  2019-01-11  4:26   ` Konstantin Khlebnikov
  2 siblings, 1 reply; 7+ messages in thread
From: Cong Wang @ 2019-01-10 18:09 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Linux Kernel Network Developers, David S. Miller, Catalin Marinas, LKML

On Tue, Jan 8, 2019 at 1:30 AM Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> @@ -443,12 +444,14 @@ static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
>         ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
>         if (!ret)
>                 return NULL;
> -       if (size <= PAGE_SIZE)
> +       if (size <= PAGE_SIZE) {
>                 buckets = kzalloc(size, GFP_ATOMIC);
> -       else
> +       } else {
>                 buckets = (struct neighbour __rcu **)
>                           __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
>                                            get_order(size));
> +               kmemleak_alloc(buckets, size, 0, GFP_ATOMIC);

Why min_count is 0 rather than 1 here?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net/core/neighbour: tell kmemleak about hash tables
  2019-01-10 18:09 ` Cong Wang
@ 2019-01-11  4:26   ` Konstantin Khlebnikov
  2019-01-11 10:56     ` Catalin Marinas
  0 siblings, 1 reply; 7+ messages in thread
From: Konstantin Khlebnikov @ 2019-01-11  4:26 UTC (permalink / raw)
  To: Cong Wang
  Cc: Konstantin Khlebnikov, Linux Kernel Network Developers,
	David S. Miller, Catalin Marinas, LKML

On Thu, Jan 10, 2019 at 11:45 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Tue, Jan 8, 2019 at 1:30 AM Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
> > @@ -443,12 +444,14 @@ static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
> >         ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
> >         if (!ret)
> >                 return NULL;
> > -       if (size <= PAGE_SIZE)
> > +       if (size <= PAGE_SIZE) {
> >                 buckets = kzalloc(size, GFP_ATOMIC);
> > -       else
> > +       } else {
> >                 buckets = (struct neighbour __rcu **)
> >                           __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
> >                                            get_order(size));
> > +               kmemleak_alloc(buckets, size, 0, GFP_ATOMIC);
>
> Why min_count is 0 rather than 1 here?

The api isn't clear and I've misread description.
So it should be 1 for reporting leak of hash table itself.
But 0 doesn't add any new issues.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net/core/neighbour: tell kmemleak about hash tables
  2019-01-11  4:26   ` Konstantin Khlebnikov
@ 2019-01-11 10:56     ` Catalin Marinas
  0 siblings, 0 replies; 7+ messages in thread
From: Catalin Marinas @ 2019-01-11 10:56 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Cong Wang, Konstantin Khlebnikov,
	Linux Kernel Network Developers, David S. Miller, LKML

On Fri, Jan 11, 2019 at 07:26:09AM +0300, Konstantin Khlebnikov wrote:
> On Thu, Jan 10, 2019 at 11:45 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Tue, Jan 8, 2019 at 1:30 AM Konstantin Khlebnikov
> > <khlebnikov@yandex-team.ru> wrote:
> > > @@ -443,12 +444,14 @@ static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
> > >         ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
> > >         if (!ret)
> > >                 return NULL;
> > > -       if (size <= PAGE_SIZE)
> > > +       if (size <= PAGE_SIZE) {
> > >                 buckets = kzalloc(size, GFP_ATOMIC);
> > > -       else
> > > +       } else {
> > >                 buckets = (struct neighbour __rcu **)
> > >                           __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
> > >                                            get_order(size));
> > > +               kmemleak_alloc(buckets, size, 0, GFP_ATOMIC);
> >
> > Why min_count is 0 rather than 1 here?
> 
> The api isn't clear and I've misread description.
> So it should be 1 for reporting leak of hash table itself.
> But 0 doesn't add any new issues.

Correct. I'd say it makes sense to set it to 1 so that you'd have
similar behaviour to the kzalloc() allocation.


-- 
Catalin

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-11 10:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-08  9:30 [PATCH] net/core/neighbour: tell kmemleak about hash tables Konstantin Khlebnikov
2019-01-08 11:59 ` Eric Dumazet
2019-01-08 12:56   ` Konstantin Khlebnikov
2019-01-10 14:35 ` David Miller
2019-01-10 18:09 ` Cong Wang
2019-01-11  4:26   ` Konstantin Khlebnikov
2019-01-11 10:56     ` Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).