[PATCH] Validate pointer when copying mount namespace.

[PATCH] Validate pointer when copying mount namespace.

[Leon Ma]

We encountered following panic. Validate the pointer to avoid this.
[35046.276380] BUG: unable to handle kernel NULL pointer 
dereference at 00000010
[35046.283316] IP: [<8095dc91>] copy_mnt_ns+0x111/0x260
[35046.288225] *pdpt = 000000001b883001 *pde = 0000000000000000
[35046.293901] Oops: 0000 [#1] PREEMPT SMP
[35046.307342] CPU: 2 PID: 6761 Comm: main Tainted: G        W  O 
[35046.315345] task: a7f06f80 ti: 82e16000 task.ti: 82e16000
[35046.320673] EIP: 0060:[<8095dc91>] EFLAGS: 00210246 CPU: 2
[35046.326106] EIP is at copy_mnt_ns+0x111/0x260
[35046.330397] EAX: 3436362e EBX: b1026880 ECX: 96822f80 EDX: 828314a8
[35046.336590] ESI: 00000000 EDI: b0dde300 EBP: 82e17f50 ESP: 82e17f24
[35046.342789]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[35046.348124] CR0: 8005003b CR2: 00000010 CR3: 033c6000 CR4: 00102720
[35046.354397] DR0: 00000000 DR1: 00000001 DR2: 00000002 DR3: 00000003
[35046.360525] DR6: 00000006 DR7: 00000007
[35046.364299] Stack:
[35046.366278]  828314a8 96822f80 82831490 82831490 82831490 a3161780 
82831480 00000018
[35046.373932]  a7f06f80 a4c130d8 00020200 82e17f6c 8085f98d b1026880 
8134b920 00020200
[35046.381593]  8134b920 82e17f9c 82e17f84 8085fbcf b1026880 00020200 
ffffffea 00000000
[35046.389259] Call Trace:
[35046.391693]  [<8085f98d>] create_new_namespaces+0x4d/0x160
[35046.397106]  [<8085fbcf>] unshare_nsproxy_namespaces+0x5f/0xa0
[35046.402870]  [<8083c7c4>] SyS_unshare+0x104/0x240
[35046.407518]  [<80fe1700>] syscall_call+0x7/0xb

Signed-off-by: Leon Ma <xindong.ma@intel.com>
---
 fs/namespace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 1f4f9da..1c61c92 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2788,7 +2788,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
 		q = next_mnt(q, new);
 		if (!q)
 			break;
-		while (p->mnt.mnt_root != q->mnt.mnt_root)
+		while (p && p->mnt.mnt_root != q->mnt.mnt_root)
 			p = next_mnt(p, old);
 	}
 	namespace_unlock();
-- 
1.7.9.5

Re: [PATCH] Validate pointer when copying mount namespace.

[Al Viro]

On Wed, Apr 29, 2015 at 09:56:43AM +0800, Leon Ma wrote:
> We encountered following panic. Validate the pointer to avoid this.

> @@ -2788,7 +2788,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
>  		q = next_mnt(q, new);
>  		if (!q)
>  			break;
> -		while (p->mnt.mnt_root != q->mnt.mnt_root)
> +		while (p && p->mnt.mnt_root != q->mnt.mnt_root)
>  			p = next_mnt(p, old);
>  	}
>  	namespace_unlock();

Details, please.  How do you reproduce that behaviour?  

I don't like that loop in its current form (it relies upon _not_ encountering
the same ->mnt_root in the parts of tree we hadn't copied), but your change
doesn't make it any better.  Seeing a reproducer would be useful in sorting
it out; in this form the patch papers over the bug rather than fixing it.

RE: [PATCH] Validate pointer when copying mount namespace.

[Ma, Xindong]

> Details, please.  How do you reproduce that behaviour?
> 
> I don't like that loop in its current form (it relies upon _not_ encountering the
> same ->mnt_root in the parts of tree we hadn't copied), but your change
> doesn't make it any better.  Seeing a reproducer would be useful in sorting it
> out; in this form the patch papers over the bug rather than fixing it.

The issue is reproduced during monkey test on android devices. Seems it's very hard to reproduce it again. I'll continue to perform the test and review the code.