Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1642088 > unrolled thread

Re: kmemleak splat on copy_process()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: kmemleak splat on copy_process() "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-16 00:00 +0200
    Re: kmemleak splat on copy_process() Michal Hocko <mhocko@kernel.org> - 2017-05-16 08:30 +0200
      Re: kmemleak splat on copy_process() Catalin Marinas <catalin.marinas@arm.com> - 2017-05-16 15:40 +0200
        Re: kmemleak splat on copy_process() Andy Lutomirski <luto@kernel.org> - 2017-05-17 02:00 +0200
          Re: kmemleak splat on copy_process() Catalin Marinas <catalin.marinas@arm.com> - 2017-05-17 13:10 +0200

#1642088 — Re: kmemleak splat on copy_process()

On Fri, Feb 17, 2017 at 11:32:34AM -0800, Andy Lutomirski wrote:
> On Fri, Feb 17, 2017 at 9:23 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > On Fri, Feb 17, 2017 at 9:07 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> >> But maybe
> >> there really is a race in which a kmemleak check right in the middle
> >> of duplicating the task struct really can't see the stack pointer.
> >
> > Funny, but it was actually using kmemleak how I can easily reproduce:
> >
> > To reproduce the kmemleak splat:
> >
> > echo clear >  /sys/kernel/debug/kmemleak
> > echo scan >  /sys/kernel/debug/kmemleak
> > cat /sys/kernel/debug/kmemleak
> 
> Worked fine for me.  Maybe your config is special?

I don't think my config is special at all, here it is its just
what I use for my qemu kvm guest image:

http://drvbp1.linux-foundation.org/~mcgrof/2017/05/15/configs/piggy-x86_64_qemu_fork_kmemleak.config

Another new kernel (next-20170515 based now on v4.12-rc1) and yet the same
kmemleeak can be triggered easily, although this time I need to hit "scan"
quite a bit more -- try using scan 6 times in a row or so.

echo clear >  /sys/kernel/debug/kmemleak
echo scan >  /sys/kernel/debug/kmemleak
echo scan >  /sys/kernel/debug/kmemleak
echo scan >  /sys/kernel/debug/kmemleak
echo scan >  /sys/kernel/debug/kmemleak
echo scan >  /sys/kernel/debug/kmemleak
cat /sys/kernel/debug/kmemleak

