Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1363990 > unrolled thread

Re: call_usermodehelper in containers

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: call_usermodehelper in containers Ian Kent <raven@themaw.net> - 2016-03-24 08:50 +0100
    Re: call_usermodehelper in containers Oleg Nesterov <oleg@redhat.com> - 2016-03-25 02:40 +0100
      Re: call_usermodehelper in containers Ian Kent <raven@themaw.net> - 2016-03-25 08:30 +0100

#1363990 — Re: call_usermodehelper in containers

On Mon, 2013-11-18 at 18:28 +0100, Oleg Nesterov wrote:
> On 11/15, Eric W. Biederman wrote:
> > 
> > I don't understand that one.  Having a preforked thread with the
> > proper
> > environment that can act like kthreadd in terms of spawning user
> > mode
> > helpers works and is simple.
> 
> Can't we ask ->child_reaper to create the non-daemonized kernel thread
> with the "right" ->nsproxy, ->fs, etc?
> 
> IOW. Please the the "patch" below. It is obviously incomplete and
> wrong,
> and it can be more clear/clean. And probably we need another API. Just
> to explain what I mean.
> 
> With this patch call_usermodehelper(..., UMH_IN_MY_NS) should do exec
> from the caller's namespace.

Umm ... I don't think this can work.

I don't think it can be assumed that the init process of a container
will behave like an init process.

If you try and do this with a Docker container that has /bin/bash as the
init process signals never arrive and work doesn't start until some
other signal arrives at which time it fails to create the kernel thread
returning an error ERESTARTNOINTER (IIRC).

In fact a number of other things relating to signalling processes to
cleanly shutdown in a container suffer the same problem.

I probably don't understand what's actually going on, this is just my
impression of what I'm seeing.

