Re: [PATCH 1/1] User namespaces: fix refcounting

[David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> Perhaps the most objectionable part of this to you may be the
> __task_commit_creds().

You're right, that looks pretty yuck.  I'm not sure why you need to do this.

I need to think about it a bit more, but I think you shouldn't be calling
[__task_]commit_creds() on any task that's not your own.

In fact, do you need to call commit_creds() on the new task?  No-one else can
have seen it yet, so RCU can be ignored; and no-one knows about it yet, so
calling proc_id_connector() is unnecessary.

The obvious thing to do would be to make copy_creds() handle the user namespace
copying.


A couple of quick other comments:

> @@ -595,6 +595,7 @@ struct user_struct {
>  	/* Hash table maintenance information */
>  	struct hlist_node uidhash_node;
>  	uid_t uid;
> +	struct user_namespace *user_ns;

Is asking for a circular dependency.  user_namespace must hold a dependency on
its the user_struct pointed to by root_user, but root_user holds a ref on
user_ns.

> +	.creator = &root_user,

Probably means that you should increment the initial usage count on root_user.

David