root@piggy:~# cat /sys/kernel/debug/kmemleak
unreferenced object 0xffffa07500d4c000 (size 16384):
  comm "bash", pid 1349, jiffies 4294895999 (age 263.204s)
  hex dump (first 32 bytes):
    9d 6e ac 57 00 00 00 00 00 00 00 00 00 00 00 00  .n.W............
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
    [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
    [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
    [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
    [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
    [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
    [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffffa07500c30000 (size 16384):
  comm "bash", pid 1394, jiffies 4294940106 (age 86.780s)
  hex dump (first 32 bytes):
    9d 6e ac 57 00 00 00 00 00 00 00 00 00 00 00 00  .n.W............
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
    [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
    [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
    [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
    [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
    [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
    [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffffa07500c98000 (size 16384):
  comm "bash", pid 1368, jiffies 4294956480 (age 21.284s)
  hex dump (first 32 bytes):
    9d 6e ac 57 00 00 00 00 00 00 00 00 fe 01 00 00  .n.W............
    c0 f4 9d 44 2f e8 ff ff 00 de ac 44 2f e8 ff ff  ...D/......D/...
  backtrace:  
    [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
    [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
    [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
    [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
    [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
    [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
    [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff

I confirm that stack is allocated and that a respective tsk->stack_vm_area gets
assigned. So neither of these BUG() triggers, for instance but yet the kmemleak
does:

diff --git a/kernel/fork.c b/kernel/fork.c
index 657373b2ddd2..9bd7ccd55b89 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -230,8 +230,12 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)
 	 * free_thread_stack() can be called in interrupt context,
 	 * so cache the vm_struct.
 	 */
-	if (stack)
+	if (stack) {
 		tsk->stack_vm_area = find_vm_area(stack);
+		if (!tsk->stack_vm_area)
+			BUG();
+	} else
+		BUG();
 	return stack;
 #else
 	struct page *page = alloc_pages_node(node, THREADINFO_GFP,



  Luis

[toc] | [next] | [standalone]

#1642242

Let's CC Catalin

this smells like a kmemleak bug to me.

On Mon 15-05-17 23:53:18, Luis R. Rodriguez wrote:
> On Fri, Feb 17, 2017 at 11:32:34AM -0800, Andy Lutomirski wrote:
> > On Fri, Feb 17, 2017 at 9:23 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > > On Fri, Feb 17, 2017 at 9:07 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> > >> But maybe
> > >> there really is a race in which a kmemleak check right in the middle
> > >> of duplicating the task struct really can't see the stack pointer.
> > >
> > > Funny, but it was actually using kmemleak how I can easily reproduce:
> > >
> > > To reproduce the kmemleak splat:
> > >
> > > echo clear >  /sys/kernel/debug/kmemleak
> > > echo scan >  /sys/kernel/debug/kmemleak
> > > cat /sys/kernel/debug/kmemleak
> > 
> > Worked fine for me.  Maybe your config is special?
> 
> I don't think my config is special at all, here it is its just
> what I use for my qemu kvm guest image:
> 
> http://drvbp1.linux-foundation.org/~mcgrof/2017/05/15/configs/piggy-x86_64_qemu_fork_kmemleak.config
> 
> Another new kernel (next-20170515 based now on v4.12-rc1) and yet the same
> kmemleeak can be triggered easily, although this time I need to hit "scan"
> quite a bit more -- try using scan 6 times in a row or so.
> 
> echo clear >  /sys/kernel/debug/kmemleak
> echo scan >  /sys/kernel/debug/kmemleak
> echo scan >  /sys/kernel/debug/kmemleak
> echo scan >  /sys/kernel/debug/kmemleak
> echo scan >  /sys/kernel/debug/kmemleak
> echo scan >  /sys/kernel/debug/kmemleak
> cat /sys/kernel/debug/kmemleak
> 
> root@piggy:~# cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffffa07500d4c000 (size 16384):
>   comm "bash", pid 1349, jiffies 4294895999 (age 263.204s)
>   hex dump (first 32 bytes):
>     9d 6e ac 57 00 00 00 00 00 00 00 00 00 00 00 00  .n.W............
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
>     [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
>     [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
>     [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
>     [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
>     [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffffa07500c30000 (size 16384):
>   comm "bash", pid 1394, jiffies 4294940106 (age 86.780s)
>   hex dump (first 32 bytes):
>     9d 6e ac 57 00 00 00 00 00 00 00 00 00 00 00 00  .n.W............
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
>     [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
>     [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
>     [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
>     [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
>     [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffffa07500c98000 (size 16384):
>   comm "bash", pid 1368, jiffies 4294956480 (age 21.284s)
>   hex dump (first 32 bytes):
>     9d 6e ac 57 00 00 00 00 00 00 00 00 fe 01 00 00  .n.W............
>     c0 f4 9d 44 2f e8 ff ff 00 de ac 44 2f e8 ff ff  ...D/......D/...
>   backtrace:  
>     [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
>     [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
>     [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
>     [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
>     [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
>     [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> 
> I confirm that stack is allocated and that a respective tsk->stack_vm_area gets
> assigned. So neither of these BUG() triggers, for instance but yet the kmemleak
> does:
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 657373b2ddd2..9bd7ccd55b89 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -230,8 +230,12 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)
>  	 * free_thread_stack() can be called in interrupt context,
>  	 * so cache the vm_struct.
>  	 */
> -	if (stack)
> +	if (stack) {
>  		tsk->stack_vm_area = find_vm_area(stack);
> +		if (!tsk->stack_vm_area)
> +			BUG();
> +	} else
> +		BUG();
>  	return stack;
>  #else
>  	struct page *page = alloc_pages_node(node, THREADINFO_GFP,
> 
> 
> 
>   Luis

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1642565

Thanks for cc'ing me. The vmalloc allocations have always been tricky
for kmemleak since there are 2-3 other memory locations with the same
value as the vmalloc'ed object: vm_struct.addr and vmap_area.va_start;
occasionally we have vmap_area.va_end pointing to the next
vmap_area.va_start.

To have a more reliable object reference counting, kmemleak avoids
scanning most of the vmap_area structure (apart from vmap_area.vm) and
requires a minimum count of 2 references for a vmalloc'ed object to not
be considered a leak (that is vm_struct.addr and the caller's location,
in this case task->stack).

> On Mon 15-05-17 23:53:18, Luis R. Rodriguez wrote:
> > root@piggy:~# cat /sys/kernel/debug/kmemleak
> > unreferenced object 0xffffa07500d4c000 (size 16384):
> >   comm "bash", pid 1349, jiffies 4294895999 (age 263.204s)
> >   hex dump (first 32 bytes):
> >     9d 6e ac 57 00 00 00 00 00 00 00 00 00 00 00 00  .n.W............
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
> >     [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
> >     [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
> >     [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
> >     [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
> >     [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
> >     [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
> >     [<ffffffffffffffff>] 0xffffffffffffffff

This stack here was allocated by bash for a child process via
alloc_thread_stack_node() when CONFIG_VMAP_STACK is enabled. When the
child died, free_stack_thread() did not call vfree_atomic() but rather
stored the corresponding vm_struct pointer in cached_stacks[i]. However,
the original task->stack pointer was lost (child task_struct freed), so
when scanning the memory kmemleak can only find a single reference to
the original stack (via vm_struct cached_stacks[i]) rather than 2 as
required for vmalloc'ed objects.

BTW, you can get more info about a specific object, including the number
of references found, with:

  echo dump=0xffffa07500d4c000 > /sys/kernel/debug/kmemleak

So basically kernel/fork.c has its own thread stack allocator on top of
vmalloc and kmemleak gets confused. A quick workaround:

------------8<------------------------------
diff --git a/kernel/fork.c b/kernel/fork.c
index 06d759ab4c62..785907fccf67 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -214,6 +214,7 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)
 		this_cpu_write(cached_stacks[i], NULL);
 
 		tsk->stack_vm_area = s;
+		kmemleak_alloc(s->addr, THREAD_SIZE, 2, GFP_ATOMIC);
 		local_irq_enable();
 		return s->addr;
 	}
@@ -254,6 +255,7 @@ static inline void free_thread_stack(struct task_struct *tsk)
 				continue;
 
 			this_cpu_write(cached_stacks[i], tsk->stack_vm_area);
+			kmemleak_free(tsk->stack);
 			local_irq_restore(flags);
 			return;
 		}
------------8<------------------------------

A better solution I think would be something like kmemleak_ignore() on
the free path paired with a new kmemleak_unignore() in order to avoid
the freeing/re-allocation of the kmemleak metadata.

A more complex solution (which I'm not yet convinced it works) may be to
pass the number of references of an objects down to the objects it
refers. In the vmalloc case we normally have a single reference to
vm_struct and two to the vmalloc'ed object. If a reference to the
vmalloc'ed object disappeared we could balance it with an increased
number of references to the vm_struct object. But this option requires
more research.

Unless there are better ideas, I'll post a patch adding
kmemleak_ignore/unignore annotations to kerne/fork.c (and the
corresponding mm/kmemleak.c changes).

Thanks.

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1642898

On Tue, May 16, 2017 at 6:39 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> Thanks for cc'ing me. The vmalloc allocations have always been tricky
> for kmemleak since there are 2-3 other memory locations with the same
> value as the vmalloc'ed object: vm_struct.addr and vmap_area.va_start;
> occasionally we have vmap_area.va_end pointing to the next
> vmap_area.va_start.
>
> To have a more reliable object reference counting, kmemleak avoids
> scanning most of the vmap_area structure (apart from vmap_area.vm) and
> requires a minimum count of 2 references for a vmalloc'ed object to not
> be considered a leak (that is vm_struct.addr and the caller's location,
> in this case task->stack).
>
>> On Mon 15-05-17 23:53:18, Luis R. Rodriguez wrote:
>> > root@piggy:~# cat /sys/kernel/debug/kmemleak
>> > unreferenced object 0xffffa07500d4c000 (size 16384):
>> >   comm "bash", pid 1349, jiffies 4294895999 (age 263.204s)
>> >   hex dump (first 32 bytes):
>> >     9d 6e ac 57 00 00 00 00 00 00 00 00 00 00 00 00  .n.W............
>> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>> >   backtrace:
>> >     [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
>> >     [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
>> >     [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
>> >     [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
>> >     [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
>> >     [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
>> >     [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
>> >     [<ffffffffffffffff>] 0xffffffffffffffff
>
> This stack here was allocated by bash for a child process via
> alloc_thread_stack_node() when CONFIG_VMAP_STACK is enabled. When the
> child died, free_stack_thread() did not call vfree_atomic() but rather
> stored the corresponding vm_struct pointer in cached_stacks[i]. However,
> the original task->stack pointer was lost (child task_struct freed), so
> when scanning the memory kmemleak can only find a single reference to
> the original stack (via vm_struct cached_stacks[i]) rather than 2 as
> required for vmalloc'ed objects.
>
> BTW, you can get more info about a specific object, including the number
> of references found, with:
>
>   echo dump=0xffffa07500d4c000 > /sys/kernel/debug/kmemleak
>
> So basically kernel/fork.c has its own thread stack allocator on top of
> vmalloc and kmemleak gets confused. A quick workaround:
>
> ------------8<------------------------------
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 06d759ab4c62..785907fccf67 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -214,6 +214,7 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)
>                 this_cpu_write(cached_stacks[i], NULL);
>
>                 tsk->stack_vm_area = s;
> +               kmemleak_alloc(s->addr, THREAD_SIZE, 2, GFP_ATOMIC);
>                 local_irq_enable();
>                 return s->addr;
>         }
> @@ -254,6 +255,7 @@ static inline void free_thread_stack(struct task_struct *tsk)
>                                 continue;
>
>                         this_cpu_write(cached_stacks[i], tsk->stack_vm_area);
> +                       kmemleak_free(tsk->stack);
>                         local_irq_restore(flags);
>                         return;
>                 }
> ------------8<------------------------------

I would do this differently.  The problem only affects cached stacks,
right?  How about kmemleal_ignoring when caching a stack and
unignoring when uncaching a stack?

Also, this needs a serious comment.  How about "kmemleak does not
presently understand that a reference to a vm_area_struct implies a
reference to the vmalloced memory"?

> A more complex solution (which I'm not yet convinced it works) may be to
> pass the number of references of an objects down to the objects it
> refers. In the vmalloc case we normally have a single reference to
> vm_struct and two to the vmalloc'ed object. If a reference to the
> vmalloc'ed object disappeared we could balance it with an increased
> number of references to the vm_struct object. But this option requires
> more research.

The idea being that, with this change, kmemleak would start to
understand that surplus references to vm_area_struct cause it to think
the memory itself is reachable?

--Andy

[toc] | [prev] | [next] | [standalone]

#1643267

On Tue, May 16, 2017 at 04:55:28PM -0700, Andy Lutomirski wrote:
> On Tue, May 16, 2017 at 6:39 AM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > Thanks for cc'ing me. The vmalloc allocations have always been tricky
> > for kmemleak since there are 2-3 other memory locations with the same
> > value as the vmalloc'ed object: vm_struct.addr and vmap_area.va_start;
> > occasionally we have vmap_area.va_end pointing to the next
> > vmap_area.va_start.
> >
> > To have a more reliable object reference counting, kmemleak avoids
> > scanning most of the vmap_area structure (apart from vmap_area.vm) and
> > requires a minimum count of 2 references for a vmalloc'ed object to not
> > be considered a leak (that is vm_struct.addr and the caller's location,
> > in this case task->stack).
> >
> >> On Mon 15-05-17 23:53:18, Luis R. Rodriguez wrote:
> >> > root@piggy:~# cat /sys/kernel/debug/kmemleak
> >> > unreferenced object 0xffffa07500d4c000 (size 16384):
> >> >   comm "bash", pid 1349, jiffies 4294895999 (age 263.204s)
> >> >   hex dump (first 32 bytes):
> >> >     9d 6e ac 57 00 00 00 00 00 00 00 00 00 00 00 00  .n.W............
> >> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >> >   backtrace:
> >> >     [<ffffffffa5464cca>] kmemleak_alloc+0x4a/0xa0
> >> >     [<ffffffffa4fdfe6c>] __vmalloc_node_range+0x20c/0x2b0
> >> >     [<ffffffffa4e7d1a2>] copy_process.part.37+0x5c2/0x1af0
> >> >     [<ffffffffa4e7e89f>] _do_fork+0xcf/0x390
> >> >     [<ffffffffa4e7ec09>] SyS_clone+0x19/0x20
> >> >     [<ffffffffa4e03b0b>] do_syscall_64+0x5b/0xc0
> >> >     [<ffffffffa547072b>] return_from_SYSCALL_64+0x0/0x6a
> >> >     [<ffffffffffffffff>] 0xffffffffffffffff
> >
> > This stack here was allocated by bash for a child process via
> > alloc_thread_stack_node() when CONFIG_VMAP_STACK is enabled. When the
> > child died, free_stack_thread() did not call vfree_atomic() but rather
> > stored the corresponding vm_struct pointer in cached_stacks[i]. However,
> > the original task->stack pointer was lost (child task_struct freed), so
> > when scanning the memory kmemleak can only find a single reference to
> > the original stack (via vm_struct cached_stacks[i]) rather than 2 as
> > required for vmalloc'ed objects.
> >
> > BTW, you can get more info about a specific object, including the number
> > of references found, with:
> >
> >   echo dump=0xffffa07500d4c000 > /sys/kernel/debug/kmemleak
> >
> > So basically kernel/fork.c has its own thread stack allocator on top of
> > vmalloc and kmemleak gets confused. A quick workaround:
> >
> > ------------8<------------------------------
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 06d759ab4c62..785907fccf67 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -214,6 +214,7 @@ static unsigned long *alloc_thread_stack_node(struct task_struct *tsk, int node)
> >                 this_cpu_write(cached_stacks[i], NULL);
> >
> >                 tsk->stack_vm_area = s;
> > +               kmemleak_alloc(s->addr, THREAD_SIZE, 2, GFP_ATOMIC);
> >                 local_irq_enable();
> >                 return s->addr;
> >         }
> > @@ -254,6 +255,7 @@ static inline void free_thread_stack(struct task_struct *tsk)
> >                                 continue;
> >
> >                         this_cpu_write(cached_stacks[i], tsk->stack_vm_area);
> > +                       kmemleak_free(tsk->stack);
> >                         local_irq_restore(flags);
> >                         return;
> >                 }
> > ------------8<------------------------------
> 
> I would do this differently.  The problem only affects cached stacks,
> right? How about kmemleal_ignoring when caching a stack and
> unignoring when uncaching a stack?

Indeed ;), that was my paragraph just below the patch:

> > A better solution I think would be something like kmemleak_ignore() on
> > the free path paired with a new kmemleak_unignore() in order to avoid
> > the freeing/re-allocation of the kmemleak metadata.

> Also, this needs a serious comment.  How about "kmemleak does not
> presently understand that a reference to a vm_area_struct implies a
> reference to the vmalloced memory"?

Yes, as most of the other kmemleak annotations.

> > A more complex solution (which I'm not yet convinced it works) may be to
> > pass the number of references of an objects down to the objects it
> > refers. In the vmalloc case we normally have a single reference to
> > vm_struct and two to the vmalloc'ed object. If a reference to the
> > vmalloc'ed object disappeared we could balance it with an increased
> > number of references to the vm_struct object. But this option requires
> > more research.
> 
> The idea being that, with this change, kmemleak would start to
> understand that surplus references to vm_area_struct cause it to think
> the memory itself is reachable?

That's the idea, possibly with a new kmemleak API specific to vmalloc
that would link the vm_struct object with the vmalloc'ed one. The
advantage is that we don't need to annotate the stack caching (that's
the only place that I'm aware of), though the actual implementation is a
bit more complex than kmemleak_ignore/unignore (and assuming the idea is
sane).

-- 
Catalin

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web