> 
> Oleg.
> ---
> 
> --- a/include/linux/kmod.h
> +++ b/include/linux/kmod.h
> @@ -24,6 +24,7 @@
>  #include <linux/errno.h>
>  #include <linux/compiler.h>
>  #include <linux/workqueue.h>
> +#include <linux/task_work.h>
>  #include <linux/sysctl.h>
>  
>  #define KMOD_PATH_LEN 256
> @@ -53,8 +54,14 @@ struct file;
>  #define UMH_WAIT_PROC	2	/* wait for the process to
> complete */
>  #define UMH_KILLABLE	4	/* wait for EXEC/PROC killable
> */
>  
> +// FIXME: IMH_* is not actually a mask
> +#define UMH_IN_MY_NS	8
> +
>  struct subprocess_info {
> -	struct work_struct work;
> +	union {
> +		struct work_struct work;
> +		struct callback_head twork;
> +	};
>  	struct completion *complete;
>  	char *path;
>  	char **argv;
> --- a/kernel/kmod.c
> +++ b/kernel/kmod.c
> @@ -541,7 +541,6 @@ struct subprocess_info
> *call_usermodehelper_setup(char *path, char **argv,
>  	if (!sub_info)
>  		goto out;
>  
> -	INIT_WORK(&sub_info->work, __call_usermodehelper);
>  	sub_info->path = path;
>  	sub_info->argv = argv;
>  	sub_info->envp = envp;
> @@ -554,6 +553,24 @@ struct subprocess_info
> *call_usermodehelper_setup(char *path, char **argv,
>  }
>  EXPORT_SYMBOL(call_usermodehelper_setup);
>  
> +static int call_call_usermodehelper(void *twork)
> +{
> +	struct subprocess_info *sub_info =
> +		container_of(twork, struct subprocess_info, twork);
> +
> +	__call_usermodehelper(&sub_info->work);
> +	do_exit(0);
> +
> +}
> +
> +static void fork_umh_helper(struct callback_head *twork)
> +{
> +	if (current->flags & PF_EXITING)
> +		return;	// WRONG, FIXME
> +
> +	kernel_thread(call_call_usermodehelper, twork, SIGCHLD);
> +}
> +
>  /**
>   * call_usermodehelper_exec - start a usermode application
>   * @sub_info: information about the subprocessa
> @@ -570,6 +587,10 @@ int call_usermodehelper_exec(struct
> subprocess_info *sub_info, int wait)
>  {
>  	DECLARE_COMPLETION_ONSTACK(done);
>  	int retval = 0;
> +	bool in_my_ns;
> +
> +	in_my_ns = wait & UMH_IN_MY_NS;
> +	wait &= ~UMH_IN_MY_NS;
>  
>  	if (!sub_info->path) {
>  		call_usermodehelper_freeinfo(sub_info);
> @@ -594,7 +615,21 @@ int call_usermodehelper_exec(struct
> subprocess_info *sub_info, int wait)
>  	sub_info->complete = &done;
>  	sub_info->wait = wait;
>  
> -	queue_work(khelper_wq, &sub_info->work);
> +	if (likely(!in_my_ns)) {
> +		INIT_WORK(&sub_info->work, __call_usermodehelper);
> +		queue_work(khelper_wq, &sub_info->work);
> +	} else {
> +		// RACY, WRONG, ETC
> +		struct task_struct *my_init =
> task_active_pid_ns(current)->child_reaper;
> +
> +		init_task_work(&sub_info->twork, fork_umh_helper);
> +		task_work_add(my_init, &sub_info->twork, false);
> +
> +		// until we have task_work_add_interruptibel()
> +		do_send_sig_info(SIGCHLD, SEND_SIG_FORCED, my_init,
> false);
> +
> +	}
> +
>  	if (wait == UMH_NO_WAIT)	/* task has freed sub_info */
>  		goto unlock;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux
> -fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [next] | [standalone]

#1364514

Hi Ian,

I can't really recall this old discussion, so I can be easily wrong...

On 03/24, Ian Kent wrote:
>
> On Mon, 2013-11-18 at 18:28 +0100, Oleg Nesterov wrote:
> >
> > IOW. Please the the "patch" below. It is obviously incomplete and
> > wrong,
> > and it can be more clear/clean. And probably we need another API. Just
> > to explain what I mean.

I hope you didn't miss this part ;)

In particular, we want to turn task_work_add(..., bool notify) into
task_work_add(..., how_to_notify mask) and this "mask" should allow
to force TIF_SIGPENDING.

> > With this patch call_usermodehelper(..., UMH_IN_MY_NS) should do exec
> > from the caller's namespace.
>
> Umm ... I don't think this can work.
>
> I don't think it can be assumed that the init process of a container
> will behave like an init process.
>
> If you try and do this with a Docker container that has /bin/bash as the
> init process signals never arrive and work doesn't start until some
> other signal arrives

only if it blocks/ignores SIGCHLD? But this doesn't matter, see above and
note the "until we have task_work_add_interruptibel()" in the pseudo-code
I showed.

> I probably don't understand what's actually going on, this is just my
> impression of what I'm seeing.

Or perhaps it is me who misunderstands your concerns.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1364581

On Fri, 2016-03-25 at 02:28 +0100, Oleg Nesterov wrote:
> Hi Ian,
> 
> I can't really recall this old discussion, so I can be easily wrong...
> 
> On 03/24, Ian Kent wrote:
> > 
> > On Mon, 2013-11-18 at 18:28 +0100, Oleg Nesterov wrote:
> > > 
> > > IOW. Please the the "patch" below. It is obviously incomplete and
> > > wrong,
> > > and it can be more clear/clean. And probably we need another API.
> > > Just
> > > to explain what I mean.
> 
> I hope you didn't miss this part ;)

Not at all.

> 
> In particular, we want to turn task_work_add(..., bool notify) into
> task_work_add(..., how_to_notify mask) and this "mask" should allow
> to force TIF_SIGPENDING.

The point of posting the reply was to try and get some advice as my
understanding of the signalling subsystem is fairly poor.

LOL, I'll have another look at the task_work_add() code and see if I can
understand what your trying to tell me.

> 
> > > With this patch call_usermodehelper(..., UMH_IN_MY_NS) should do
> > > exec
> > > from the caller's namespace.
> > 
> > Umm ... I don't think this can work.
> > 
> > I don't think it can be assumed that the init process of a container
> > will behave like an init process.
> > 
> > If you try and do this with a Docker container that has /bin/bash as
> > the
> > init process signals never arrive and work doesn't start until some
> > other signal arrives
> 
> only if it blocks/ignores SIGCHLD? But this doesn't matter, see above
> and
> note the "until we have task_work_add_interruptibel()" in the pseudo
> -code
> I showed.

It seems, and this is not the only case I've encountered, that the init
process in docker containers can be a problem when you want to capture
and handle signals.

I've seen this with /bin/bash and supervisord so far.
I don't know if it is the docker container creation doing this or
something else .... certainly I can catch signals within subordinate
processes.

The other thing that occurs to me is that just about anything in a
container could be subverted so the definition of a privileged process
which can be used as a template form execution is essentially undefined.

Mmm ... maybe I've got that wrong too, ;)

Ian

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web