[RFC PATCH v4 00/12] Second attempt at contained helper execution

[RFC PATCH v4 00/12] Second attempt at contained helper execution

[Ian Kent]

Here is another update to the attempt at contained helper execution.

The main change is I've tried to incorporate Oleg's suggestions
of directly constructing the namespaces rather than using the
open/setns approach and the addition of a namespace hash store.

I'm not particularly happy with this so far as there are a bunch
of ref counted objects and I've almost certainly got that wrong.
But also there are object lifetime problems, some I'm aware of
and for sure others I'm not. Also there is the integrity of the
thread runner process. I haven't performed a double fork on thread
execution, it might be painful to implement, so the thread runner
might end up with the wrong namespace setup if an error occurs.

Anyway, I've decided to stop spinning my wheels with this and
post an update in the hope that others can offer suggestions to
help and, of course, point out things I've missed.

The other change has been to the nfs and KEYS patches.
I've introduced the ability to get a token that can be used to
save namespace information for later execution and I've attempted
to use that for persistent namespace execution, as was discussed
previously.

I'm not at all sure I've done this in a sensible way but the
token does need to be accessible at helper execution time which
is why I've done it this way.

I definitely need advice here too. 

---

Ian Kent (12):
      nsproxy - make create_new_namespaces() non-static
      kmod - rename call_usermodehelper() flags parameter
      vfs - move mnt_namespace definition to linux/mount.h
      kmod - add namespace aware thread runner
      kmod - teach call_usermodehelper() to use a namespace
      kmod - add namespace info store
      kmod - add call_usermodehelper_ns()
      nfsd - use namespace if not executing in init namespace
      nfs - cache_lib use namespace if not executing in init namespace
      nfs - objlayout use namespace if not executing in init namespace
      KEYS - use correct memory allocation flag in call_usermodehelper_keys()
      KEYS: exec request-key within the requesting task's init namespace


 fs/mount.h                   |   12 -
 fs/nfs/cache_lib.c           |    7 +
 fs/nfs/objlayout/objlayout.c |    7 +
 fs/nfsd/netns.h              |    3 
 fs/nfsd/nfs4recover.c        |   48 +++-
 fs/nfsd/nfsctl.c             |    6 +
 include/linux/key.h          |    3 
 include/linux/kmod.h         |   56 +++++
 include/linux/mount.h        |   14 +
 include/linux/nsproxy.h      |    3 
 include/linux/sunrpc/cache.h |    2 
 kernel/kmod.c                |  465 ++++++++++++++++++++++++++++++++++++++++--
 kernel/nsproxy.c             |    2 
 net/sunrpc/cache.c           |    5 
 security/keys/gc.c           |    2 
 security/keys/key.c          |    4 
 security/keys/request_key.c  |   39 +++-
 17 files changed, 620 insertions(+), 58 deletions(-)

--
Ian

[RFC PATCH v4 01/12] nsproxy - make create_new_namespaces() non-static

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

create_new_namespaces() will be needed by usermodehelper namespace
restricted execution.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
 include/linux/nsproxy.h |    3 +++
 kernel/nsproxy.c        |    2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index 35fa08f..dfe7dda 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -62,6 +62,9 @@ extern struct nsproxy init_nsproxy;
  *
  */
 
+struct nsproxy *create_new_namespaces(unsigned long flags,
+	struct task_struct *tsk, struct user_namespace *user_ns,
+	struct fs_struct *new_fs);
 int copy_namespaces(unsigned long flags, struct task_struct *tsk);
 void exit_task_namespaces(struct task_struct *tsk);
 void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new);
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index 49746c8..48d5e4a 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -56,7 +56,7 @@ static inline struct nsproxy *create_nsproxy(void)
  * Return the newly created nsproxy.  Do not attach this to the task,
  * leave it to the caller to do proper locking and attach it to task.
  */
-static struct nsproxy *create_new_namespaces(unsigned long flags,
+struct nsproxy *create_new_namespaces(unsigned long flags,
 	struct task_struct *tsk, struct user_namespace *user_ns,
 	struct fs_struct *new_fs)
 {

[RFC PATCH v4 02/12] kmod - rename call_usermodehelper() flags parameter

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

The wait parameter of call_usermodehelper() is not quite a parameter
that describes the wait behaviour alone and will later be used to
request execution within the current namespaces. This flag is tied
to the wait field of the subprocess_info structure which is also
a field that doesn't specify wait behaviour alone and is used to
hold the passed flags information.

So change both the parameter and structure field name to flags.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 include/linux/kmod.h |    6 +++---
 kernel/kmod.c        |   32 +++++++++++++++++---------------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 0555cc6..e647ddb 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -59,7 +59,7 @@ struct subprocess_info {
 	char *path;
 	char **argv;
 	char **envp;
-	int wait;
+	unsigned int flags;
 	int retval;
 	int (*init)(struct subprocess_info *info, struct cred *new);
 	void (*cleanup)(struct subprocess_info *info);
@@ -67,7 +67,7 @@ struct subprocess_info {
 };
 
 extern int
-call_usermodehelper(char *path, char **argv, char **envp, int wait);
+call_usermodehelper(char *path, char **argv, char **envp, unsigned int flags);
 
 extern struct subprocess_info *
 call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask,
@@ -75,7 +75,7 @@ call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask,
 			  void (*cleanup)(struct subprocess_info *), void *data);
 
 extern int
-call_usermodehelper_exec(struct subprocess_info *info, int wait);
+call_usermodehelper_exec(struct subprocess_info *info, unsigned int flags);
 
 extern struct ctl_table usermodehelper_table[];
 
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 2777f40..e968e2d 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -259,7 +259,7 @@ static int ____call_usermodehelper(void *data)
 out:
 	sub_info->retval = retval;
 	/* wait_for_helper() will call umh_complete if UHM_WAIT_PROC. */
-	if (!(sub_info->wait & UMH_WAIT_PROC))
+	if (!(sub_info->flags & UMH_WAIT_PROC))
 		umh_complete(sub_info);
 	if (!retval)
 		return 0;
@@ -310,7 +310,7 @@ static void __call_usermodehelper(struct work_struct *work)
 		container_of(work, struct subprocess_info, work);
 	pid_t pid;
 
-	if (sub_info->wait & UMH_WAIT_PROC)
+	if (sub_info->flags & UMH_WAIT_PROC)
 		pid = kernel_thread(wait_for_helper, sub_info,
 				    CLONE_FS | CLONE_FILES | SIGCHLD);
 	else
@@ -525,16 +525,17 @@ EXPORT_SYMBOL(call_usermodehelper_setup);
 /**
  * call_usermodehelper_exec - start a usermode application
  * @sub_info: information about the subprocessa
- * @wait: wait for the application to finish and return status.
- *        when UMH_NO_WAIT don't wait at all, but you get no useful error back
- *        when the program couldn't be exec'ed. This makes it safe to call
- *        from interrupt context.
+ * @flags: flag to indicate whether to wait for the application to finish
+ * 	  and return status. If UMH_NO_WAIT is set don't wait at all, but
+ * 	  you get no useful error back when the program couldn't be exec'ed.
+ * 	  This makes it safe to call from interrupt context.
  *
  * Runs a user-space application.  The application is started
  * asynchronously if wait is not set, and runs as a child of keventd.
  * (ie. it runs with full root capabilities).
  */
-int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait)
+int call_usermodehelper_exec(struct subprocess_info *sub_info,
+			     unsigned int flags)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
 	int retval = 0;
@@ -553,14 +554,14 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait)
 	 * This makes it possible to use umh_complete to free
 	 * the data structure in case of UMH_NO_WAIT.
 	 */
-	sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &done;
-	sub_info->wait = wait;
+	sub_info->complete = (flags & UMH_NO_WAIT) ? NULL : &done;
+	sub_info->flags = flags;
 
 	queue_work(khelper_wq, &sub_info->work);
-	if (wait == UMH_NO_WAIT)	/* task has freed sub_info */
+	if (flags & UMH_NO_WAIT)	/* task has freed sub_info */
 		goto unlock;
 
