From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932754AbbIYRee (ORCPT ); Fri, 25 Sep 2015 13:34:34 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:59107 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932474AbbIYReb (ORCPT ); Fri, 25 Sep 2015 13:34:31 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Seth Forshee Cc: Alexander Viro , Jeff Layton , "J. Bruce Fields" , Serge Hallyn , Andy Lutomirski , linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org References: <1443039368-55445-1-git-send-email-seth.forshee@canonical.com> <1443039368-55445-2-git-send-email-seth.forshee@canonical.com> <87vbazplja.fsf@x220.int.ebiederm.org> <20150925125414.GC104990@ubuntu-hedt> Date: Fri, 25 Sep 2015 12:27:09 -0500 In-Reply-To: <20150925125414.GC104990@ubuntu-hedt> (Seth Forshee's message of "Fri, 25 Sep 2015 07:54:14 -0500") Message-ID: <87eghmjtoy.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1/u8t8gatPvq+9SZbvAEtp133ejJpqdU1I= X-SA-Exim-Connect-IP: 67.3.201.231 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.7 XMSubLong Long Subject * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 1397; Body=1 Fuz1=1 Fuz2=1] * 0.4 XMBrknScrpt_02 Possible Broken Spam Script * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa04 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Seth Forshee X-Spam-Relay-Country: X-Spam-Timing: total 1057 ms - load_scoreonly_sql: 0.06 (0.0%), signal_user_changed: 9 (0.9%), b_tie_ro: 2.6 (0.2%), parse: 0.90 (0.1%), extract_message_metadata: 18 (1.7%), get_uri_detail_list: 3.0 (0.3%), tests_pri_-1000: 4.8 (0.5%), tests_pri_-950: 0.94 (0.1%), tests_pri_-900: 0.79 (0.1%), tests_pri_-400: 39 (3.7%), check_bayes: 38 (3.6%), b_tokenize: 8 (0.8%), b_tok_get_all: 18 (1.7%), b_comp_prob: 2.8 (0.3%), b_tok_touch_all: 6 (0.6%), b_finish: 1.11 (0.1%), tests_pri_0: 976 (92.4%), tests_pri_500: 4.3 (0.4%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH v4 1/7] fs: Add user namesapace member to struct super_block X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Seth Forshee writes: > On Thu, Sep 24, 2015 at 04:14:33PM -0500, Eric W. Biederman wrote: >> Seth Forshee writes: >> >> > Initially this will be used to eliminate the implicit MNT_NODEV >> > flag for mounts from user namespaces. In the future it will also >> > be used for translating ids and checking capabilities for >> > filesystems mounted from user namespaces. >> > >> > s_user_ns is initialized in alloc_super() and is generally set to >> > current_user_ns(). To avoid security and corruption issues, two >> > additional mount checks are also added: >> > >> > - do_new_mount() gains a check that the user has CAP_SYS_ADMIN >> > in current_user_ns(). >> > >> > - sget() will fail with EBUSY when the filesystem it's looking >> > for is already mounted from another user namespace. >> > >> > proc requires some special handling. The user namespace of >> > current isn't appropriate when forking as a result of clone (2) >> > with CLONE_NEWPID|CLONE_NEWUSER, as it will set s_user_ns to the >> > namespace of the parent and make proc unmountable in the new user >> > namespace. Instead, the user namespace which owns the new pid >> > namespace is used. sget_userns() is allowed to allow passing in >> > a namespace other than that of current, and sget becomes a >> > wrapper around sget_userns() which passes current_user_ns(). >> >> Minor nits below. I have fixed them up. >> >> > Signed-off-by: Seth Forshee >> > --- >> > fs/namespace.c | 3 +++ >> > fs/proc/root.c | 3 ++- >> > fs/super.c | 38 +++++++++++++++++++++++++++++++++----- >> > include/linux/fs.h | 9 ++++++++- >> > 4 files changed, 46 insertions(+), 7 deletions(-) >> > >> > diff --git a/fs/namespace.c b/fs/namespace.c >> > index 0570729c87fd..d023a353dc63 100644 >> > --- a/fs/namespace.c >> > +++ b/fs/namespace.c >> > @@ -2381,6 +2381,9 @@ static int do_new_mount(struct path *path, const char *fstype, int flags, >> > struct vfsmount *mnt; >> > int err; >> > >> > + if (!ns_capable(current_user_ns(), CAP_SYS_ADMIN)) >> > + return -EPERM; >> > + >> > if (!fstype) >> > return -EINVAL; >> > >> > diff --git a/fs/proc/root.c b/fs/proc/root.c >> > index 361ab4ee42fc..4b302cbf13f9 100644 >> > --- a/fs/proc/root.c >> > +++ b/fs/proc/root.c >> > @@ -117,7 +117,8 @@ static struct dentry *proc_mount(struct file_system_type *fs_type, >> > return ERR_PTR(-EPERM); >> > } >> > >> > - sb = sget(fs_type, proc_test_super, proc_set_super, flags, ns); >> > + sb = sget_userns(fs_type, proc_test_super, proc_set_super, flags, >> > + ns->user_ns, ns); >> > if (IS_ERR(sb)) >> > return ERR_CAST(sb); >> > >> > diff --git a/fs/super.c b/fs/super.c >> > index 954aeb80e202..42837da7d641 100644 >> > --- a/fs/super.c >> > +++ b/fs/super.c >> > @@ -33,6 +33,7 @@ >> > #include >> > #include >> > #include >> > +#include >> > #include "internal.h" >> > >> > >> > @@ -163,6 +164,7 @@ static void destroy_super(struct super_block *s) >> > { >> > list_lru_destroy(&s->s_dentry_lru); >> > list_lru_destroy(&s->s_inode_lru); >> > + put_user_ns(s->s_user_ns); >> > security_sb_free(s); >> > WARN_ON(!list_empty(&s->s_mounts)); >> > kfree(s->s_subtype); >> > @@ -178,7 +180,8 @@ static void destroy_super(struct super_block *s) >> > * Allocates and initializes a new &struct super_block. alloc_super() >> > * returns a pointer new superblock or %NULL if allocation had failed. >> > */ >> > -static struct super_block *alloc_super(struct file_system_type *type, int flags) >> > +static struct super_block *alloc_super(struct file_system_type *type, int flags, >> > + struct user_namespace *user_ns) >> > { >> > struct super_block *s = kzalloc(sizeof(struct super_block), GFP_USER); >> > static const struct super_operations default_op; >> > @@ -246,6 +249,8 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags) >> > s->s_shrink.count_objects = super_cache_count; >> > s->s_shrink.batch = 1024; >> > s->s_shrink.flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE; >> > + >> > + s->s_user_ns = get_user_ns(user_ns); >> > return s; >> > >> > fail: >> > @@ -442,17 +447,17 @@ void generic_shutdown_super(struct super_block *sb) >> > EXPORT_SYMBOL(generic_shutdown_super); >> > >> > /** >> > - * sget - find or create a superblock >> > + * sget_userns - find or create a superblock >> > * @type: filesystem type superblock should belong to >> > * @test: comparison callback >> > * @set: setup callback >> > * @flags: mount flags >> >> You don't mention the user namespace parameter here. I have fixed that >> as. >> >> + * @user_ns: User namespace you need CAP_SYS_ADMIN over to mount this fs. > > Looks good, thanks. Seems I also missed it in alloc_super though. FYI I have placed everything that I has made it through my review in my for-testing branch up on kernel.org. So you can see what I have merged, and the build test bots can look and see if they find anything to complain about. git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git for-testing Eric