From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) Subject: [PATCH] userns,pidns: Verify the userns for new pid namespaces Date: Sat, 29 Apr 2017 14:25:24 -0500 Message-ID: <87vapnrp7f.fsf_-_@xmission.com> References: <149329634856.21195.14196911999722279118.stgit@localhost.localdomain> <87mvb16fv7.fsf@xmission.com> <12a73543-79ea-4bac-7e96-6ab237534af2@virtuozzo.com> <877f254yx0.fsf@xmission.com> <8737crt4dz.fsf@xmission.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <8737crt4dz.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> (Eric W. Biederman's message of "Sat, 29 Apr 2017 14:12:08 -0500") 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: Linux Containers Cc: paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org, keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, gorcunov-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org, luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org, viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org, avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org, mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Kirill Tkhai , agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org List-Id: containers.vger.kernel.org It is pointless and confusing to allow a pid namespace hierarchy and the user namespace hierarchy to get out of sync. The owner of a child pid namespace should be the owner of the parent pid namespace or a descendant of the owner of the parent pid namespace. Otherwise it is possible to construct scenarios where it is legal to do something in a parent pid namespace but in a child pid namespace. It requires use of setns into a pid namespace (but not into a user namespace) to create such a scenario. Add the function in_userns to help in making this determination. Signed-off-by: "Eric W. Biederman" --- While review a patch from Kiril Tkhai I realized we were missing this sanity check.... include/linux/user_namespace.h | 8 +++++++- kernel/pid_namespace.c | 4 ++++ kernel/user_namespace.c | 18 ++++++++++++------ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h index 32354b4b4b2b..497ed50004db 100644 --- a/include/linux/user_namespace.h +++ b/include/linux/user_namespace.h @@ -112,8 +112,9 @@ extern ssize_t proc_projid_map_write(struct file *, const char __user *, size_t, extern ssize_t proc_setgroups_write(struct file *, const char __user *, size_t, loff_t *); extern int proc_setgroups_show(struct seq_file *m, void *v); extern bool userns_may_setgroups(const struct user_namespace *ns); +extern bool in_userns(const struct user_namespace *ancestor, + const struct user_namespace *child); extern bool current_in_userns(const struct user_namespace *target_ns); - struct ns_common *ns_get_owner(struct ns_common *ns); #else @@ -144,6 +145,11 @@ static inline bool userns_may_setgroups(const struct user_namespace *ns) return true; } +static inline bool in_userns(const struct user_namespace *target_ns) +{ + return true; +} + static inline bool current_in_userns(const struct user_namespace *target_ns) { return true; diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index de461aa0bf9a..749147f5a613 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -101,6 +101,10 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns int i; int err; + err = -EINVAL; + if (!in_userns(parent_pid_ns->user_ns, user_ns)) + goto out; + err = -ENOSPC; if (level > MAX_PID_NS_LEVEL) goto out; diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index 2f735cbe05e8..7d8658fbabc8 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -986,19 +986,25 @@ bool userns_may_setgroups(const struct user_namespace *ns) } /* - * Returns true if @ns is the same namespace as or a descendant of - * @target_ns. + * Returns true if @child is the same namespace or a descendant of + * @ancestor. */ -bool current_in_userns(const struct user_namespace *target_ns) +bool in_userns(const struct user_namespace *ancestor, + const struct user_namespace *child) { - struct user_namespace *ns; - for (ns = current_user_ns(); ns; ns = ns->parent) { - if (ns == target_ns) + const struct user_namespace *ns; + for (ns = child; ns; ns = ns->parent) { + if (ns == ancestor) return true; } return false; } +bool current_in_userns(const struct user_namespace *target_ns) +{ + return in_userns(target_ns, current_user_ns()); +} + static inline struct user_namespace *to_user_ns(struct ns_common *ns) { return container_of(ns, struct user_namespace, ns); -- 2.10.1 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out01.mta.xmission.com ([166.70.13.231]:49593 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1036424AbdD2Tbr (ORCPT ); Sat, 29 Apr 2017 15:31:47 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Linux Containers Cc: , , , , , , , , , , , , , , , , Kirill Tkhai References: <149329634856.21195.14196911999722279118.stgit@localhost.localdomain> <87mvb16fv7.fsf@xmission.com> <12a73543-79ea-4bac-7e96-6ab237534af2@virtuozzo.com> <877f254yx0.fsf@xmission.com> <8737crt4dz.fsf@xmission.com> Date: Sat, 29 Apr 2017 14:25:24 -0500 In-Reply-To: <8737crt4dz.fsf@xmission.com> (Eric W. Biederman's message of "Sat, 29 Apr 2017 14:12:08 -0500") Message-ID: <87vapnrp7f.fsf_-_@xmission.com> MIME-Version: 1.0 Content-Type: text/plain Subject: [PATCH] userns,pidns: Verify the userns for new pid namespaces Sender: linux-fsdevel-owner@vger.kernel.org List-ID: It is pointless and confusing to allow a pid namespace hierarchy and the user namespace hierarchy to get out of sync. The owner of a child pid namespace should be the owner of the parent pid namespace or a descendant of the owner of the parent pid namespace. Otherwise it is possible to construct scenarios where it is legal to do something in a parent pid namespace but in a child pid namespace. It requires use of setns into a pid namespace (but not into a user namespace) to create such a scenario. Add the function in_userns to help in making this determination. Signed-off-by: "Eric W. Biederman" --- While review a patch from Kiril Tkhai I realized we were missing this sanity check.... include/linux/user_namespace.h | 8 +++++++- kernel/pid_namespace.c | 4 ++++ kernel/user_namespace.c | 18 ++++++++++++------ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h index 32354b4b4b2b..497ed50004db 100644 --- a/include/linux/user_namespace.h +++ b/include/linux/user_namespace.h @@ -112,8 +112,9 @@ extern ssize_t proc_projid_map_write(struct file *, const char __user *, size_t, extern ssize_t proc_setgroups_write(struct file *, const char __user *, size_t, loff_t *); extern int proc_setgroups_show(struct seq_file *m, void *v); extern bool userns_may_setgroups(const struct user_namespace *ns); +extern bool in_userns(const struct user_namespace *ancestor, + const struct user_namespace *child); extern bool current_in_userns(const struct user_namespace *target_ns); - struct ns_common *ns_get_owner(struct ns_common *ns); #else @@ -144,6 +145,11 @@ static inline bool userns_may_setgroups(const struct user_namespace *ns) return true; } +static inline bool in_userns(const struct user_namespace *target_ns) +{ + return true; +} + static inline bool current_in_userns(const struct user_namespace *target_ns) { return true; diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index de461aa0bf9a..749147f5a613 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -101,6 +101,10 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns int i; int err; + err = -EINVAL; + if (!in_userns(parent_pid_ns->user_ns, user_ns)) + goto out; + err = -ENOSPC; if (level > MAX_PID_NS_LEVEL) goto out; diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index 2f735cbe05e8..7d8658fbabc8 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -986,19 +986,25 @@ bool userns_may_setgroups(const struct user_namespace *ns) } /* - * Returns true if @ns is the same namespace as or a descendant of - * @target_ns. + * Returns true if @child is the same namespace or a descendant of + * @ancestor. */ -bool current_in_userns(const struct user_namespace *target_ns) +bool in_userns(const struct user_namespace *ancestor, + const struct user_namespace *child) { - struct user_namespace *ns; - for (ns = current_user_ns(); ns; ns = ns->parent) { - if (ns == target_ns) + const struct user_namespace *ns; + for (ns = child; ns; ns = ns->parent) { + if (ns == ancestor) return true; } return false; } +bool current_in_userns(const struct user_namespace *target_ns) +{ + return in_userns(target_ns, current_user_ns()); +} + static inline struct user_namespace *to_user_ns(struct ns_common *ns) { return container_of(ns, struct user_namespace, ns); -- 2.10.1