-	if (wait & UMH_KILLABLE) {
+	if (flags & UMH_KILLABLE) {
 		retval = wait_for_completion_killable(&done);
 		if (!retval)
 			goto wait_done;
@@ -587,7 +588,7 @@ EXPORT_SYMBOL(call_usermodehelper_exec);
  * @path: path to usermode executable
  * @argv: arg vector for process
  * @envp: environment for process
- * @wait: wait for the application to finish and return status.
+ * @flags: wait for the application to finish and return status.
  *        when UMH_NO_WAIT don't wait at all, but you get no useful error back
  *        when the program couldn't be exec'ed. This makes it safe to call
  *        from interrupt context.
@@ -595,17 +596,18 @@ EXPORT_SYMBOL(call_usermodehelper_exec);
  * This function is the equivalent to use call_usermodehelper_setup() and
  * call_usermodehelper_exec().
  */
-int call_usermodehelper(char *path, char **argv, char **envp, int wait)
+int call_usermodehelper(char *path,
+			char **argv, char **envp, unsigned int flags)
 {
 	struct subprocess_info *info;
-	gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
+	gfp_t gfp_mask = (flags & UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL;
 
 	info = call_usermodehelper_setup(path, argv, envp, gfp_mask,
 					 NULL, NULL, NULL);
 	if (info == NULL)
 		return -ENOMEM;
 
-	return call_usermodehelper_exec(info, wait);
+	return call_usermodehelper_exec(info, flags);
 }
 EXPORT_SYMBOL(call_usermodehelper);
 

[RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

The mnt_namespace definition will be needed by the usermode helper
contained execution implementation, move it to include/linux/mount.h.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/mount.h            |   12 ------------
 include/linux/mount.h |   14 +++++++++++++-
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/mount.h b/fs/mount.h
index 6a61c2b..5b8423b 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -1,20 +1,8 @@
 #include <linux/mount.h>
 #include <linux/seq_file.h>
 #include <linux/poll.h>
-#include <linux/ns_common.h>
 #include <linux/fs_pin.h>
 
-struct mnt_namespace {
-	atomic_t		count;
-	struct ns_common	ns;
-	struct mount *	root;
-	struct list_head	list;
-	struct user_namespace	*user_ns;
-	u64			seq;	/* Sequence number to prevent loops */
-	wait_queue_head_t poll;
-	u64 event;
-};
-
 struct mnt_pcp {
 	int mnt_count;
 	int mnt_writers;
diff --git a/include/linux/mount.h b/include/linux/mount.h
index c2c561d..39dbcdf 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -15,11 +15,12 @@
 #include <linux/spinlock.h>
 #include <linux/seqlock.h>
 #include <linux/atomic.h>
+#include <linux/ns_common.h>
+#include <linux/wait.h>
 
 struct super_block;
 struct vfsmount;
 struct dentry;
-struct mnt_namespace;
 
 #define MNT_NOSUID	0x01
 #define MNT_NODEV	0x02
@@ -62,6 +63,17 @@ struct mnt_namespace;
 #define MNT_SYNC_UMOUNT		0x2000000
 #define MNT_MARKED		0x4000000
 
+struct mnt_namespace {
+	atomic_t		count;
+	struct ns_common	ns;
+	struct mount *	root;
+	struct list_head	list;
+	struct user_namespace	*user_ns;
+	u64			seq;	/* Sequence number to prevent loops */
+	wait_queue_head_t poll;
+	u64 event;
+};
+
 struct vfsmount {
 	struct dentry *mnt_root;	/* root of the mounted tree */
 	struct super_block *mnt_sb;	/* pointer to superblock */

[RFC PATCH v4 04/12] kmod - add namespace aware thread runner

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

Make usermode helper thread runner namespace aware.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 include/linux/kmod.h |   12 ++++++
 kernel/kmod.c        |   98 ++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index e647ddb..64c81c9 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -25,6 +25,7 @@
 #include <linux/compiler.h>
 #include <linux/workqueue.h>
 #include <linux/sysctl.h>
+#include <linux/user_namespace.h>
 
 #define KMOD_PATH_LEN 256
 
@@ -52,6 +53,14 @@ struct file;
 #define UMH_WAIT_EXEC	1	/* wait for the exec, but not the process */
 #define UMH_WAIT_PROC	2	/* wait for the process to complete */
 #define UMH_KILLABLE	4	/* wait for EXEC/PROC killable */
+#define UMH_USE_NS	32	/* exec using caller's init namespace */
+
+#ifdef CONFIG_NAMESPACES
+struct umh_ns_info {
+	struct nsproxy *nsproxy;
+	struct user_namespace *user_ns;
+};
+#endif
 
 struct subprocess_info {
 	struct work_struct work;
@@ -64,6 +73,9 @@ struct subprocess_info {
 	int (*init)(struct subprocess_info *info, struct cred *new);
 	void (*cleanup)(struct subprocess_info *info);
 	void *data;
+#ifdef CONFIG_NAMESPACES
+	struct umh_ns_info nsinfo;
+#endif
 };
 
 extern int
diff --git a/kernel/kmod.c b/kernel/kmod.c
index e968e2d..213dbe0 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -39,6 +39,7 @@
 #include <linux/rwsem.h>
 #include <linux/ptrace.h>
 #include <linux/async.h>
+#include <linux/proc_ns.h>
 #include <asm/uaccess.h>
 
 #include <trace/events/module.h>
@@ -303,11 +304,8 @@ static int wait_for_helper(void *data)
 	do_exit(0);
 }
 
-/* This is run by khelper thread  */
-static void __call_usermodehelper(struct work_struct *work)
+static pid_t umh_kernel_thread(struct subprocess_info *sub_info)
 {
-	struct subprocess_info *sub_info =
-		container_of(work, struct subprocess_info, work);
 	pid_t pid;
 
 	if (sub_info->flags & UMH_WAIT_PROC)
@@ -316,7 +314,99 @@ static void __call_usermodehelper(struct work_struct *work)
 	else
 		pid = kernel_thread(____call_usermodehelper, sub_info,
 				    SIGCHLD);
+	return pid;
+}
+
+#ifndef CONFIG_NAMESPACES
+static pid_t umh_kernel_thread_in_ns(struct subprocess_info *sub_info)
+{
+	return -ENOTSUP;
+}
+#else
+static pid_t umh_kernel_thread_in_ns(struct subprocess_info *sub_info)
+{
+	struct umh_ns_info *info = &sub_info->nsinfo;
+	struct nsproxy *saved_nsp, *new_nsp;
+	struct user_namespace *user_ns;
+	struct pid_namespace *pid_ns;
+	struct mnt_namespace *mnt_ns;
+	pid_t pid;
+	int err;
+
+	saved_nsp = current->nsproxy;
+	get_nsproxy(saved_nsp);
+
+	new_nsp = info->nsproxy;
+	get_nsproxy(new_nsp);
+
+	user_ns = get_user_ns(current->cred->user_ns);
+	if (info->user_ns) {
+		err = user_ns->ns.ops->install(new_nsp, &info->user_ns->ns);
+		if (err)
+			goto out;
+	}
+
+	/* May need to wait4() completion so a pid valid in the
+	 * thread runners namespace is needed. Install current
+	 * pid ns in the nsproxy.
+	 */
+	pid_ns = current->nsproxy->pid_ns_for_children;
+	if (pid_ns) {
+		err = pid_ns->ns.ops->install(new_nsp, &pid_ns->ns);
+		if (err)
+			goto out_user_ns;
+	}
+
+	/* The mount namespace install function is a little
+	 * more than a no-op, as the install functions of the
+	 * other namespace types are in our case, we need to
+	 * call it to setup current fs root and pwd.
+	 */
+	mnt_ns = current->nsproxy->mnt_ns;
+	err = mnt_ns->ns.ops->install(new_nsp, &new_nsp->mnt_ns->ns);
+	if (err)
+		goto out_user_ns;
 
+	/* Finally, switch to the nsproxy of the init namespace
+	 * of the caller.
+	 */
+	switch_task_namespaces(current, new_nsp);
+
+	pid = umh_kernel_thread(sub_info);
+
+	mnt_ns->ns.ops->install(saved_nsp, &saved_nsp->mnt_ns->ns);
+
+	if (info->user_ns)
+		user_ns->ns.ops->install(saved_nsp, &user_ns->ns);
+
+	switch_task_namespaces(current, saved_nsp);
+
+	put_user_ns(user_ns);
+
+	return pid;
+
+out_user_ns:
+	if (info->user_ns)
+		user_ns->ns.ops->install(saved_nsp, &user_ns->ns);
+out:
+	put_user_ns(user_ns);
+	put_nsproxy(new_nsp);
+	put_nsproxy(saved_nsp);
+	return err;
+}
+#endif
+
+/* This is run by khelper thread  */
+static void __call_usermodehelper(struct work_struct *work)
+{
+	struct subprocess_info *sub_info =
+		container_of(work, struct subprocess_info, work);
+	pid_t pid;
+
+	if (sub_info->flags & UMH_USE_NS)
+		pid = umh_kernel_thread_in_ns(sub_info);
+	else
+		pid = umh_kernel_thread(sub_info);
 	if (pid < 0) {
 		sub_info->retval = pid;
 		umh_complete(sub_info);

[RFC PATCH v4 05/12] kmod - teach call_usermodehelper() to use a namespace

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

The call_usermodehelper() function executes all binaries in the
global "init" root context. This doesn't allow a binary to be run
within a namespace (eg. the namespaces of a container).

The init process of the callers environment is used to setup the
namespaces in almost the same way the root init process when the
UMH_USE_NS flag is used.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 kernel/kmod.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/kernel/kmod.c b/kernel/kmod.c
index 213dbe0..d6ee21a 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -56,6 +56,8 @@ static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
 static DEFINE_SPINLOCK(umh_sysctl_lock);
 static DECLARE_RWSEM(umhelper_sem);
 
+static void umh_put_nsproxy(struct subprocess_info *);
+
 #ifdef CONFIG_MODULES
 
 /*
@@ -194,6 +196,7 @@ static void call_usermodehelper_freeinfo(struct subprocess_info *info)
 {
 	if (info->cleanup)
 		(*info->cleanup)(info);
+	umh_put_nsproxy(info);
 	kfree(info);
 }
 
@@ -565,6 +568,61 @@ static void helper_unlock(void)
 		wake_up(&running_helpers_waitq);
 }
 
+#ifndef CONFIG_NAMESPACES
+static int umh_get_nsproxy(struct subprocess_info *sub_info)
+{
+	return -ENOTSUP;
+}
+
+static void umh_put_nsproxy(struct subprocess_info *sub_info)
+{
+}
+#else
+static int umh_get_nsproxy(struct subprocess_info *sub_info)
+{
+	struct umh_ns_info *nsinfo = &sub_info->nsinfo;
+	struct task_struct *tsk;
+	struct user_namespace *user_ns;
+	struct nsproxy *new;
+	int err = 0;
+
+	rcu_read_lock();
+	tsk = find_task_by_vpid(1);
+	if (tsk)
+		get_task_struct(tsk);
+	rcu_read_unlock();
+	if (!tsk) {
+		err = -ESRCH;
+		goto out;
+	}
+
+	user_ns = get_user_ns(tsk->cred->user_ns);
+
+	new = create_new_namespaces(0, tsk, user_ns, tsk->fs);
+	if (IS_ERR(new)) {
+		err = PTR_ERR(new);
+		put_user_ns(user_ns);
+		put_task_struct(tsk);
+		goto out;
+	}
+
+	put_task_struct(tsk);
+
+	nsinfo->nsproxy = new;
+	nsinfo->user_ns = user_ns;
+out:
+	return err;
+}
+
+static void umh_put_nsproxy(struct subprocess_info *sub_info)
+{
+	if (sub_info->nsinfo.nsproxy) {
+		put_nsproxy(sub_info->nsinfo.nsproxy);
+		put_user_ns(sub_info->nsinfo.user_ns);
+	}
+}
+#endif
+
 /**
  * call_usermodehelper_setup - prepare to call a usermode helper
  * @path: path to usermode executable
@@ -697,6 +755,14 @@ int call_usermodehelper(char *path,
 	if (info == NULL)
 		return -ENOMEM;
 
+	if (flags & UMH_USE_NS) {
+		int err = umh_get_nsproxy(info);
+		if (err) {
+			kfree(info);
+			return err;
+		}
+	}
+
 	return call_usermodehelper_exec(info, flags);
 }
 EXPORT_SYMBOL(call_usermodehelper);

[RFC PATCH v4 06/12] kmod - add namespace info store

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

Persistent use of namespace information is needed where contained
execution is needed in a namespace other than the current namespace.

Use a simple random token as a key to store namespace information
in a hashed list for later usermode helper execution.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 include/linux/kmod.h |   14 ++++
 kernel/kmod.c        |  185 ++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 193 insertions(+), 6 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 64c81c9..77f41ce 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -78,6 +78,20 @@ struct subprocess_info {
 #endif
 };
 
+#ifndef CONFIG_NAMESPACES
+static inline long umh_ns_get_token(long token)
+{
+	return -ENOTSUP;
+}
+
+static inline void umh_ns_put_token(long token)
+{
+}
+#else
+extern long umh_ns_get_token(long token);
+extern void umh_ns_put_token(long token);
+#endif
+
 extern int
 call_usermodehelper(char *path, char **argv, char **envp, unsigned int flags);
 
diff --git a/kernel/kmod.c b/kernel/kmod.c
index d6ee21a..ddd41f1 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -41,6 +41,9 @@
 #include <linux/async.h>
 #include <linux/proc_ns.h>
 #include <asm/uaccess.h>
+#include <linux/hash.h>
+#include <linux/list.h>
+#include <linux/random.h>
 
 #include <trace/events/module.h>
 
@@ -48,6 +51,21 @@ extern int max_threads;
 
 static struct workqueue_struct *khelper_wq;
 
+#ifdef CONFIG_NAMESPACES
+#define UMH_HASH_SHIFT  6
+#define UMH_HASH_SIZE   1 << UMH_HASH_SHIFT
+
+struct umh_ns_entry {
+	long token;
+	unsigned int count;
+	struct umh_ns_info nsinfo;
+	struct hlist_node umh_ns_hlist;
+};
+
+static DEFINE_SPINLOCK(umh_ns_hash_lock);
+static struct hlist_head umh_ns_hash[UMH_HASH_SIZE];
+#endif
+
 #define CAP_BSET	(void *)1
 #define CAP_PI		(void *)2
 
@@ -577,10 +595,13 @@ static int umh_get_nsproxy(struct subprocess_info *sub_info)
 static void umh_put_nsproxy(struct subprocess_info *sub_info)
 {
 }
+
+static void umh_ns_hash_init(void)
+{
+}
 #else
-static int umh_get_nsproxy(struct subprocess_info *sub_info)
+static int _umh_get_nsproxy(struct umh_ns_info *nsinfo)
 {
-	struct umh_ns_info *nsinfo = &sub_info->nsinfo;
 	struct task_struct *tsk;
 	struct user_namespace *user_ns;
 	struct nsproxy *new;
@@ -614,14 +635,165 @@ out:
 	return err;
 }
 
+static int umh_get_nsproxy(struct subprocess_info *sub_info)
+{
+	return _umh_get_nsproxy(&sub_info->nsinfo);
+}
+
+static void _umh_put_nsproxy(struct umh_ns_info *nsinfo)
+{
+	if (nsinfo->nsproxy) {
+		put_nsproxy(nsinfo->nsproxy);
+		put_user_ns(nsinfo->user_ns);
+	}
+}
+
 static void umh_put_nsproxy(struct subprocess_info *sub_info)
 {
-	if (sub_info->nsinfo.nsproxy) {
-		put_nsproxy(sub_info->nsinfo.nsproxy);
-		put_user_ns(sub_info->nsinfo.user_ns);
+	return _umh_put_nsproxy(&sub_info->nsinfo);
+}
+
+static void umh_ns_hash_init(void)
+{
+	int i;
+
+	for (i = 0; i < UMH_HASH_SIZE; i++)
+		INIT_HLIST_HEAD(&umh_ns_hash[i]);
+}
+
+static struct umh_ns_entry *__umh_ns_find_entry(long token)
+{
+	struct umh_ns_entry *this, *entry;
+	struct hlist_head *bucket;
+	unsigned int hash;
+
+	hash = hash_64((unsigned long) token, UMH_HASH_SHIFT);
+	bucket = &umh_ns_hash[hash];
+
+	entry = ERR_PTR(-ENOENT);
+	if (hlist_empty(bucket))
+		goto out;
+
+	hlist_for_each_entry(this, bucket, umh_ns_hlist) {
+		if (this->token == token) {
+			entry = this;
+			break;
+		}
 	}
+out:
+	return entry;
 }
-#endif
+
+static struct umh_ns_entry *umh_ns_find_entry(long token, unsigned int nowait)
+{
+	struct umh_ns_entry *entry;
+	unsigned long flags;
+
+	if (nowait)
+		spin_lock_irqsave(&umh_ns_hash_lock, flags);
+	else
+		spin_lock(&umh_ns_hash_lock);
+	entry = __umh_ns_find_entry(token);
+	if (nowait)
+		spin_unlock_irqrestore(&umh_ns_hash_lock, flags);
+	else
+		spin_unlock(&umh_ns_hash_lock);
+
+	return entry;
+}
+
+/**
+ * umh_ns_get_token - allocate and store namespace information of the
+ * 			init process of the caller
+ * @token: token of stored namspace information or zero for a new
+ * 	   token.
+ *
+ * Returns a token used to locate the namespace information for calls to
+ * call_usermode_helper_ns() calls. On failure returns a negative errno.
+ */
+long umh_ns_get_token(long token)
+{
+	struct umh_ns_entry *entry;
+	struct hlist_head *bucket;
+	unsigned int hash;
+	unsigned int new_token;
+	int err;
+
+	if (token) {
+		spin_lock(&umh_ns_hash_lock);
+		entry = __umh_ns_find_entry(token);
+		if (entry) {
+			entry->count++;
+			spin_unlock(&umh_ns_hash_lock);
+			return token;
+		}
+		spin_unlock(&umh_ns_hash_lock);
+	}
+
+	entry = kzalloc(sizeof(struct umh_ns_entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	err = _umh_get_nsproxy(&entry->nsinfo);
+	if (err) {
+		kfree(entry);
+		return err;
+	}
+
+	do {
+		new_token = get_random_int();
+		if (!token)
+			continue;
+		spin_lock(&umh_ns_hash_lock);
+		entry = __umh_ns_find_entry(new_token);
+		if (likely(IS_ERR(entry))) {
+			hash = hash_32(new_token, UMH_HASH_SHIFT);
+			bucket = &umh_ns_hash[hash];
+			hlist_add_head(&entry->umh_ns_hlist, bucket);
+			entry->token = (long) new_token;
+			spin_unlock(&umh_ns_hash_lock);
+			break;
+		}
+		spin_unlock(&umh_ns_hash_lock);
+	} while (1);
+
+	return (long) new_token;
+}
+EXPORT_SYMBOL(umh_ns_get_token);
+
+/**
+ * umh_ns_put_token - remove and free a stored namespace corresponding to
+ * 			passed token
+ * @token: token of the stored namspace information.
+ *
+ * Returns 0 if the token is not longer in use or the token if it is in
+ * use.
+ */
+void umh_ns_put_token(long token)
+{
+	struct umh_ns_entry *entry;
+
+	if (!token)
+		return;
+
+	spin_lock(&umh_ns_hash_lock);
+	entry = __umh_ns_find_entry(token);
+	if (unlikely(IS_ERR(entry)))
+		spin_unlock(&umh_ns_hash_lock);
+	else {
+		if (--entry->count)
+			spin_unlock(&umh_ns_hash_lock);
+		else {
+			hlist_del(&entry->umh_ns_hlist);
+			spin_unlock(&umh_ns_hash_lock);
+			_umh_put_nsproxy(&entry->nsinfo);
+			kfree(entry);
+		}
+	}
+	return;
+}
+EXPORT_SYMBOL(umh_ns_put_token);
+#endif /* CONFIG_NAMESPACES */
 
 /**
  * call_usermodehelper_setup - prepare to call a usermode helper
@@ -849,4 +1021,5 @@ void __init usermodehelper_init(void)
 {
 	khelper_wq = create_singlethread_workqueue("khelper");
 	BUG_ON(!khelper_wq);
+	umh_ns_hash_init();
 }

[RFC PATCH v4 07/12] kmod - add call_usermodehelper_ns()

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

Add function call_usermodehelper_ns() to allow passing a namespace
token to lookup previously stored namespace information for usermode
helper execution.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 include/linux/kmod.h |   24 +++++++++++++
 kernel/kmod.c        |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 77f41ce..a761650 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -87,9 +87,33 @@ static inline long umh_ns_get_token(long token)
 static inline void umh_ns_put_token(long token)
 {
 }
+
+static inline int
+call_usermodehelper_ns(char *path, char **argv, char **envp,
+		       unsigned int flags, long token)
+{
+	return -ENOTSUP;
+}
+
+static inline struct subprocess_info *
+call_usermodehelper_setup_ns(char *path, char **argv, char **envp, gfp_t gfp_mask,
+			     int (*init)(struct subprocess_info *info, struct cred *new),
+			     void (*cleanup)(struct subprocess_info *), void *data,
+			     long token)
+{
+	return -ENOTSUP;
+}
 #else
 extern long umh_ns_get_token(long token);
 extern void umh_ns_put_token(long token);
+extern int
+call_usermodehelper_ns(char *path, char **argv, char **envp,
+		       unsigned int flags, long token);
+extern struct subprocess_info *
+call_usermodehelper_setup_ns(char *path, char **argv, char **envp, gfp_t gfp_mask,
+			     int (*init)(struct subprocess_info *info, struct cred *new),
+			     void (*cleanup)(struct subprocess_info *), void *data,
+			     long token);
 #endif
 
 extern int
diff --git a/kernel/kmod.c b/kernel/kmod.c
index ddd41f1..d711240 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -842,6 +842,62 @@ struct subprocess_info *call_usermodehelper_setup(char *path, char **argv,
 }
 EXPORT_SYMBOL(call_usermodehelper_setup);
 
+#ifdef CONFIG_NAMESPACES
+/**
+ * call_usermodehelper_setup_ns - prepare to call a usermode helper
+ * 			within a namespace
+ * @path: path to usermode executable
+ * @argv: arg vector for process
+ * @envp: environment for process
+ * @gfp_mask: gfp mask for memory allocation
+ * @cleanup: a cleanup function
+ * @init: an init function
+ * @data: arbitrary context sensitive data
+ * @token: token used to locate namespace setup.
+ *
+ * Returns either an errno error cast, or a subprocess_info structure.
+ * This should be passed to call_usermodehelper_exec to exec the process
+ * and free the structure.
+ *
+ * The init function is used to customize the helper process prior to
+ * exec.  A non-zero return code causes the process to error out, exit,
+ * and return the failure to the calling process
+ *
+ * The cleanup function is run just before the subprocess_info is about
+ * to be freed.  This can be used for freeing the argv and envp.  The
+ * Function must be runnable in either a process context or the
+ * context in which call_usermodehelper_exec is called.
+ */
+struct subprocess_info *call_usermodehelper_setup_ns(char *path, char **argv,
+		char **envp, gfp_t gfp_mask,
+		int (*init)(struct subprocess_info *info, struct cred *new),
+		void (*cleanup)(struct subprocess_info *info),
+		void *data, long token)
+{
+	struct subprocess_info *info;
+	unsigned int nowait = gfp_mask == GFP_ATOMIC ? 1 : 0;
+	struct umh_ns_entry *entry;
+
+	info = call_usermodehelper_setup(path, argv, envp,
+					 gfp_mask, NULL, NULL, NULL);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	entry = umh_ns_find_entry(token, nowait);
+	if (IS_ERR(entry)) {
+		kfree(info);
+		info = ERR_CAST(entry);
+		goto out;
+	}
+	get_nsproxy(entry->nsinfo.nsproxy);
+	info->nsinfo.nsproxy = entry->nsinfo.nsproxy;
+	info->nsinfo.user_ns = get_user_ns(entry->nsinfo.user_ns);
+out:
+	return info;
+}
+EXPORT_SYMBOL(call_usermodehelper_setup_ns);
+#endif /* CONFIG_NAMESPACES */
+
 /**
  * call_usermodehelper_exec - start a usermode application
  * @sub_info: information about the subprocessa
@@ -939,6 +995,46 @@ int call_usermodehelper(char *path,
 }
 EXPORT_SYMBOL(call_usermodehelper);
 
+#ifdef CONFIG_NAMESPACES
+/**
+ * call_usermodehelper_ns() - prepare and start a usermode application and
+ * 			execute using the stored namspace information
+ * 			corresponding to the passed token
+ * @path: path to usermode executable
+ * @argv: arg vector for process
+ * @envp: environment for process
+ * @flags: wait for the application to finish and return status.
+ *        when UMH_NO_WAIT don't wait at all, but you get no useful error back
+ *        when the program couldn't be exec'ed. This makes it safe to call
+ *        from interrupt context.
+ * @token: key of stored namespace to use or 0 to use the namespace of
+ * 	  init process of the caller.
+ *
+ * Returns 0 or an errno error if not successful.
+ */
+int call_usermodehelper_ns(char *path, char **argv, char **envp,
+			   unsigned int flags, long token)
+{
+	struct subprocess_info *info;
+	unsigned int nowait = flags & UMH_NO_WAIT;
+	gfp_t gfp_mask = nowait ? GFP_ATOMIC : GFP_KERNEL;
+
+	if (token < 0)
+		return -EINVAL;
+
+	if (!token)
+		return call_usermodehelper(path, argv, envp, flags|UMH_USE_NS);
+
+	info = call_usermodehelper_setup_ns(path, argv, envp,
+					    gfp_mask, NULL, NULL, NULL, token);
+	if (IS_ERR(info))
+		return PTR_ERR(info);
+
+	return call_usermodehelper_exec(info, flags|UMH_USE_NS);
+}
+EXPORT_SYMBOL(call_usermodehelper_ns);
+#endif /* CONFIG_NAMESPACES */
+
 static int proc_cap_handler(struct ctl_table *table, int write,
 			 void __user *buffer, size_t *lenp, loff_t *ppos)
 {

[RFC PATCH v4 08/12] nfsd - use namespace if not executing in init namespace

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

If nfsd is running within a container the client tracking operations
should run within the originating container also.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfsd/netns.h       |    3 +++
 fs/nfsd/nfs4recover.c |   48 +++++++++++++++++++++++++++++++-----------------
 fs/nfsd/nfsctl.c      |    6 ++++++
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index ea6749a..c85c13a 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -112,6 +112,9 @@ struct nfsd_net {
 	u32 clientid_counter;
 
 	struct svc_serv *nfsd_serv;
+
+	/* Namespace token */
+	long umh_token;
 };
 
 /* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 1c307f0..df13b54 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -1184,7 +1184,8 @@ nfsd4_cltrack_grace_start(time_t grace_start)
 }
 
 static int
-nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1)
+nfsd4_umh_cltrack_upcall(char *cmd, char *arg,
+			 char *env0, char *env1, long token)
 {
 	char *envp[3];
 	char *argv[4];
@@ -1209,7 +1210,11 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1)
 	argv[2] = arg;
 	argv[3] = NULL;
 
-	ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
+	if (token > 0)
+		ret = call_usermodehelper_ns(argv[0], argv, envp,
+					     UMH_WAIT_PROC, token);
+	else
+		ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
 	/*
 	 * Disable the upcall mechanism if we're getting an ENOENT or EACCES
 	 * error. The admin can re-enable it on the fly by using sysfs
@@ -1252,14 +1257,8 @@ nfsd4_umh_cltrack_init(struct net *net)
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 	char *grace_start = nfsd4_cltrack_grace_start(nn->boot_time);
 
-	/* XXX: The usermode helper s not working in container yet. */
-	if (net != &init_net) {
-		WARN(1, KERN_ERR "NFSD: attempt to initialize umh client "
-			"tracking in a container!\n");
-		return -EINVAL;
-	}
-
-	ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL);
+	ret = nfsd4_umh_cltrack_upcall("init", NULL,
+					grace_start, NULL, nn->umh_token);
 	kfree(grace_start);
 	return ret;
 }
@@ -1285,6 +1284,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)
 {
 	char *hexid, *has_session, *grace_start;
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+	int ret;
 
 	/*
 	 * With v4.0 clients, there's little difference in outcome between a
@@ -1312,7 +1312,10 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)
 	grace_start = nfsd4_cltrack_grace_start(nn->boot_time);
 
 	nfsd4_cltrack_upcall_lock(clp);
-	if (!nfsd4_umh_cltrack_upcall("create", hexid, has_session, grace_start))
+	ret = nfsd4_umh_cltrack_upcall("create",
+				       hexid, has_session, grace_start,
+				       nn->umh_token);
+	if (!ret)
 		set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
 	nfsd4_cltrack_upcall_unlock(clp);
 
@@ -1324,7 +1327,9 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)
 static void
 nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
 {
+	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 	char *hexid;
+	int ret;
 
 	if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
@@ -1336,9 +1341,13 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
 	}
 
 	nfsd4_cltrack_upcall_lock(clp);
-	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags) &&
-	    nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL) == 0)
-		clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
+	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) {
+		ret = nfsd4_umh_cltrack_upcall("remove",
+					       hexid, NULL, NULL,
+					       nn->umh_token);
+		if (ret == 0)
+			clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
+	}
 	nfsd4_cltrack_upcall_unlock(clp);
 
 	kfree(hexid);
@@ -1347,8 +1356,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
 static int
 nfsd4_umh_cltrack_check(struct nfs4_client *clp)
 {
-	int ret;
+	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 	char *hexid, *has_session, *legacy;
+	int ret;
 
 	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return 0;
@@ -1366,7 +1376,9 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp)
 	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) {
 		ret = 0;
 	} else {
-		ret = nfsd4_umh_cltrack_upcall("check", hexid, has_session, legacy);
+		ret = nfsd4_umh_cltrack_upcall("check", hexid,
+					       has_session, legacy,
+					       nn->umh_token);
 		if (ret == 0)
 			set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
 	}
@@ -1386,7 +1398,9 @@ nfsd4_umh_cltrack_grace_done(struct nfsd_net *nn)
 
 	sprintf(timestr, "%ld", nn->boot_time);
 	legacy = nfsd4_cltrack_legacy_topdir();
-	nfsd4_umh_cltrack_upcall("gracedone", timestr, legacy, NULL);
+	nfsd4_umh_cltrack_upcall("gracedone",
+				 timestr, legacy, NULL,
+				 nn->umh_token);
 	kfree(legacy);
 }
 
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index aa47d75..0509b65 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1224,6 +1224,8 @@ static __net_init int nfsd_init_net(struct net *net)
 		goto out_idmap_error;
 	nn->nfsd4_lease = 90;	/* default lease time */
 	nn->nfsd4_grace = 90;
+	if (net != &init_net)
+		nn->umh_token = umh_ns_get_token(nn->umh_token);
 	return 0;
 
 out_idmap_error:
@@ -1234,6 +1236,10 @@ out_export_error:
 
 static __net_exit void nfsd_exit_net(struct net *net)
 {
+	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+
+	umh_ns_put_token(nn->umh_token);
+	nn->umh_token = 0;
 	nfsd_idmap_shutdown(net);
 	nfsd_export_shutdown(net);
 }

[RFC PATCH v4 09/12] nfs - cache_lib use namespace if not executing in init namespace

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

If pipefs is registered within a namespace other than the root init
namespace subsequent pipefs requests should be run within the init
namespace of registration.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/cache_lib.c           |    7 ++++++-
 include/linux/sunrpc/cache.h |    2 ++
 net/sunrpc/cache.c           |    5 +++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/cache_lib.c b/fs/nfs/cache_lib.c
index 5f7b053..4f381ad 100644
--- a/fs/nfs/cache_lib.c
+++ b/fs/nfs/cache_lib.c
@@ -48,7 +48,12 @@ int nfs_cache_upcall(struct cache_detail *cd, char *entry_name)
 
 	if (nfs_cache_getent_prog[0] == '\0')
 		goto out;
-	ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
+	if (cd->u.pipefs.umh_token) {
+		long token = cd->u.pipefs.umh_token;
+		ret = call_usermodehelper_ns(argv[0], argv, envp,
+					     UMH_WAIT_EXEC, token);
+	} else
+		ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
 	/*
 	 * Disable the upcall mechanism if we're getting an ENOENT or
 	 * EACCES error. The admin can re-enable it on the fly by using
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 437ddb6..f6c1eb2 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -68,6 +68,8 @@ struct cache_detail_procfs {
 
 struct cache_detail_pipefs {
 	struct dentry *dir;
+	/* Namespace token */
+	long umh_token;
 };
 
 struct cache_detail {
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 5199bb1..a635efb 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1811,6 +1811,9 @@ int sunrpc_cache_register_pipefs(struct dentry *parent,
 	if (IS_ERR(dir))
 		return PTR_ERR(dir);
 	cd->u.pipefs.dir = dir;
+	if (cd->net != &init_net)
+		cd->u.pipefs.umh_token =
+			 umh_ns_get_token(cd->u.pipefs.umh_token);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(sunrpc_cache_register_pipefs);
@@ -1819,6 +1822,8 @@ void sunrpc_cache_unregister_pipefs(struct cache_detail *cd)
 {
 	rpc_remove_cache_dir(cd->u.pipefs.dir);
 	cd->u.pipefs.dir = NULL;
+	umh_ns_put_token(cd->u.pipefs.umh_token);
+	cd->u.pipefs.umh_token = 0;
 }
 EXPORT_SYMBOL_GPL(sunrpc_cache_unregister_pipefs);
 

[RFC PATCH v4 10/12] nfs - objlayout use namespace if not executing in init namespace

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

If the caller is running within a container then execute the usermode
helper callback within the init namespace of the container.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/objlayout/objlayout.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index 919efd4..00c9a34 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -599,9 +599,14 @@ static int __objlayout_upcall(struct __auto_login *login)
 		"PATH=/sbin:/usr/sbin:/bin:/usr/bin",
 		NULL
 	};
+	unsigned int umh_flags = UMH_WAIT_PROC;
 	char *argv[8];
 	int ret;
 
+	/* If running within a container use the container namespace */
+	if (current->nsproxy->net_ns != &init_net)
+		umh_flags |= UMH_USE_NS;
+
 	if (unlikely(!osd_login_prog[0])) {
 		dprintk("%s: osd_login_prog is disabled\n", __func__);
 		return -EACCES;
@@ -620,7 +625,7 @@ static int __objlayout_upcall(struct __auto_login *login)
 	argv[6] = login->systemid_hex;
 	argv[7] = NULL;
 
-	ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
+	ret = call_usermodehelper(argv[0], argv, envp, umh_flags);
 	/*
 	 * Disable the upcall mechanism if we're getting an ENOENT or
 	 * EACCES error. The admin can re-enable it on the fly by using

[RFC PATCH v4 11/12] KEYS - use correct memory allocation flag in call_usermodehelper_keys()

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

When call_usermodehelper_keys() is called it assumes it won't be called
with the flag UMH_NO_WAIT. Currently that's always the case.

Change this to check the flag and use the correct kernel memory allocation
flag to guard against future changes.

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 security/keys/request_key.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 486ef6f..e865f9f 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -76,8 +76,10 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp,
 					struct key *session_keyring, int wait)
 {
 	struct subprocess_info *info;
+	unsigned int gfp_mask = (wait & UMH_NO_WAIT) ?
+					GFP_ATOMIC : GFP_KERNEL;
 
-	info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL,
+	info = call_usermodehelper_setup(path, argv, envp, gfp_mask,
 					  umh_keys_init, umh_keys_cleanup,
 					  session_keyring);
 	if (!info)

[RFC PATCH v4 12/12] KEYS: exec request-key within the requesting task's init namespace

[Ian Kent]

From: Ian Kent <ikent@redhat.com>

Containerized request key helper callbacks need the ability to execute
a binary in a container's context. To do this calling an in kernel
equivalent of setns(2) should be sufficient since the user mode helper
execution kernel thread ultimately calls do_execve().

Signed-off-by: Ian Kent <ikent@redhat.com>
Cc: Benjamin Coddington <bcodding@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Oleg Nesterov <onestero@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jeff Layton <jeff.layton@primarydata.com>
---
 include/linux/key.h         |    3 +++
 security/keys/gc.c          |    2 ++
 security/keys/key.c         |    4 ++++
 security/keys/request_key.c |   35 +++++++++++++++++++++++++++++++++--
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index e1d4715..89dc2d7 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -209,6 +209,9 @@ struct key {
 		} payload;
 		struct assoc_array keys;
 	};
+
+	/* Namespace token */
+	long umh_token;
 };
 
 extern struct key *key_alloc(struct key_type *type,
diff --git a/security/keys/gc.c b/security/keys/gc.c
index c795237..57a0730 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -156,6 +156,8 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 
 		kfree(key->description);
 
+		umh_ns_put_token(key->umh_token);
+
 #ifdef KEY_DEBUGGING
 		key->magic = KEY_DEBUG_MAGIC_X;
 #endif
diff --git a/security/keys/key.c b/security/keys/key.c
index aee2ec5..e7ab89d 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -18,6 +18,7 @@
 #include <linux/workqueue.h>
 #include <linux/random.h>
 #include <linux/err.h>
+#include <net/net_namespace.h>
 #include "internal.h"
 
 struct kmem_cache *key_jar;
@@ -309,6 +310,9 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	/* publish the key by giving it a serial number */
 	atomic_inc(&user->nkeys);
 	key_alloc_serial(key);
+	/* If running within a container use the container namespace */
+	if (current->nsproxy->net_ns != &init_net)
+		key->umh_token = umh_ns_get_token(0);
 
 error:
 	return key;
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index e865f9f..16ac3b0 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -90,6 +90,31 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp,
 }
 
 /*
+ * Call a usermode helper with a specific session keyring and execute
+ * within a namespace.
+ */
+static int call_usermodehelper_keys_ns(char *path, char **argv, char **envp,
+					struct key *session_keyring,
+					unsigned int wait, long token)
+{
+	struct subprocess_info *info;
+	unsigned int gfp_mask = (wait & UMH_NO_WAIT) ?
+					GFP_ATOMIC : GFP_KERNEL;
+
+	if (token <= 0)
+		return -EINVAL;
+
+	info = call_usermodehelper_setup_ns(path, argv, envp, gfp_mask,
+					    umh_keys_init, umh_keys_cleanup,
+					    session_keyring, token);
+	if (!info)
+		return -ENOMEM;
+
+	key_get(session_keyring);
+	return call_usermodehelper_exec(info, wait|UMH_USE_NS);
+}
+
+/*
  * Request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"
  */
@@ -104,6 +129,7 @@ static int call_sbin_request_key(struct key_construction *cons,
 	char *argv[9], *envp[3], uid_str[12], gid_str[12];
 	char key_str[12], keyring_str[3][12];
 	char desc[20];
+	unsigned int wait = UMH_WAIT_PROC;
 	int ret, i;
 
 	kenter("{%d},{%d},%s", key->serial, authkey->serial, op);
@@ -174,8 +200,13 @@ static int call_sbin_request_key(struct key_construction *cons,
 	argv[i] = NULL;
 
 	/* do it */
-	ret = call_usermodehelper_keys(argv[0], argv, envp, keyring,
-				       UMH_WAIT_PROC);
+	/* If running within a container use the container namespace */
+	if (key->umh_token)
+		ret = call_usermodehelper_keys_ns(argv[0], argv, envp,
+					       keyring, wait, key->umh_token);
+	else
+		ret = call_usermodehelper_keys(argv[0],
+					       argv, envp, keyring, wait);
 	kdebug("usermode -> 0x%x", ret);
 	if (ret >= 0) {
 		/* ret is the exit/wait code */

Re: [RFC PATCH v4 00/12] Second attempt at contained helper execution

[J. Bruce Fields]

On Tue, Mar 17, 2015 at 10:44:49AM +0800, Ian Kent wrote:
> The other change has been to the nfs and KEYS patches.
> I've introduced the ability to get a token that can be used to
> save namespace information for later execution and I've attempted
> to use that for persistent namespace execution, as was discussed
> previously.

The nfsd patch at least definitely looks like what we need.

--b.

Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h

[Al Viro]

On Tue, Mar 17, 2015 at 10:45:09AM +0800, Ian Kent wrote:
> From: Ian Kent <ikent@redhat.com>
> 
> The mnt_namespace definition will be needed by the usermode helper
> contained execution implementation, move it to include/linux/mount.h.

I really don't like that.  AFAICS, the root of the evil is that fscking
nsproxy keeps a pointer to mnt_namespace while all it really needs to
know about is the address of ns_common field embedded into it.  Let's see...

fs/namespace.c:697:     struct mnt_namespace *ns = current->nsproxy->mnt_ns;
fs/namespace.c:770:     return mnt->mnt_ns == current->nsproxy->mnt_ns;
fs/namespace.c:1502:    return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
fs/namespace.c:1587:    return current->nsproxy->mnt_ns->seq >= mnt_ns->seq;
fs/namespace.c:2293:    struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns;
fs/namespace.c:2961:    touch_mnt_namespace(current->nsproxy->mnt_ns);
fs/namespace.c:3003:    init_task.nsproxy->mnt_ns = ns;
fs/namespace.c:3101:    ns_root.mnt = &current->nsproxy->mnt_ns->root->mnt;
fs/namespace.c:3119:    struct mnt_namespace *ns = current->nsproxy->mnt_ns;
fs/namespace.c:3159:            ns = &nsproxy->mnt_ns->ns;
fs/namespace.c:3187:    put_mnt_ns(nsproxy->mnt_ns);
fs/namespace.c:3188:    nsproxy->mnt_ns = mnt_ns;
fs/pnode.c:282: user_ns = current->nsproxy->mnt_ns->user_ns;
fs/proc_namespace.c:246:        if (!nsp || !nsp->mnt_ns) {
fs/proc_namespace.c:251:        ns = nsp->mnt_ns;
include/linux/nsproxy.h:33:     struct mnt_namespace *mnt_ns;
kernel/nsproxy.c:37:    .mnt_ns                 = NULL,
kernel/nsproxy.c:70:    new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
kernel/nsproxy.c:71:    if (IS_ERR(new_nsp->mnt_ns)) {
kernel/nsproxy.c:72:            err = PTR_ERR(new_nsp->mnt_ns);
kernel/nsproxy.c:113:   if (new_nsp->mnt_ns)
kernel/nsproxy.c:114:           put_mnt_ns(new_nsp->mnt_ns);
kernel/nsproxy.c:160:   if (ns->mnt_ns)
kernel/nsproxy.c:161:           put_mnt_ns(ns->mnt_ns);

OK, so we need

a) add in fs/mount.h
static inline struct mnt_namespace *current_mnt_ns(void)
{
	return current->nsproxy->mnt_ns;
}

and replace a lot of open-coded instances.

b) lift to_mnt_ns() into fs/mount.h (as static inline)

c) switch put_mnt_ns() to struct ns_common *, replacing put_mnt_ns(ns)
with put_mnt_ns(&ns->ns) and using to_mnt_ns() in the body to recover
the original.

d) make copy_mnt_ns() take and return struct ns_common * (same treatment as for
put_mnt_ns() in (c); replace
        new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
        if (IS_ERR(new_nsp->mnt_ns)) {
                err = PTR_ERR(new_nsp->mnt_ns);
                goto out_ns;
        }
with
	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
	if (IS_ERR(ns)) {
		err = PTR_ERR(ns);
		goto out_ns;
	}
	new_nsp->mnt_ns = to_mnt_ns(ns);

e) replace struct mnt_namespace *mnt_ns with struct ns_common *__mnt_ns in
nsproxy.h, deal with users of mnt_ns by
* everywhere in the tree replace put_mnt_ns(&...->mnt_ns->ns) with
put_mnt_ns(...->__mnt_ns) (i.e. modify the lines modified by (c) again).
* in fs/namespace.c replace
	init_task.nsproxy->mnt_ns = ns;
with
	init_task.nsproxy->__mnt_ns = &ns->ns;
replace
	ns = &nsproxy->mnt_ns->ns;
with
	ns = nsproxy->__mnt_ns;
replace
	nsproxy->mnt_ns = mnt_ns;
with
	nsproxy>__mnt_ns = &mnt_ns->ns;
* in fs/proc_namespace.c replace
	ns = nsp->mnt_ns;
with
	ns = to_mnt_ns(nsp->__mnt_ns);
and
	if (!nsp || !nsp->mnt_ns) {
with
	if (!nsp || !nsp->__mnt_ns) {
* in kernel/nsproxy.c: replace
	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
added in (d) with
	ns = copy_mnt_ns(flags, tsk->nsproxy->__mnt_ns, user_ns, new_fs);
and
	new_nsp->mnt_ns = to_mnt_ns(ns);
with
	new_nsp->__mnt_ns = ns;
The rest of mnt_ns in that file get replaced by __mnt_ns.
* in current_mnt_ns() replace ...->mnt_ns with to_mnt_ns(...->__mnt_ns)

Do you want me to push such a series in vfs.git?  After such 5 commits we have
no pointers to struct mnt_namespace in nsproxy.h *and* no need for your patches
to dig into ->mnt_ns->ns - we already have that as ->__mnt_ns.

Re: [RFC PATCH v4 00/12] Second attempt at contained helper execution

[Eric W. Biederman]

Ian Kent <raven@themaw.net> writes:

> Here is another update to the attempt at contained helper execution.
>
> The main change is I've tried to incorporate Oleg's suggestions
> of directly constructing the namespaces rather than using the
> open/setns approach and the addition of a namespace hash store.
>
> I'm not particularly happy with this so far as there are a bunch
> of ref counted objects and I've almost certainly got that wrong.
> But also there are object lifetime problems, some I'm aware of
> and for sure others I'm not. Also there is the integrity of the
> thread runner process. I haven't performed a double fork on thread
> execution, it might be painful to implement, so the thread runner
> might end up with the wrong namespace setup if an error occurs.
>
> Anyway, I've decided to stop spinning my wheels with this and
> post an update in the hope that others can offer suggestions to
> help and, of course, point out things I've missed.
>
> The other change has been to the nfs and KEYS patches.
> I've introduced the ability to get a token that can be used to
> save namespace information for later execution and I've attempted
> to use that for persistent namespace execution, as was discussed
> previously.
>
> I'm not at all sure I've done this in a sensible way but the
> token does need to be accessible at helper execution time which
> is why I've done it this way.
>
> I definitely need advice here too. 

As far as I can tell this patchset continues to be broken for ignoring
my earlier advice.

This patchset provides an escape from cgroup, lsm, rlimit, and
seccomp policy.

This patchset does not appear particularly nice in how it uses
namespaces.

The only safe and sane way to do this is to have a kernel thread with
all of the proper attributes configured waiting around ready to start
the user mode helper.

The problem you are trying to solve is so hard that we totally failed to
solve it outside of the container case.  Which is why we have kthreadd.
I will be very surprised if you can figure out how to cleanly solve the
problem the way you are attacking it.

Eric

Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h

[Ian Kent]

On Thu, 2015-03-19 at 19:47 +0000, Al Viro wrote:
> On Tue, Mar 17, 2015 at 10:45:09AM +0800, Ian Kent wrote:
> > From: Ian Kent <ikent@redhat.com>
> > 
> > The mnt_namespace definition will be needed by the usermode helper
> > contained execution implementation, move it to include/linux/mount.h.
> 
> I really don't like that.  AFAICS, the root of the evil is that fscking
> nsproxy keeps a pointer to mnt_namespace while all it really needs to
> know about is the address of ns_common field embedded into it.  Let's see...

Thought that might be the case.

> 
> fs/namespace.c:697:     struct mnt_namespace *ns = current->nsproxy->mnt_ns;
> fs/namespace.c:770:     return mnt->mnt_ns == current->nsproxy->mnt_ns;
> fs/namespace.c:1502:    return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
> fs/namespace.c:1587:    return current->nsproxy->mnt_ns->seq >= mnt_ns->seq;
> fs/namespace.c:2293:    struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns;
> fs/namespace.c:2961:    touch_mnt_namespace(current->nsproxy->mnt_ns);
> fs/namespace.c:3003:    init_task.nsproxy->mnt_ns = ns;
> fs/namespace.c:3101:    ns_root.mnt = &current->nsproxy->mnt_ns->root->mnt;
> fs/namespace.c:3119:    struct mnt_namespace *ns = current->nsproxy->mnt_ns;
> fs/namespace.c:3159:            ns = &nsproxy->mnt_ns->ns;
> fs/namespace.c:3187:    put_mnt_ns(nsproxy->mnt_ns);
> fs/namespace.c:3188:    nsproxy->mnt_ns = mnt_ns;
> fs/pnode.c:282: user_ns = current->nsproxy->mnt_ns->user_ns;
> fs/proc_namespace.c:246:        if (!nsp || !nsp->mnt_ns) {
> fs/proc_namespace.c:251:        ns = nsp->mnt_ns;
> include/linux/nsproxy.h:33:     struct mnt_namespace *mnt_ns;
> kernel/nsproxy.c:37:    .mnt_ns                 = NULL,
> kernel/nsproxy.c:70:    new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
> kernel/nsproxy.c:71:    if (IS_ERR(new_nsp->mnt_ns)) {
> kernel/nsproxy.c:72:            err = PTR_ERR(new_nsp->mnt_ns);
> kernel/nsproxy.c:113:   if (new_nsp->mnt_ns)
> kernel/nsproxy.c:114:           put_mnt_ns(new_nsp->mnt_ns);
> kernel/nsproxy.c:160:   if (ns->mnt_ns)
> kernel/nsproxy.c:161:           put_mnt_ns(ns->mnt_ns);
> 
> OK, so we need
> 
> a) add in fs/mount.h
> static inline struct mnt_namespace *current_mnt_ns(void)
> {
> 	return current->nsproxy->mnt_ns;
> }
> 
> and replace a lot of open-coded instances.
> 
> b) lift to_mnt_ns() into fs/mount.h (as static inline)
> 
> c) switch put_mnt_ns() to struct ns_common *, replacing put_mnt_ns(ns)
> with put_mnt_ns(&ns->ns) and using to_mnt_ns() in the body to recover
> the original.
> 
> d) make copy_mnt_ns() take and return struct ns_common * (same treatment as for
> put_mnt_ns() in (c); replace
>         new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
>         if (IS_ERR(new_nsp->mnt_ns)) {
>                 err = PTR_ERR(new_nsp->mnt_ns);
>                 goto out_ns;
>         }
> with
> 	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
> 	if (IS_ERR(ns)) {
> 		err = PTR_ERR(ns);
> 		goto out_ns;
> 	}
> 	new_nsp->mnt_ns = to_mnt_ns(ns);
> 
> e) replace struct mnt_namespace *mnt_ns with struct ns_common *__mnt_ns in
> nsproxy.h, deal with users of mnt_ns by
> * everywhere in the tree replace put_mnt_ns(&...->mnt_ns->ns) with
> put_mnt_ns(...->__mnt_ns) (i.e. modify the lines modified by (c) again).
> * in fs/namespace.c replace
> 	init_task.nsproxy->mnt_ns = ns;
> with
> 	init_task.nsproxy->__mnt_ns = &ns->ns;
> replace
> 	ns = &nsproxy->mnt_ns->ns;
> with
> 	ns = nsproxy->__mnt_ns;
> replace
> 	nsproxy->mnt_ns = mnt_ns;
> with
> 	nsproxy>__mnt_ns = &mnt_ns->ns;
> * in fs/proc_namespace.c replace
> 	ns = nsp->mnt_ns;
> with
> 	ns = to_mnt_ns(nsp->__mnt_ns);
> and
> 	if (!nsp || !nsp->mnt_ns) {
> with
> 	if (!nsp || !nsp->__mnt_ns) {
> * in kernel/nsproxy.c: replace
> 	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
> added in (d) with
> 	ns = copy_mnt_ns(flags, tsk->nsproxy->__mnt_ns, user_ns, new_fs);
> and
> 	new_nsp->mnt_ns = to_mnt_ns(ns);
> with
> 	new_nsp->__mnt_ns = ns;
> The rest of mnt_ns in that file get replaced by __mnt_ns.
> * in current_mnt_ns() replace ...->mnt_ns with to_mnt_ns(...->__mnt_ns)
> 
> Do you want me to push such a series in vfs.git?  After such 5 commits we have
> no pointers to struct mnt_namespace in nsproxy.h *and* no need for your patches
> to dig into ->mnt_ns->ns - we already have that as ->__mnt_ns.

Yes please, I'd be more confident if you did this than me, there's
already enough to worry about with the series.

Ian

Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h

[Eric W. Biederman]

Ian Kent <raven@themaw.net> writes:

2> On Thu, 2015-03-19 at 19:47 +0000, Al Viro wrote:
>> On Tue, Mar 17, 2015 at 10:45:09AM +0800, Ian Kent wrote:
>> > From: Ian Kent <ikent@redhat.com>
>> > 
>> > The mnt_namespace definition will be needed by the usermode helper
>> > contained execution implementation, move it to include/linux/mount.h.
>> 
>> I really don't like that.  AFAICS, the root of the evil is that fscking
>> nsproxy keeps a pointer to mnt_namespace while all it really needs to
>> know about is the address of ns_common field embedded into it.  Let's see...
>
> Thought that might be the case.
>
>> 
>> fs/namespace.c:697:     struct mnt_namespace *ns = current->nsproxy->mnt_ns;
>> fs/namespace.c:770:     return mnt->mnt_ns == current->nsproxy->mnt_ns;
>> fs/namespace.c:1502:    return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
>> fs/namespace.c:1587:    return current->nsproxy->mnt_ns->seq >= mnt_ns->seq;
>> fs/namespace.c:2293:    struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns;
>> fs/namespace.c:2961:    touch_mnt_namespace(current->nsproxy->mnt_ns);
>> fs/namespace.c:3003:    init_task.nsproxy->mnt_ns = ns;
>> fs/namespace.c:3101:    ns_root.mnt = &current->nsproxy->mnt_ns->root->mnt;
>> fs/namespace.c:3119:    struct mnt_namespace *ns = current->nsproxy->mnt_ns;
>> fs/namespace.c:3159:            ns = &nsproxy->mnt_ns->ns;
>> fs/namespace.c:3187:    put_mnt_ns(nsproxy->mnt_ns);
>> fs/namespace.c:3188:    nsproxy->mnt_ns = mnt_ns;
>> fs/pnode.c:282: user_ns = current->nsproxy->mnt_ns->user_ns;
>> fs/proc_namespace.c:246:        if (!nsp || !nsp->mnt_ns) {
>> fs/proc_namespace.c:251:        ns = nsp->mnt_ns;
>> include/linux/nsproxy.h:33:     struct mnt_namespace *mnt_ns;
>> kernel/nsproxy.c:37:    .mnt_ns                 = NULL,
>> kernel/nsproxy.c:70:    new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
>> kernel/nsproxy.c:71:    if (IS_ERR(new_nsp->mnt_ns)) {
>> kernel/nsproxy.c:72:            err = PTR_ERR(new_nsp->mnt_ns);
>> kernel/nsproxy.c:113:   if (new_nsp->mnt_ns)
>> kernel/nsproxy.c:114:           put_mnt_ns(new_nsp->mnt_ns);
>> kernel/nsproxy.c:160:   if (ns->mnt_ns)
>> kernel/nsproxy.c:161:           put_mnt_ns(ns->mnt_ns);
>> 
>> OK, so we need
>> 
>> a) add in fs/mount.h
>> static inline struct mnt_namespace *current_mnt_ns(void)
>> {
>> 	return current->nsproxy->mnt_ns;
>> }
>> 
>> and replace a lot of open-coded instances.
>> 
>> b) lift to_mnt_ns() into fs/mount.h (as static inline)
>> 
>> c) switch put_mnt_ns() to struct ns_common *, replacing put_mnt_ns(ns)
>> with put_mnt_ns(&ns->ns) and using to_mnt_ns() in the body to recover
>> the original.
>> 
>> d) make copy_mnt_ns() take and return struct ns_common * (same treatment as for
>> put_mnt_ns() in (c); replace
>>         new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
>>         if (IS_ERR(new_nsp->mnt_ns)) {
>>                 err = PTR_ERR(new_nsp->mnt_ns);
>>                 goto out_ns;
>>         }
>> with
>> 	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
>> 	if (IS_ERR(ns)) {
>> 		err = PTR_ERR(ns);
>> 		goto out_ns;
>> 	}
>> 	new_nsp->mnt_ns = to_mnt_ns(ns);
>> 
>> e) replace struct mnt_namespace *mnt_ns with struct ns_common *__mnt_ns in
>> nsproxy.h, deal with users of mnt_ns by
>> * everywhere in the tree replace put_mnt_ns(&...->mnt_ns->ns) with
>> put_mnt_ns(...->__mnt_ns) (i.e. modify the lines modified by (c) again).
>> * in fs/namespace.c replace
>> 	init_task.nsproxy->mnt_ns = ns;
>> with
>> 	init_task.nsproxy->__mnt_ns = &ns->ns;
>> replace
>> 	ns = &nsproxy->mnt_ns->ns;
>> with
>> 	ns = nsproxy->__mnt_ns;
>> replace
>> 	nsproxy->mnt_ns = mnt_ns;
>> with
>> 	nsproxy>__mnt_ns = &mnt_ns->ns;
>> * in fs/proc_namespace.c replace
>> 	ns = nsp->mnt_ns;
>> with
>> 	ns = to_mnt_ns(nsp->__mnt_ns);
>> and
>> 	if (!nsp || !nsp->mnt_ns) {
>> with
>> 	if (!nsp || !nsp->__mnt_ns) {
>> * in kernel/nsproxy.c: replace
>> 	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
>> added in (d) with
>> 	ns = copy_mnt_ns(flags, tsk->nsproxy->__mnt_ns, user_ns, new_fs);
>> and
>> 	new_nsp->mnt_ns = to_mnt_ns(ns);
>> with
>> 	new_nsp->__mnt_ns = ns;
>> The rest of mnt_ns in that file get replaced by __mnt_ns.
>> * in current_mnt_ns() replace ...->mnt_ns with to_mnt_ns(...->__mnt_ns)
>> 
>> Do you want me to push such a series in vfs.git?  After such 5 commits we have
>> no pointers to struct mnt_namespace in nsproxy.h *and* no need for your patches
>> to dig into ->mnt_ns->ns - we already have that as ->__mnt_ns.
>
> Yes please, I'd be more confident if you did this than me, there's
> already enough to worry about with the series.

Given that this patchset is a security hole waiting to happen I don't
see why Al should bother unless there are good reasons to do this
otherwise.

Eric

Re: [RFC PATCH v4 00/12] Second attempt at contained helper execution

[Ian Kent]

On Thu, 2015-03-19 at 16:38 -0500, Eric W. Biederman wrote:
> Ian Kent <raven@themaw.net> writes:
> 
> > Here is another update to the attempt at contained helper execution.
> >
> > The main change is I've tried to incorporate Oleg's suggestions
> > of directly constructing the namespaces rather than using the
> > open/setns approach and the addition of a namespace hash store.
> >
> > I'm not particularly happy with this so far as there are a bunch
> > of ref counted objects and I've almost certainly got that wrong.
> > But also there are object lifetime problems, some I'm aware of
> > and for sure others I'm not. Also there is the integrity of the
> > thread runner process. I haven't performed a double fork on thread
> > execution, it might be painful to implement, so the thread runner
> > might end up with the wrong namespace setup if an error occurs.
> >
> > Anyway, I've decided to stop spinning my wheels with this and
> > post an update in the hope that others can offer suggestions to
> > help and, of course, point out things I've missed.
> >
> > The other change has been to the nfs and KEYS patches.
> > I've introduced the ability to get a token that can be used to
> > save namespace information for later execution and I've attempted
> > to use that for persistent namespace execution, as was discussed
> > previously.
> >
> > I'm not at all sure I've done this in a sensible way but the
> > token does need to be accessible at helper execution time which
> > is why I've done it this way.
> >
> > I definitely need advice here too. 

Thanks for offering your advice once again Eric.

> 
> As far as I can tell this patchset continues to be broken for ignoring
> my earlier advice.

Ignoring is the wrong word.

I am ignorant of aspects of the bigger picture here but working on this
is helping with that quite a bit and your continued advice is very much
needed.

> 
> This patchset provides an escape from cgroup, lsm, rlimit, and
> seccomp policy.

OK.

> 
> This patchset does not appear particularly nice in how it uses
> namespaces.

Yeah, I definitely agree with that.

> 
> The only safe and sane way to do this is to have a kernel thread with
> all of the proper attributes configured waiting around ready to start
> the user mode helper.

I originally thought that wouldn't be viable due to the potential number
of threads that would be needed.

I still think that a thread per mountpoint, as we originally discussed,
won't work but looking at nfsd, nfs and the keys code for the recent
patch series makes me think that there might not be so many threads
needed and that depends on choosing a better place to start the threads.

> 
> The problem you are trying to solve is so hard that we totally failed to
> solve it outside of the container case.  Which is why we have kthreadd.
> I will be very surprised if you can figure out how to cleanly solve the
> problem the way you are attacking it.

So the previous approach of using file_open_root()/setns_inode() is
equally broken in the same respects as you mention above? You didn't
mention on that before?

I get that the problem is hard to solve but I'd rather not give up on
it.

Maybe the token I use could relate to a previously created kernel thread
instead of namespace information.

LOL, then you can describe what I've done wrong creating the kernel
threads as well, ;)

> 
> Eric
> 
> 

Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h

[Ian Kent]

On Thu, 2015-03-19 at 20:14 -0500, Eric W. Biederman wrote:
> Ian Kent <raven@themaw.net> writes:
> 
> 2> On Thu, 2015-03-19 at 19:47 +0000, Al Viro wrote:
> >> On Tue, Mar 17, 2015 at 10:45:09AM +0800, Ian Kent wrote:
> >> > From: Ian Kent <ikent@redhat.com>
> >> > 
> >> > The mnt_namespace definition will be needed by the usermode helper
> >> > contained execution implementation, move it to include/linux/mount.h.
> >> 
> >> I really don't like that.  AFAICS, the root of the evil is that fscking
> >> nsproxy keeps a pointer to mnt_namespace while all it really needs to
> >> know about is the address of ns_common field embedded into it.  Let's see...
> >
> > Thought that might be the case.
> >
> >> 
> >> fs/namespace.c:697:     struct mnt_namespace *ns = current->nsproxy->mnt_ns;
> >> fs/namespace.c:770:     return mnt->mnt_ns == current->nsproxy->mnt_ns;
> >> fs/namespace.c:1502:    return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
> >> fs/namespace.c:1587:    return current->nsproxy->mnt_ns->seq >= mnt_ns->seq;
> >> fs/namespace.c:2293:    struct user_namespace *user_ns = current->nsproxy->mnt_ns->user_ns;
> >> fs/namespace.c:2961:    touch_mnt_namespace(current->nsproxy->mnt_ns);
> >> fs/namespace.c:3003:    init_task.nsproxy->mnt_ns = ns;
> >> fs/namespace.c:3101:    ns_root.mnt = &current->nsproxy->mnt_ns->root->mnt;
> >> fs/namespace.c:3119:    struct mnt_namespace *ns = current->nsproxy->mnt_ns;
> >> fs/namespace.c:3159:            ns = &nsproxy->mnt_ns->ns;
> >> fs/namespace.c:3187:    put_mnt_ns(nsproxy->mnt_ns);
> >> fs/namespace.c:3188:    nsproxy->mnt_ns = mnt_ns;
> >> fs/pnode.c:282: user_ns = current->nsproxy->mnt_ns->user_ns;
> >> fs/proc_namespace.c:246:        if (!nsp || !nsp->mnt_ns) {
> >> fs/proc_namespace.c:251:        ns = nsp->mnt_ns;
> >> include/linux/nsproxy.h:33:     struct mnt_namespace *mnt_ns;
> >> kernel/nsproxy.c:37:    .mnt_ns                 = NULL,
> >> kernel/nsproxy.c:70:    new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
> >> kernel/nsproxy.c:71:    if (IS_ERR(new_nsp->mnt_ns)) {
> >> kernel/nsproxy.c:72:            err = PTR_ERR(new_nsp->mnt_ns);
> >> kernel/nsproxy.c:113:   if (new_nsp->mnt_ns)
> >> kernel/nsproxy.c:114:           put_mnt_ns(new_nsp->mnt_ns);
> >> kernel/nsproxy.c:160:   if (ns->mnt_ns)
> >> kernel/nsproxy.c:161:           put_mnt_ns(ns->mnt_ns);
> >> 
> >> OK, so we need
> >> 
> >> a) add in fs/mount.h
> >> static inline struct mnt_namespace *current_mnt_ns(void)
> >> {
> >> 	return current->nsproxy->mnt_ns;
> >> }
> >> 
> >> and replace a lot of open-coded instances.
> >> 
> >> b) lift to_mnt_ns() into fs/mount.h (as static inline)
> >> 
> >> c) switch put_mnt_ns() to struct ns_common *, replacing put_mnt_ns(ns)
> >> with put_mnt_ns(&ns->ns) and using to_mnt_ns() in the body to recover
> >> the original.
> >> 
> >> d) make copy_mnt_ns() take and return struct ns_common * (same treatment as for
> >> put_mnt_ns() in (c); replace
> >>         new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, new_fs);
> >>         if (IS_ERR(new_nsp->mnt_ns)) {
> >>                 err = PTR_ERR(new_nsp->mnt_ns);
> >>                 goto out_ns;
> >>         }
> >> with
> >> 	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
> >> 	if (IS_ERR(ns)) {
> >> 		err = PTR_ERR(ns);
> >> 		goto out_ns;
> >> 	}
> >> 	new_nsp->mnt_ns = to_mnt_ns(ns);
> >> 
> >> e) replace struct mnt_namespace *mnt_ns with struct ns_common *__mnt_ns in
> >> nsproxy.h, deal with users of mnt_ns by
> >> * everywhere in the tree replace put_mnt_ns(&...->mnt_ns->ns) with
> >> put_mnt_ns(...->__mnt_ns) (i.e. modify the lines modified by (c) again).
> >> * in fs/namespace.c replace
> >> 	init_task.nsproxy->mnt_ns = ns;
> >> with
> >> 	init_task.nsproxy->__mnt_ns = &ns->ns;
> >> replace
> >> 	ns = &nsproxy->mnt_ns->ns;
> >> with
> >> 	ns = nsproxy->__mnt_ns;
> >> replace
> >> 	nsproxy->mnt_ns = mnt_ns;
> >> with
> >> 	nsproxy>__mnt_ns = &mnt_ns->ns;
> >> * in fs/proc_namespace.c replace
> >> 	ns = nsp->mnt_ns;
> >> with
> >> 	ns = to_mnt_ns(nsp->__mnt_ns);
> >> and
> >> 	if (!nsp || !nsp->mnt_ns) {
> >> with
> >> 	if (!nsp || !nsp->__mnt_ns) {
> >> * in kernel/nsproxy.c: replace
> >> 	ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs);
> >> added in (d) with
> >> 	ns = copy_mnt_ns(flags, tsk->nsproxy->__mnt_ns, user_ns, new_fs);
> >> and
> >> 	new_nsp->mnt_ns = to_mnt_ns(ns);
> >> with
> >> 	new_nsp->__mnt_ns = ns;
> >> The rest of mnt_ns in that file get replaced by __mnt_ns.
> >> * in current_mnt_ns() replace ...->mnt_ns with to_mnt_ns(...->__mnt_ns)
> >> 
> >> Do you want me to push such a series in vfs.git?  After such 5 commits we have
> >> no pointers to struct mnt_namespace in nsproxy.h *and* no need for your patches
> >> to dig into ->mnt_ns->ns - we already have that as ->__mnt_ns.
> >
> > Yes please, I'd be more confident if you did this than me, there's
> > already enough to worry about with the series.
> 
> Given that this patchset is a security hole waiting to happen I don't
> see why Al should bother unless there are good reasons to do this
> otherwise.

Having now got to your mail now, I agree.

> 
> Eric

Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h

[Al Viro]

On Thu, Mar 19, 2015 at 08:14:05PM -0500, Eric W. Biederman wrote:

> > Yes please, I'd be more confident if you did this than me, there's
> > already enough to worry about with the series.
> 
> Given that this patchset is a security hole waiting to happen I don't
> see why Al should bother unless there are good reasons to do this
> otherwise.

There might be, actually.  &...->mnt_ns->ns is a lot saner candidate for
a reference in nsproxy than ...->mnt_ns - *that* is the part nsproxy-related
code cares about anyway, and unlike the rest of struct mnt_namespace it
doesn't have to be opaque for everything outside of (small part of) core
VFS.  Additionally, ->mnt_ns is a bad name choice - it sounds like a field
of struct mount and, worse yet, there *is* a field of struct mount with
that name.  Confusing for no good reason and makes both harder to grep for.
And current_mnt_ns() is definitely open-coded too many times - the first
commit in that series makes sense regardless of anything else.