Re: [PATCH 2/2] Fix user namespace exiting OOPs

Previous thread: [PATCH 1/2] Convert uid hash to hlist by Pavel Emelyanov on Friday, September 14, 2007 - 9:05 am. (2 messages)

Next thread: Oops in 2.6.22.5 by Alex Davis on Friday, September 14, 2007 - 9:42 am. (1 message)
To: Andrew Morton <akpm@...>
Cc: Serge Hallyn <serue@...>, Linux Kernel Mailing List <linux-kernel@...>, <devel@...>, Alexey Dobriyan <adobriyan@...>
Date: Friday, September 14, 2007 - 9:12 am

It turned out, that the user namespace is released during
the do_exit() in exit_task_namespaces(), but the struct
user_struct is released only during the put_task_struct(),
i.e. MUCH later.

On debug kernels with poisoned slabs this will cause the
oops in uid_hash_remove() because the head of the chain,
which resides inside the struct user_namespace, will be
already freed and poisoned.

Since the uid hash itself is required only when someone
can search it, i.e. when the namespace is alive, we can
safely unhash all the user_struct-s from it during the
namespace exiting. The subsequent free_uid() will complete
the user_struct destruction.

For example simple program

#include <sched.h>

char stack[2 * 1024 * 1024];

int f(void *foo)
{
return 0;
}

int main(void)
{
clone(f, stack + 1 * 1024 * 1024, 0x10000000, 0);
return 0;
}

run on kernel with CONFIG_USER_NS turned on will oops the
kernel immediately.

This was spotted during OpenVZ kernel testing.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: Alexey Dobriyan <adobriyan@openvz.org>

---

include/linux/sched.h | 1 +
kernel/user.c | 26 +++++++++++++++++++++++++-
kernel/user_namespace.c | 2 +-
3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a2afa88..b4a4211 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1530,6 +1530,7 @@ static inline struct user_struct *get_ui
}
extern void free_uid(struct user_struct *);
extern void switch_uid(struct user_struct *);
+extern void release_uids(struct user_namespace *ns);

#include <asm/current.h>

diff --git a/kernel/user.c b/kernel/user.c
index add57c7..e1f2d32 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -62,7 +62,7 @@ static inline void uid_hash_insert(struc

static inline void uid_hash_remove(struct user_struct *up)
{
- hlist_del(&up->uidhas...

To: Pavel Emelyanov <xemul@...>
Cc: Andrew Morton <akpm@...>, Serge Hallyn <serue@...>, Linux Kernel Mailing List <linux-kernel@...>, <devel@...>, Alexey Dobriyan <adobriyan@...>
Date: Friday, September 14, 2007 - 2:23 pm

Good spot. Interesting solution :)

thanks,
-

To: Serge E. Hallyn <serue@...>
Cc: Pavel Emelyanov <xemul@...>, Linux Kernel Mailing List <linux-kernel@...>, <devel@...>, Alexey Dobriyan <adobriyan@...>
Date: Friday, September 14, 2007 - 10:32 pm

Do we want to fix this in 2.6.23?

If so then at present I'll need to merge

kernel-userc-use-list_for_each_entry-instead-of-list_for_each.patch
convert-uid-hash-to-hlist.patch
fix-user-namespace-exiting-oops.patch

which is rather a lot of merging at this stage - surely more than
is really needed?
-

To: Andrew Morton <akpm@...>
Cc: Serge E. Hallyn <serue@...>, Linux Kernel Mailing List <linux-kernel@...>, <devel@...>, Alexey Dobriyan <adobriyan@...>
Date: Monday, September 17, 2007 - 2:21 am

This is not a security issue at all. This BUG can be triggered only
by CAP_SYS_ADMIN capable task on the kernel with CONFIG_USER_NS=y,

Thanks,
Pavel
-

Previous thread: [PATCH 1/2] Convert uid hash to hlist by Pavel Emelyanov on Friday, September 14, 2007 - 9:05 am. (2 messages)

Next thread: Oops in 2.6.22.5 by Alex Davis on Friday, September 14, 2007 - 9:42 am. (1 message)