Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933432AbcCOWIE (ORCPT ); Tue, 15 Mar 2016 18:08:04 -0400 Received: from h2.hallyn.com ([78.46.35.8]:49856 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932195AbcCOWIC (ORCPT ); Tue, 15 Mar 2016 18:08:02 -0400 Date: Tue, 15 Mar 2016 17:07:58 -0500 From: "Serge E. Hallyn" To: Andy Lutomirski Cc: Linux FS Devel , linux-kernel@vger.kernel.org, "Eric W. Biederman" , gnome-os-list@gnome.org, Linux Containers , Serge Hallyn , mclasen@redhat.com Subject: Re: [PATCH v2] devpts: Make ptmx be owned by the userns owner as a fallback Message-ID: <20160315220758.GA18236@mail.hallyn.com> References: <820e57306e342ca310414ed0f58e75ac99731871.1458072215.git.luto@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <820e57306e342ca310414ed0f58e75ac99731871.1458072215.git.luto@kernel.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2976 Lines: 93 Quoting Andy Lutomirski (luto@kernel.org): > New devpts instances have ptmx owned by the inner uid and gid 0. > > For container-style namespaces (LXC, etc), this should have no > effect, this is fine. > > For sandbox-style namespaces (xdg-app and similar), this is > problematic -- there may not be an inner 0:0. If that happens, > devpts mounts will fail. > > Fix it by adding a fallback: if 0:0 is not mapped but the userns > owner and group are mapped, then ptmx will be owned by the namespace > owner. > > This won't change behavior except in cases where mount would > currently return -EINVAL. > > Cc: Alexander Larsson > Cc: mclasen@redhat.com > Cc: "Eric W. Biederman" > Cc: Linux Containers > Signed-off-by: Andy Lutomirski Thanks, looks good. Acked-by: Serge Hallyn > --- > > Changes from v1: > - Reversed the preference order (Serge) > - Fixed misuse of uid_valid on userns->owner > > fs/devpts/inode.c | 29 +++++++++++++++++++++++++---- > 1 file changed, 25 insertions(+), 4 deletions(-) > > diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c > index 655f21f99160..42b1e04d8334 100644 > --- a/fs/devpts/inode.c > +++ b/fs/devpts/inode.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > > #define DEVPTS_DEFAULT_MODE 0600 > /* > @@ -247,13 +248,33 @@ static int mknod_ptmx(struct super_block *sb) > struct dentry *root = sb->s_root; > struct pts_fs_info *fsi = DEVPTS_SB(sb); > struct pts_mount_opts *opts = &fsi->mount_opts; > + struct user_namespace *userns = current_user_ns(); > kuid_t root_uid; > kgid_t root_gid; > > - root_uid = make_kuid(current_user_ns(), 0); > - root_gid = make_kgid(current_user_ns(), 0); > - if (!uid_valid(root_uid) || !gid_valid(root_gid)) > - return -EINVAL; > + /* > + * For a new devpts instance, ptmx is owned by 0:0 if that uid > + * and gid are mapped in the creating namespace. > + */ > + root_uid = make_kuid(userns, 0); > + root_gid = make_kgid(userns, 0); > + > + if (!uid_valid(root_uid) || !gid_valid(root_gid)) { > + /* > + * If the creating namespace does not have 0:0 mapped > + * but does have the owner mapped (this is rare in > + * container-style namespaces but common in > + * sandbox-style namespaces), then let ptmx be owned by > + * the namespace owner. > + */ > + root_uid = userns->owner; > + root_gid = userns->group; > + > + /* If this still doesn't work, give up. */ > + if (!kuid_has_mapping(userns, root_uid) || > + !kgid_has_mapping(userns, root_gid)) > + return -EINVAL; > + } > > inode_lock(d_inode(root)); > > -- > 2.5.0 > > _______________________________________________ > Containers mailing list > Containers@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/containers