[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrVtGE8LdBCFTe1_cqpLf=SxPNX5iCe5wa-hZ0pe8ps_jA@mail.gmail.com>
Date: Thu, 26 Mar 2015 12:29:31 -0700
From: Andy Lutomirski <luto@...capital.net>
To: "Eric W. Biederman" <ebiederm@...ssion.com>,
Linux FS Devel <linux-fsdevel@...r.kernel.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
mclasen@...hat.com, gnome-os-list@...me.org,
Alexander Larsson <alexl@...hat.com>,
Linux Containers <containers@...ts.linux-foundation.org>,
Andy Lutomirski <luto@...capital.net>
Subject: Re: [PATCH] devpts: Add ptmx_uid and ptmx_gid options
Ping? It's been over a month.
On Fri, Feb 20, 2015 at 5:04 PM, Andy Lutomirski <luto@...capital.net> wrote:
> It's currently impossible to mount devpts in a user namespace that
> has no root user, since ptmx can't be created. This adds options
> ptmx_uid and ptmx_gid that override the default uid and gid of 0.
>
> These options are not shown in mountinfo because they have no effect
> other than changing the initial mode of ptmx, and, in particular, it
> wouldn't make any sense to change them on remount. Instead, we
> disallow them on remount.
>
> This could be changed, but we'd probably want to fix the userns
> behavior of uid and gid at the same time if we did so.
>
> Signed-off-by: Andy Lutomirski <luto@...capital.net>
> ---
> Documentation/filesystems/devpts.txt | 4 +++
> fs/devpts/inode.c | 58 ++++++++++++++++++++++++++----------
> 2 files changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/filesystems/devpts.txt b/Documentation/filesystems/devpts.txt
> index 68dffd87f9b7..7808e77d0d72 100644
> --- a/Documentation/filesystems/devpts.txt
> +++ b/Documentation/filesystems/devpts.txt
> @@ -121,6 +121,10 @@ once), following user-space issues should be noted.
>
> chmod 666 /dev/pts/ptmx
>
> + The ownership for /dev/pts/ptmx can be specified using the ptmxuid
> + and ptmxgid options. Both default to zero, which, in user namespaces
> + that have no root user, will cause mounting to fail.
> +
> 7. A mount of devpts without the 'newinstance' option results in binding to
> initial kernel mount. This behavior while preserving legacy semantics,
> does not provide strict isolation in a container environment. i.e by
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index cfe8466f7fef..b60d1438c660 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -102,6 +102,8 @@ struct pts_mount_opts {
> int setgid;
> kuid_t uid;
> kgid_t gid;
> + uid_t ptmx_uid;
> + gid_t ptmx_gid;
> umode_t mode;
> umode_t ptmxmode;
> int newinstance;
> @@ -109,8 +111,8 @@ struct pts_mount_opts {
> };
>
> enum {
> - Opt_uid, Opt_gid, Opt_mode, Opt_ptmxmode, Opt_newinstance, Opt_max,
> - Opt_err
> + Opt_uid, Opt_gid, Opt_ptmx_uid, Opt_ptmx_gid, Opt_mode, Opt_ptmxmode,
> + Opt_newinstance, Opt_max, Opt_err,
> };
>
> static const match_table_t tokens = {
> @@ -118,6 +120,8 @@ static const match_table_t tokens = {
> {Opt_gid, "gid=%u"},
> {Opt_mode, "mode=%o"},
> #ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
> + {Opt_ptmx_uid, "ptmxuid=%u"},
> + {Opt_ptmx_gid, "ptmxgid=%u"},
> {Opt_ptmxmode, "ptmxmode=%o"},
> {Opt_newinstance, "newinstance"},
> {Opt_max, "max=%d"},
> @@ -162,14 +166,17 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts)
> char *p;
> kuid_t uid;
> kgid_t gid;
> -
> - opts->setuid = 0;
> - opts->setgid = 0;
> - opts->uid = GLOBAL_ROOT_UID;
> - opts->gid = GLOBAL_ROOT_GID;
> - opts->mode = DEVPTS_DEFAULT_MODE;
> + bool setptmxid = false;
> +
> + opts->setuid = 0;
> + opts->setgid = 0;
> + opts->uid = GLOBAL_ROOT_UID;
> + opts->gid = GLOBAL_ROOT_GID;
> + opts->ptmx_uid = 0;
> + opts->ptmx_gid = 0;
> + opts->mode = DEVPTS_DEFAULT_MODE;
> opts->ptmxmode = DEVPTS_DEFAULT_PTMX_MODE;
> - opts->max = NR_UNIX98_PTY_MAX;
> + opts->max = NR_UNIX98_PTY_MAX;
>
> /* newinstance makes sense only on initial mount */
> if (op == PARSE_MOUNT)
> @@ -209,6 +216,22 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts)
> opts->mode = option & S_IALLUGO;
> break;
> #ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
> + case Opt_ptmx_uid:
> + if (match_int(&args[0], &option))
> + return -EINVAL;
> + if (op != PARSE_MOUNT)
> + return -EINVAL;
> + opts->ptmx_uid = option;
> + setptmxid = true;
> + break;
> + case Opt_ptmx_gid:
> + if (match_int(&args[0], &option))
> + return -EINVAL;
> + if (op != PARSE_MOUNT)
> + return -EINVAL;
> + opts->ptmx_gid = option;
> + setptmxid = true;
> + break;
> case Opt_ptmxmode:
> if (match_octal(&args[0], &option))
> return -EINVAL;
> @@ -232,6 +255,9 @@ static int parse_mount_options(char *data, int op, struct pts_mount_opts *opts)
> }
> }
>
> + if (setptmxid && !opts->newinstance)
> + return -EINVAL;
> +
> return 0;
> }
>
> @@ -245,12 +271,12 @@ 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;
> - kuid_t root_uid;
> - kgid_t root_gid;
> + kuid_t ptmx_uid;
> + kgid_t ptmx_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))
> + ptmx_uid = make_kuid(current_user_ns(), opts->ptmx_uid);
> + ptmx_gid = make_kgid(current_user_ns(), opts->ptmx_gid);
> + if (!uid_valid(ptmx_uid) || !gid_valid(ptmx_gid))
> return -EINVAL;
>
> mutex_lock(&root->d_inode->i_mutex);
> @@ -282,8 +308,8 @@ static int mknod_ptmx(struct super_block *sb)
>
> mode = S_IFCHR|opts->ptmxmode;
> init_special_inode(inode, mode, MKDEV(TTYAUX_MAJOR, 2));
> - inode->i_uid = root_uid;
> - inode->i_gid = root_gid;
> + inode->i_uid = ptmx_uid;
> + inode->i_gid = ptmx_gid;
>
> d_add(dentry, inode);
>
> --
> 2.3.0
>
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists