Re: + daemonize-detach-from-current-namespace.patch added to -mm tree

Re: + daemonize-detach-from-current-namespace.patch added to -mm tree

[Arjan van de Ven]

> diff -puN kernel/exit.c~daemonize-detach-from-current-namespace kernel/exit.c
> --- devel/kernel/exit.c~daemonize-detach-from-current-namespace	2006-02-19 20:36:58.000000000 -0800
> +++ devel-akpm/kernel/exit.c	2006-02-19 20:36:58.000000000 -0800
> @@ -360,6 +360,9 @@ void daemonize(const char *name, ...)
>  	fs = init_task.fs;
>  	current->fs = fs;
>  	atomic_inc(&fs->count);
> +	exit_namespace(current);
> +	current->namespace = init_task.namespace;
> +	get_namespace(current->namespace);
>   	exit_files(current);

not that it'll matter much here, but this is normally the wrong order of
refcounting ;) First take the count, then start using it ;)

Re: + daemonize-detach-from-current-namespace.patch added to -mm tree

[Eric W. Biederman]

Arjan van de Ven <arjan@infradead.org> writes:

>> diff -puN kernel/exit.c~daemonize-detach-from-current-namespace kernel/exit.c
>> --- devel/kernel/exit.c~daemonize-detach-from-current-namespace 2006-02-19
> 20:36:58.000000000 -0800
>> +++ devel-akpm/kernel/exit.c	2006-02-19 20:36:58.000000000 -0800
>> @@ -360,6 +360,9 @@ void daemonize(const char *name, ...)
>>  	fs = init_task.fs;
>>  	current->fs = fs;
>>  	atomic_inc(&fs->count);
>> +	exit_namespace(current);
>> +	current->namespace = init_task.namespace;
>> +	get_namespace(current->namespace);
>>   	exit_files(current);
>
> not that it'll matter much here, but this is normally the wrong order of
> refcounting ;) First take the count, then start using it ;)

Well what would need in a general case situation here would be task_lock,
for the two tasks.  Having that as long as all of the operations happened
under the lock it wouldn't matter.  In this case the init_task is known
not to change, and no one else should be messing with current.

I am beginning to suspect that we will want to fix kernel_thread so it
creates copies of the init_task rather than copies of whatever random
user space process we happen to be a member of at the time.  With an
enhanced kernel_thread this problem could more easily avoided, as
we add additional namespaces to the kernel.

The problem with a kernel thread running in a private namespace like this
is that you can't kill the kernel thread and when it is the last user
of a private namespace.  So you can never free up your mounts etc.

As for this patch in particular matching the style that is already there for
a bug fix seems to make a lot of sense :)

Eric

Re: + daemonize-detach-from-current-namespace.patch added to -mm tree

[Andrew Morton]

ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> I am beginning to suspect that we will want to fix kernel_thread so it
>  creates copies of the init_task rather than copies of whatever random
>  user space process we happen to be a member of at the time.  With an
>  enhanced kernel_thread this problem could more easily avoided, as
>  we add additional namespaces to the kernel.

You wouldn't believe the problems we had with kernel_thread followed by
call_usermodehelper() due to inheritance of random stuff from the userspace
parent.

A suitable solution is to stop using kernel_thread(), migrate to the
kthread API - that way the threads are parented by keventd which is a known
and good environment.

Re: + daemonize-detach-from-current-namespace.patch added to -mm tree

[Eric W. Biederman]

Andrew Morton <akpm@osdl.org> writes:

> ebiederm@xmission.com (Eric W. Biederman) wrote:
>>
>> I am beginning to suspect that we will want to fix kernel_thread so it
>>  creates copies of the init_task rather than copies of whatever random
>>  user space process we happen to be a member of at the time.  With an
>>  enhanced kernel_thread this problem could more easily avoided, as
>>  we add additional namespaces to the kernel.
>
> You wouldn't believe the problems we had with kernel_thread followed by
> call_usermodehelper() due to inheritance of random stuff from the userspace
> parent.
>
> A suitable solution is to stop using kernel_thread(), migrate to the
> kthread API - that way the threads are parented by keventd which is a known
> and good environment.

Thanks.  This sounds worth investigating.

On the other hand call_usermodehelper is the one case where we might
want to share some of the attributes with an existing process, at
least if we ever want it to run in a non-default namespace.

Eric

Re: + daemonize-detach-from-current-namespace.patch added to -mm tree

[Eric W. Biederman]

ebiederm@xmission.com (Eric W. Biederman) writes:

> Andrew Morton <akpm@osdl.org> writes:
>
>
> Thanks.  This sounds worth investigating.

Ok I after looking I agree that the kthread API is the way to go long term.
If no one beats me to it I will look at this after I get some of pending
patches merged.

I am still concerned that we might want to launch user space processes
in non-default namespace but it is almost certainly not guaranteed to
be the namespace from where we see the event happen, so inheriting the
namespace in the wrong thing in all cases I can think of.

Eric