Quoting David Howells (dhowells@xxxxxxxxxx): > Serge E. Hallyn <serue@xxxxxxxxxx> 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. That's a good idea. Now that won't handle unshare(), but then I'm tempted to do like CLONE_NEWPID and return -EINVAL on unshare() anyway. > 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. Thanks it does seem i got confused somewhere. In fact there is no need for user_ns to point to its root_user at all. The root_user will be pinned by the task,and if that task is the only one in the user_ns and it does does setuid(500), then there is no reason not to keep root_user from being freed. That way we stick with the simple refcounting rules: The task pins the user struct. The user struct pins its user namespace. The user namespace pins the struct user which created it. > > + .creator = &root_user, > > Probably means that you should increment the initial usage count on root_user. Not given the above since the init_user_ns.root_user is going away. And yet... Yes, it should be 2, one for the init_user_ns.creator link and one for the init task pointing to it. Thanks very much, David! I'll respin. -serge _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers