[PATCH] fs: Fix NULL pointer dereference at copy_mnt_ns().

[PATCH] fs: Fix NULL pointer dereference at copy_mnt_ns().

[Tetsuo Handa]

I get oops when running below test program.

---------- test.c start ----------
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <sched.h>

static int child2(void *arg)
{
        return 0;
}

static int child(void *arg)
{
        char *stack = malloc(8192);
        const pid_t pid = clone(child2, stack + (8192 / 2), CLONE_NEWNS, NULL); // Oops here.
        waitpid(pid, NULL, __WALL);
        return 0;
}

int main(int argc, char *argv[])
{
        char *stack = malloc(8192);
        const pid_t pid = clone(child, stack + (8192 / 2), CLONE_NEWNS, NULL);
        waitpid(pid, NULL, __WALL);
        return 0;
}
---------- test.c end ----------

[   10.686782] BUG: unable to handle kernel NULL pointer dereference at 00000010
[   10.687753] IP: [<c10f379b>] copy_mnt_ns+0x5b/0x2a0
[   10.688767] *pde = 00000000
[   10.688991] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[   10.689647] Modules linked in:
[   10.689929]
[   10.690322] Pid: 1044, comm: init Not tainted 3.4.0-next-20120521 #25 Bochs Bochs
[   10.690727] EIP: 0060:[<c10f379b>] EFLAGS: 00000292 CPU: 0
[   10.690727] EIP is at copy_mnt_ns+0x5b/0x2a0
[   10.690727] EAX: 00000000 EBX: c79c9608 ECX: 00000005 EDX: c7d5dc60
[   10.690727] ESI: c6650420 EDI: fffffff4 EBP: c6641ea8 ESP: c6641e78
[   10.690727]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   10.690727] CR0: 8005003b CR2: 00000010 CR3: 06645000 CR4: 00000690
[   10.690727] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[   10.690727] DR6: 00000000 DR7: 00000000
[   10.690727] Process init (pid: 1044, ti=c6640000 task=c6f823e0 task.ti=c6640000)
[   10.690727] Stack:
[   10.690727]  c664b400 000000d0 c1052305 c78425c0 c6641ea8 00000000 c65f3558 c65f35d0
[   10.690727]  00000246 c664b400 c6650420 fffffff4 c6641ec4 c1052329 c79c9608 00020000
[   10.690727]  c664b430 c6650420 00020000 c6641edc c105256a c6654d80 c6650420 fffffff4
[   10.690727] Call Trace:
[   10.690727]  [<c1052305>] ? create_new_namespaces+0x25/0x150
[   10.690727]  [<c1052329>] create_new_namespaces+0x49/0x150
[   10.690727]  [<c105256a>] copy_namespaces+0x6a/0xb0
[   10.690727]  [<c102d3cf>] copy_process+0x67f/0xf30
[   10.690727]  [<c102dd63>] do_fork+0x53/0x270
[   10.690727]  [<c1026c6b>] ? do_page_fault+0x17b/0x360
[   10.690727]  [<c13c8544>] ? restore_all+0xf/0xf
[   10.690727]  [<c11c4d54>] ? trace_hardirqs_on_thunk+0xc/0x10
[   10.690727]  [<c13c8544>] ? restore_all+0xf/0xf
[   10.690727]  [<c100a09f>] sys_clone+0x2f/0x40
[   10.690727]  [<c13c8e2d>] ptregs_clone+0x15/0x28
[   10.690727]  [<c13c8511>] ? syscall_call+0x7/0xb
[   10.690727] Code: 8b 42 04 89 45 e4 e8 85 e0 ff ff 3d 00 f0 ff ff 89 45 e8 0f 87 b4 01 00 00 b8 60 9c c7 c1 e8 fd 24 2d 00 8b 45 e4 b9 05 00 00 00 <8b> 50 10 e8 ed fc ff ff 85 c0 89 45 e0 0f 84 fa 01 00 00 b8 c0
[   10.690727] EIP: [<c10f379b>] copy_mnt_ns+0x5b/0x2a0 SS:ESP 0068:c6641e78
[   10.690727] CR2: 0000000000000010
[   10.708430] ---[ end trace 17c0ee6742ac34a8 ]---

Bisected to

@@ -2249,10 +2250,9 @@ static struct mnt_namespace *dup_mnt_ns(struct mnt_namespace *mnt_ns,
                kfree(new_ns);
                return ERR_PTR(-ENOMEM);
        }
-       new_ns->root = new;
-       br_write_lock(vfsmount_lock);
+       br_write_lock(&vfsmount_lock);
        list_add_tail(&new_ns->list, &new->mnt_list);
-       br_write_unlock(vfsmount_lock);
+       br_write_unlock(&vfsmount_lock);

        /*
         * Second pass: switch the tsk->fs->* elements and mark new vfsmounts

in commit e497169c "brlocks/lglocks: API cleanups".
Please add this patch for 3.5-rc1.
----------------------------------------
>From b181b16adbd9e164020a9d4ee76eb2f405062f71 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 22 May 2012 00:13:46 +0900
Subject: [PATCH] fs: Fix NULL pointer dereference at copy_mnt_ns().

Commit e497169c "brlocks/lglocks: API cleanups" by error removed

  new_ns->root = new;

line, resulting oops when clone(CLONE_NEWNS) is called from a thread
created by clone(CLONE_NEWNS).

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/namespace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 5a6208b..224aff1 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2250,6 +2250,7 @@ static struct mnt_namespace *dup_mnt_ns(struct mnt_namespace *mnt_ns,
 		kfree(new_ns);
 		return ERR_PTR(-ENOMEM);
 	}
+	new_ns->root = new;
 	br_write_lock(&vfsmount_lock);
 	list_add_tail(&new_ns->list, &new->mnt_list);
 	br_write_unlock(&vfsmount_lock);
-- 
1.7.1

Re: [PATCH] fs: Fix NULL pointer dereference at copy_mnt_ns().

[Andi Kleen]

> diff --git a/fs/namespace.c b/fs/namespace.c
> index 5a6208b..224aff1 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2250,6 +2250,7 @@ static struct mnt_namespace *dup_mnt_ns(struct mnt_namespace *mnt_ns,
>  		kfree(new_ns);
>  		return ERR_PTR(-ENOMEM);
>  	}
> +	new_ns->root = new;


oops. Thanks for finding that. Must have been a merge error somewhere.

Should be a stable candidate.

Acked-by: Andi Kleen <ak@linux.intel.com>

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH] fs: Fix NULL pointer dereference at copy_mnt_ns().

[Al Viro]

On Mon, May 21, 2012 at 11:27:21AM -0700, Andi Kleen wrote:
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index 5a6208b..224aff1 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -2250,6 +2250,7 @@ static struct mnt_namespace *dup_mnt_ns(struct mnt_namespace *mnt_ns,
> >  		kfree(new_ns);
> >  		return ERR_PTR(-ENOMEM);
> >  	}
> > +	new_ns->root = new;
> 
> 
> oops. Thanks for finding that. Must have been a merge error somewhere.

Nah, just me cherry-picking unfixed variant.  It had come up back in March,
remember?  Replaced with fixed variant in the local tree, will go into
for-next (and for-linus, while we are at it) tonight or tomorrow morning.