From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: [PATCH RFC] User namespaces: general cleanups Date: Mon, 13 Oct 2008 11:01:44 -0500 Message-ID: <20081013160144.GA10359@us.ibm.com> References: <20081010011917.GA8046@us.ibm.com> <30854.1223633214@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <30854.1223633214-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: David Howells Cc: Linux Containers , "Eric W. Biederman" List-Id: containers.vger.kernel.org Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org): > Serge E. Hallyn wrote: > > > + new->uid = new->euid = new->suid = new->fsuid = 0; > > + new->gid = new->egid = new->sgid = new->fsgid = 0; > > Should the supplementary groups be zapped too? Do the GIDs therein still have > meaning in the new user namespace? > > Note also that eCryptFS is broken by your patch. > > I suggest adding the attached incremental patch. It makes the following > changes: > > (1) Provides a current_user_ns() macro to wrap accesses to current's user > namespace. > > (2) Fixes eCryptFS. > > (3) Renames create_new_userns() to create_user_ns() to be more consistent > with the other associated functions and because the 'new' in the name is > superfluous. > > (4) Moves the argument and permission checks made for CLONE_NEWUSER to the > beginning of do_fork() so that they're done prior to making any attempts > at allocation. > > (5) Calls create_user_ns() after prepare_creds(), and gives it the new creds > to fill in rather than have it return the new root user. I don't imagine > the new root user being used for anything other than filling in a cred > struct. > > This also permits me to get rid of a get_uid() and a free_uid(), as the > reference the creds were holding on the old user_struct can just be > transferred to the new namespace's creator pointer. > > (6) Makes create_user_ns() reset the UIDs and GIDs of the creds under > preparation rather than doing it in copy_creds(). Hmm, with this patch, with CONFIG_KEYS=y users in child user_namespaces never get freed. Ones in the init_user_ns do, and with CONFIG_KEYS=n, those in child user_namespaces do as well. I don't see anything obvious in copy_creds() that would cause this... (also, when CONFIG_KEYS=n, then the enomem label in copy_creds() is unused - it might be kind of ugly to put just those two lines under #ifdef, though) -serge