[RFC PATCH 1/8] nsproxy - refactor setns()

[RFC PATCH 1/8] nsproxy - refactor setns()

[Ian Kent]

For usermode helpers to execute within a namspace a slightly different
entry point to setns() that takes a namspace inode is needed.

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/nsproxy.h |    1 +
 kernel/nsproxy.c        |   21 ++++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index 35fa08f..c75bf12 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -62,6 +62,7 @@ extern struct nsproxy init_nsproxy;
  *
  */
 
+int setns_inode(struct inode *inode, int nstype);
 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..27cc544 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -218,20 +218,15 @@ void exit_task_namespaces(struct task_struct *p)
 	switch_task_namespaces(p, NULL);
 }
 
-SYSCALL_DEFINE2(setns, int, fd, int, nstype)
+int setns_inode(struct inode *inode, int nstype)
 {
 	struct task_struct *tsk = current;
 	struct nsproxy *new_nsproxy;
-	struct file *file;
 	struct ns_common *ns;
 	int err;
 
-	file = proc_ns_fget(fd);
-	if (IS_ERR(file))
-		return PTR_ERR(file);
-
 	err = -EINVAL;
-	ns = get_proc_ns(file_inode(file));
+	ns = get_proc_ns(inode);
 	if (nstype && (ns->ops->type != nstype))
 		goto out;
 
@@ -248,6 +243,18 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype)
 	}
 	switch_task_namespaces(tsk, new_nsproxy);
 out:
+	return err;
+}
+
+SYSCALL_DEFINE2(setns, int, fd, int, nstype)
+{
+	struct file *file;
+	int err;
+
+	file = proc_ns_fget(fd);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+	err = setns_inode(file_inode(file), nstype);
 	fput(file);
 	return err;
 }

[RFC PATCH 2/8] kmod - rename call_usermodehelper() flags parameter

[Ian Kent]

The wait parameter of call_usermodehelper() is not quite a parameter
that describes the wait behaviour alone and will later be used to
request exec within a namespace.

So change its 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 |    4 ++--
 kernel/kmod.c        |   16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 0555cc6..15bdeed 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -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, 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, int flags);
 
 extern struct ctl_table usermodehelper_table[];
 
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 2777f40..14c0188 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -534,7 +534,7 @@ EXPORT_SYMBOL(call_usermodehelper_setup);
  * 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, int flags)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
 	int retval = 0;
@@ -553,14 +553,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->wait = 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;
@@ -595,17 +595,17 @@ 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, 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);
 

Re: [RFC PATCH 1/8] nsproxy - refactor setns()

[Ian Kent]

Oops!
Please ignore these, mistakenly sent.

On Tue, 2015-02-03 at 15:16 +0800, Ian Kent wrote:
> For usermode helpers to execute within a namspace a slightly different
> entry point to setns() that takes a namspace inode is needed.
> 
> 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/nsproxy.h |    1 +
>  kernel/nsproxy.c        |   21 ++++++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
> index 35fa08f..c75bf12 100644
> --- a/include/linux/nsproxy.h
> +++ b/include/linux/nsproxy.h
> @@ -62,6 +62,7 @@ extern struct nsproxy init_nsproxy;
>   *
>   */
>  
> +int setns_inode(struct inode *inode, int nstype);
>  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..27cc544 100644
> --- a/kernel/nsproxy.c
> +++ b/kernel/nsproxy.c
> @@ -218,20 +218,15 @@ void exit_task_namespaces(struct task_struct *p)
>  	switch_task_namespaces(p, NULL);
>  }
>  
> -SYSCALL_DEFINE2(setns, int, fd, int, nstype)
> +int setns_inode(struct inode *inode, int nstype)
>  {
>  	struct task_struct *tsk = current;
>  	struct nsproxy *new_nsproxy;
> -	struct file *file;
>  	struct ns_common *ns;
>  	int err;
>  
> -	file = proc_ns_fget(fd);
> -	if (IS_ERR(file))
> -		return PTR_ERR(file);
> -
>  	err = -EINVAL;
> -	ns = get_proc_ns(file_inode(file));
> +	ns = get_proc_ns(inode);
>  	if (nstype && (ns->ops->type != nstype))
>  		goto out;
>  
> @@ -248,6 +243,18 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype)
>  	}
>  	switch_task_namespaces(tsk, new_nsproxy);
>  out:
> +	return err;
> +}
> +
> +SYSCALL_DEFINE2(setns, int, fd, int, nstype)
> +{
> +	struct file *file;
> +	int err;
> +
> +	file = proc_ns_fget(fd);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +	err = setns_inode(file_inode(file), nstype);
>  	fput(file);
>  	return err;
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 0/8] v3 contained usermode helper execution

