Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrWbsT+HGvVQMV_fUEDa1otG+er0gU3H4v-_9FdWgYpY1g@mail.gmail.com>
Date: Tue, 6 Aug 2013 11:14:24 -0700
From: Andy Lutomirski <luto@...capital.net>
To: Oleg Nesterov <oleg@...hat.com>
Cc: security@...nel.org, oss-security@...ts.openwall.com, 
	Petr Matousek <pmatouse@...hat.com>, "Eric W. Biederman" <ebiederm@...ssion.com>, 
	David Howells <dhowells@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] userns: unshare_userns(&cred) should not populate
 cred on failure

On Tue, Aug 6, 2013 at 10:38 AM, Oleg Nesterov <oleg@...hat.com> wrote:
> 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.
>
> Cc: stable@...r.kernel.org
> Signed-off-by: Oleg Nesterov <oleg@...hat.com>
> ---
>  kernel/user_namespace.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index d8c30db..6e50a44 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 user_namespace *ns)
> --
> 1.5.5.1
>
>

Reviewed-by: Andy Lutomirski <luto@...capital.net>


-- 
Andy Lutomirski
AMA Capital Management, LLC

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.