Re: mm_alloc()'ed structure leak

Peter Zijlstra
2009-02-09T16:46:23Z

On Mon, 2009-02-09 at 16:12 +0100, Peter Zijlstra wrote:
> On Mon, 2009-02-09 at 14:44 +0000, Catalin Marinas wrote:
> > On Mon, 2009-02-09 at 12:18 +0000, Catalin Marinas wrote:
> > > Basically bash forks and executes a command like "host kernel.org" which
> > > finishes normally but the corresponding mm_struct isn't freed (I get
> > > this consistently every time I run the above command):
> > >
> > > unreferenced object 0xcfed4070 (size 368):
> > > comm "bash", pid 1674, jiffies 421592
> > > backtrace:
> > > [<c0082bd4>] kmemleak_alloc+0x140/0x2b0
> > > [<c007ff2c>] kmem_cache_alloc+0xd0/0x100
> > > [<c0036980>] mm_alloc+0x14/0x44
> > > [<c008a99c>] bprm_mm_init+0xc/0x13c
> > > [<c008ab70>] do_execve+0xa4/0x218
> > > [<c002718c>] sys_execve+0x34/0x54
> > > [<c0023e80>] ret_fast_syscall+0x0/0x28
> >
> > Dumping the object in question:
> >
> > mm_struct.mm_users = 0
> > mm_struct.mm_count = 1
> >
> > It looks like the mm_count didn't get to 0 hence no structure freeing
> > via mmdrop().
> >
> > The leak disappears if I revert commit 38d47c1b7075 - "futex: rely on
> > get_user_pages() for shared futexes". Peter, any idea?
>
> Looks like the futex key references go wrong somewhere, I'll go look at
> it.

How does this work for you?

Not-signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
kernel/futex.c | 26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index f89d373..4aecf77 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -55,6 +55,7 @@
#include <linux/magic.h>
#include <linux/pid.h>
#include <linux/nsproxy.h>
+#include <linux/ftrace.h>

#include <asm/futex.h>

@@ -156,9 +157,15 @@ static void get_futex_key_refs(union futex_key *key)

switch (key->both.offset & (FUT_OFF_INODE|FUT_OFF_MMSHARED)) {
case FUT_OFF_INODE:
+ ftrace_printk("(%lx:%x) inode++: %p\n",
+ key->both.word, key->both.offset & ~3,
+ &key->shared.inode);
atomic_inc(&key->shared.inode->i_count);
break;
case FUT_OFF_MMSHARED:
+ ftrace_printk("(%lx:%x) mm++: %p\n",
+ key->both.word, key->both.offset & ~3,
+ &key->shared.inode);
atomic_inc(&key->private.mm->mm_count);
break;
}
@@ -178,9 +185,15 @@ static void drop_futex_key_refs(union futex_key *key)

switch (key->both.offset & (FUT_OFF_INODE|FUT_OFF_MMSHARED)) {
case FUT_OFF_INODE:
+ ftrace_printk("(%lx:%x) inode--: %p\n",
+ key->both.word, key->both.offset & ~3,
+ &key->shared.inode);
iput(key->shared.inode);
break;
case FUT_OFF_MMSHARED:
+ ftrace_printk("(%lx:%x) mm--: %p\n",
+ key->both.word, key->both.offset & ~3,
+ &key->shared.inode);
mmdrop(key->private.mm);
break;
}
@@ -1284,17 +1297,20 @@ retry:
*/

/* If we were woken (and unqueued), we succeeded, whatever. */
+ ret = 0;
if (!unqueue_me(&q))
- return 0;
+ goto out_put_key;
+ ret = -ETIMEDOUT;
if (rem)
- return -ETIMEDOUT;
+ goto out_put_key;

/*
* We expect signal_pending(current), but another thread may
* have handled it for us already.
*/
+ ret = -ERESTARTSYS;
if (!abs_time)
- return -ERESTARTSYS;
+ goto out_put_key;
else {
struct restart_block *restart;
restart = &current_thread_info()->restart_block;
@@ -1309,11 +1325,13 @@ retry:
restart->futex.flags |= FLAGS_SHARED;
if (clockrt)
restart->futex.flags |= FLAGS_CLOCKRT;
- return -ERESTART_RESTARTBLOCK;
+ ret = -ERESTARTSYS;
+ goto out_put_key;
}

out_unlock_put_key:
queue_unlock(&q, hb);
+out_put_key:
put_futex_key(fshared, &q.key);