All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman)
To: Linux Containers
	<containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>
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 <ktkhai-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>,
	agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Subject: [PATCH] userns,pidns: Verify the userns for new pid namespaces
Date: Sat, 29 Apr 2017 14:25:24 -0500	[thread overview]
Message-ID: <87vapnrp7f.fsf_-_@xmission.com> (raw)
In-Reply-To: <8737crt4dz.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> (Eric W. Biederman's message of "Sat, 29 Apr 2017 14:12:08 -0500")


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" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---

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

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Linux Containers <containers@lists.linux-foundation.org>
Cc: <serge@hallyn.com>, <agruenba@redhat.com>,
	<gregkh@linuxfoundation.org>, <linux-kernel@vger.kernel.org>,
	<oleg@redhat.com>, <paul@paul-moore.com>,
	<viro@zeniv.linux.org.uk>, <avagin@openvz.org>,
	<linux-api@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
	<mtk.manpages@gmail.com>, <akpm@linux-foundation.org>,
	<luto@amacapital.net>, <gorcunov@openvz.org>, <mingo@kernel.org>,
	<keescook@chromium.org>, Kirill Tkhai <ktkhai@virtuozzo.com>
Subject: [PATCH] userns,pidns: Verify the userns for new pid namespaces
Date: Sat, 29 Apr 2017 14:25:24 -0500	[thread overview]
Message-ID: <87vapnrp7f.fsf_-_@xmission.com> (raw)
In-Reply-To: <8737crt4dz.fsf@xmission.com> (Eric W. Biederman's message of "Sat, 29 Apr 2017 14:12:08 -0500")


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" <ebiederm@xmission.com>
---

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

  parent reply	other threads:[~2017-04-29 19:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-27 12:32 [PATCH v3] pid_ns: Introduce ioctl to set vector of ns_last_pid's on ns hierarhy Kirill Tkhai
2017-04-27 12:32 ` Kirill Tkhai
     [not found] ` <149329634856.21195.14196911999722279118.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-04-27 15:15   ` Eric W. Biederman
2017-04-27 15:15     ` Eric W. Biederman
     [not found]     ` <87mvb16fv7.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2017-04-27 15:52       ` Kirill Tkhai
2017-04-27 15:52         ` Kirill Tkhai
     [not found]         ` <12a73543-79ea-4bac-7e96-6ab237534af2-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>
2017-04-27 16:07           ` Eric W. Biederman
2017-04-27 16:07             ` Eric W. Biederman
     [not found]             ` <877f254yx0.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2017-04-28  9:13               ` Kirill Tkhai
2017-04-28  9:13                 ` Kirill Tkhai
     [not found]                 ` <a9941daf-861b-453c-37b8-e95389a9959b-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>
2017-04-29 19:12                   ` Eric W. Biederman
2017-04-29 19:12                     ` Eric W. Biederman
     [not found]                     ` <8737crt4dz.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2017-04-29 19:25                       ` Eric W. Biederman [this message]
2017-04-29 19:25                         ` [PATCH] userns,pidns: Verify the userns for new pid namespaces Eric W. Biederman
2017-04-29 20:53                         ` Serge E. Hallyn
     [not found]                           ` <20170429205325.GB1119-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
2017-04-30  4:33                             ` Eric W. Biederman
2017-04-30  4:33                               ` Eric W. Biederman
     [not found]                               ` <87a86yseej.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2017-04-30  4:42                                 ` Eric W. Biederman
2017-04-30  4:42                                   ` Eric W. Biederman
     [not found]                         ` <87vapnrp7f.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2017-04-29 20:53                           ` Serge E. Hallyn
2017-05-02 10:03                           ` Kirill Tkhai
2017-05-02 10:03                           ` Kirill Tkhai
2017-05-02 10:03                             ` Kirill Tkhai
2017-05-02 10:04                             ` Kirill Tkhai
2017-05-02 10:04                               ` Kirill Tkhai
     [not found]                               ` <fab323b8-a909-bbce-77d2-afbcd3b0452e-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>
2017-05-02 20:39                                 ` Eric W. Biederman
2017-05-02 20:39                                 ` Eric W. Biederman
2017-05-02 20:39                                   ` Eric W. Biederman
     [not found]                             ` <d67c8c06-2e6b-9cc0-df81-71d3cf4bb43d-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>
2017-05-02 10:04                               ` Kirill Tkhai
2017-05-02  9:53                     ` [PATCH v3] pid_ns: Introduce ioctl to set vector of ns_last_pid's on ns hierarhy Kirill Tkhai
2017-05-02  9:53                       ` Kirill Tkhai
     [not found]                       ` <d2b88252-d579-8deb-2c32-4118b8464b6b-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>
2017-05-02 21:26                         ` Eric W. Biederman
2017-05-02 21:26                           ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87vapnrp7f.fsf_-_@xmission.com \
    --to=ebiederm-as9lmozglivwk0htik3j/w@public.gmane.org \
    --cc=agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=gorcunov-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=ktkhai-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org \
    --cc=mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org \
    --cc=serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org \
    --cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.