[Ian Kent]

There haven't been any comments about the previous series not being
an acceptable approach. Perhaps people were away, didn't notice or
didn't have time.

So here's another chance to speak up.

In summary it's assumed that, since the usermode helper uses the
root init namespace for process creation, using the init namespace
of a container is eqivalent and sufficient when execution within a
container is needed.

Thinking further about callers I believe there are cases that won't
be handled properly so I've tried to work out what the current use
cases are and added patches that demonstrate simple minded usage.
I'm not sure at all that this is sufficient so I need feedback.

I've changed the execution to pin the calling task for the duration
of the call as recommended by Jeff Layton but other than that not a
lot has changed in the call back code.

It's also not clear if the request key infrastructure will continue
to use a usermode callback so we'll need to wait on that.

---

Ian Kent (8):
      nsproxy - refactor setns()
      kmod - rename call_usermodehelper() flags parameter
      kmod - teach call_usermodehelper() to use a namespace
      KEYS - rename call_usermodehelper_keys() flags parameter
      KEYS: exec request-key within the requesting task's init namespace
      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


 fs/nfs/cache_lib.c           |    6 ++
 fs/nfs/objlayout/objlayout.c |    7 ++
 fs/nfsd/netns.h              |    2 +
 fs/nfsd/nfs4recover.c        |   48 ++++++++++-----
 include/linux/kmod.h         |   20 ++++++
 include/linux/nsproxy.h      |    1 
 kernel/kmod.c                |  131 ++++++++++++++++++++++++++++++++++++++----
 kernel/nsproxy.c             |   21 ++++---
 security/keys/request_key.c  |   64 +++++++++++++++++----
 9 files changed, 252 insertions(+), 48 deletions(-)

--
Ian

[RFC PATCH 2/8] kmod - rename call_usermodehelper() flags parameter

[Ian Kent]

The wait parameter of call_usermodehelper() is not quite a parameter
that describes the wait behaviour alone and will later be used to
request exec within a namespace.

So change its 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 |    4 ++--
 kernel/kmod.c        |   16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 0555cc6..15bdeed 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -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, 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, int flags);
 
 extern struct ctl_table usermodehelper_table[];
 
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 2777f40..14c0188 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -534,7 +534,7 @@ EXPORT_SYMBOL(call_usermodehelper_setup);
  * 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, int flags)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
 	int retval = 0;
@@ -553,14 +553,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->wait = 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;
@@ -595,17 +595,17 @@ 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, 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);
 

Re: [RFC PATCH 2/8] kmod - rename call_usermodehelper() flags parameter

[David Howells]

Ian Kent <ikent@redhat.com> wrote:

> -call_usermodehelper(char *path, char **argv, char **envp, int wait);
> +call_usermodehelper(char *path, char **argv, char **envp, int flags);

Can we make flags unsigned whilst we're at it?  Other than that:

Acked-by: David Howells <dhowells@redhat.com>

Re: [RFC PATCH 2/8] kmod - rename call_usermodehelper() flags parameter

[Ian Kent]

On Thu, 2015-02-05 at 15:01 +0000, David Howells wrote:
> Ian Kent <ikent@redhat.com> wrote:
> 
> > -call_usermodehelper(char *path, char **argv, char **envp, int wait);
> > +call_usermodehelper(char *path, char **argv, char **envp, int flags);
> 
> Can we make flags unsigned whilst we're at it?  Other than that:

Sure, thanks for your comments, here and elsewhere, I'll get onto fixing
them.

> 
> Acked-by: David Howells <dhowells@redhat.com>