| Message ID | 1376606877-22599-1-git-send-email-kamal@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 08/15, Kamal Mostafa wrote: > > commit 6160968cee8b90a5dd95318d716e31d7775c4ef3 upstream. > > unshare_userns(new_cred) does *new_cred = prepare_creds() before > create_user_ns() which can fail. However, the caller expects that > it doesn't need to take care of new_cred if unshare_userns() fails. I'd also suggest you to take the next commit, 8742f229b635b "userns: limit the maximum depth of user_namespace->parent chain". I forgot to cc -stable, sorry. As Andy pointed out unshare_userns() has problems even if succeeds. Oleg.
On Fri, 2013-08-16 at 13:59 +0200, Oleg Nesterov wrote: > On 08/15, Kamal Mostafa wrote: > > > > commit 6160968cee8b90a5dd95318d716e31d7775c4ef3 upstream. > > > > unshare_userns(new_cred) does *new_cred = prepare_creds() before > > create_user_ns() which can fail. However, the caller expects that > > it doesn't need to take care of new_cred if unshare_userns() fails. > > I'd also suggest you to take the next commit, 8742f229b635b > "userns: limit the maximum depth of user_namespace->parent chain". > I forgot to cc -stable, sorry. > > > As Andy pointed out unshare_userns() has problems even if succeeds. > > Oleg. Thanks very much, Oleg. I'll queue up 8742f229b635b as well. -Kamal
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index f359dc7..38ae0f5 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -105,16 +105,21 @@ int create_user_ns(struct cred *new) int unshare_userns(unsigned long unshare_flags, struct cred **new_cred) { struct cred *cred; + int err = -ENOMEM; if (!(unshare_flags & CLONE_NEWUSER)) return 0; cred = prepare_creds(); - if (!cred) - return -ENOMEM; + if (cred) { + err = create_user_ns(cred); + if (err) + put_cred(cred); + else + *new_cred = cred; + } - *new_cred = cred; - return create_user_ns(cred); + return err; } void free_user_ns(struct kref *kref)
This is a note to let you know that I have just added a patch titled userns: unshare_userns(&cred) should not populate cred on failure to the linux-3.8.y-queue branch of the 3.8.y.z extended stable tree which can be found at: http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.8.y-queue This patch is scheduled to be released in version 3.8.13.7. If you, or anyone else, feels it should not be added to this tree, please reply to this email. For more information about the 3.8.y.z tree, see https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable Thanks. -Kamal ------ From cd9c7111df7579e703cebf039082c115c0de4471 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov <oleg@redhat.com> Date: Tue, 6 Aug 2013 19:38:55 +0200 Subject: userns: unshare_userns(&cred) should not populate cred on failure commit 6160968cee8b90a5dd95318d716e31d7775c4ef3 upstream. unshare_userns(new_cred) does *new_cred = prepare_creds() before create_user_ns() which can fail. However, the caller expects that it doesn't need to take care of new_cred if unshare_userns() fails. We could change the single caller, sys_unshare(), but I think it would be more clean to avoid the side effects on failure, so with this patch unshare_userns() does put_cred() itself and initializes *new_cred only if create_user_ns() succeeeds. Signed-off-by: Oleg Nesterov <oleg@redhat.com> Reviewed-by: Andy Lutomirski <luto@amacapital.net> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Kamal Mostafa <kamal@canonical.com> --- kernel/user_namespace.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) -- 1.8.1.2