Re: [PATCH 2/3] make kernel threads invisible to /sbin/init

2007-04-11 Thread Oleg Nesterov
On 04/10, Eric W. Biederman wrote:
>
> Oleg Nesterov <[EMAIL PROTECTED]> writes:
> 
> > 1. rename reparent_to_init() to reparent_kthread() and export it
> >
> > 2. use init_pid_ns.child_reaper instead of child_reaper(current)
> >
> > 3. set ->exit_signal = -1, so init can't see us and we don't use
> >it to reap the task.
> >
> > 4. add reparent_kthread() to kthread() and stopmachine()
> >
> 
> If the goal is to hide from /sbin/init.  We don't need to touch
> kernel/kthread.c or 
> kernel/stop_machine.c
> 
> Their parents are already kernel threads.
> 
> For the kernel thread they all inherit signals with SIGCHLD set to
> SIG_IGN, so there is child auto reaping in that form.  Adding
> the ->exit_signal = -1 would be a bonus but is not required.

Unless a kernel thread does kernel_thread() (not kthread_create) and
exits. In that case the child will be re-parented to init which doesn't
ignore SIGCHLD.

Robin Holt wrote:
>
> wait_task_zombie() is taking many seconds to get through the list.
> For the case of a modprobe, stop_machine creates one thread per cpu
> (remember big number). All are parented to init and their exit will
> cause wait_task_zombie to scan multiple times most of the way through
> this very long list looking for threads which need to be reaped.

initially, "stopmachine" threads were not parented to init.

However, I agree, your patch is better, and solves most problems in more
simple way. Including the above problem, I believe. "stopmachine" likely
does exit_notify() and notices SIG_IGN (inherited from kthreadd_setup())
before "do_stop" does forget_original_parent().

Oleg.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] make kernel threads invisible to /sbin/init

2007-04-10 Thread Eric W. Biederman
Oleg Nesterov <[EMAIL PROTECTED]> writes:

> 1. rename reparent_to_init() to reparent_kthread() and export it
>
> 2. use init_pid_ns.child_reaper instead of child_reaper(current)
>
> 3. set ->exit_signal = -1, so init can't see us and we don't use
>it to reap the task.
>
> 4. add reparent_kthread() to kthread() and stopmachine()
>

If the goal is to hide from /sbin/init.  We don't need to touch
kernel/kthread.c or 
kernel/stop_machine.c

Their parents are already kernel threads.

Yes. There is a startup issue for kernel/kthread.c I just about have
a patch I can stand that addresses that issue.

For the kernel thread they all inherit signals with SIGCHLD set to
SIG_IGN, so there is child auto reaping in that form.  Adding
the ->exit_signal = -1 would be a bonus but is not required.

I am a little concerned with removing the knowledge of who our
real parent is as that may compromise debugging.  The reason
why kernel threads are visible in the first place.

I do support reparenting kernel threads that call daemonize,
and the modifications to reparent_kthread here look reasonable.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] make kernel threads invisible to /sbin/init

2007-04-10 Thread Eric W. Biederman
"Serge E. Hallyn" <[EMAIL PROTECTED]> writes:

> Quoting Oleg Nesterov ([EMAIL PROTECTED]):
>> 1. rename reparent_to_init() to reparent_kthread() and export it
>> 
>> 2. use init_pid_ns.child_reaper instead of child_reaper(current)
>
> Each of these patches looks good to me, but this part in particular
> is a must-have bugfix.

Removing daemonize is a must have bug fix.  This falls short of that so
it is a good fix, but it doesn't solve the core problem that kernel daemons
are being assigned pids inside of child pid namespaces.

It doesn't solve the problem that some kernel daemons are using signals
to communicate with user space.

It doesn't solve the problem that we have to do a lot of massaging and
maintenance to get kernel threads from grabbing references to namespaces
and other kernel pieces they should not be grabbing.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] make kernel threads invisible to /sbin/init

2007-04-10 Thread Serge E. Hallyn
Quoting Oleg Nesterov ([EMAIL PROTECTED]):
> 1. rename reparent_to_init() to reparent_kthread() and export it
> 
> 2. use init_pid_ns.child_reaper instead of child_reaper(current)

Each of these patches looks good to me, but this part in particular
is a must-have bugfix.

Just started some tests, if any failures come back I'll report them
tonight.

thanks Oleg,
-serge

> 3. set ->exit_signal = -1, so init can't see us and we don't use
>it to reap the task.
> 
> 4. add reparent_kthread() to kthread() and stopmachine()
> 
> See also
> 
>   http://marc.info/?t=11758028223&r=1
>   http://marc.info/?t=9529928483&r=1
> 
> Signed-off-by: Oleg Nesterov <[EMAIL PROTECTED]>
> 
>  include/linux/sched.h |1 +
>  kernel/exit.c |   16 
>  kernel/kthread.c  |1 +
>  kernel/stop_machine.c |1 +
>  4 files changed, 11 insertions(+), 8 deletions(-)
> 
> --- 2.6.21-rc5/include/linux/sched.h~2_DETACH 2007-04-05 12:18:28.0 
> +0400
> +++ 2.6.21-rc5/include/linux/sched.h  2007-04-10 21:52:27.0 +0400
> @@ -1401,6 +1401,7 @@ extern void exit_itimers(struct signal_s
>  
>  extern NORET_TYPE void do_group_exit(int);
>  
> +extern void reparent_kthread(void);
>  extern void daemonize(const char *, ...);
>  extern int allow_signal(int);
>  extern int disallow_signal(int);
> --- 2.6.21-rc5/kernel/exit.c~2_DETACH 2007-04-10 21:32:44.0 +0400
> +++ 2.6.21-rc5/kernel/exit.c  2007-04-10 21:59:41.0 +0400
> @@ -255,7 +255,7 @@ static int has_stopped_jobs(struct pid *
>  }
>  
>  /**
> - * reparent_to_init - Reparent the calling kernel thread to the init task of 
> the pid space that the thread belongs to.
> + * reparent_kthread - Reparent the calling kernel thread to the init task of 
> the pid space that the thread belongs to.
>   *
>   * If a kernel thread is launched as a result of a system call, or if
>   * it ever exits, it should generally reparent itself to init so that
> @@ -264,20 +264,20 @@ static int has_stopped_jobs(struct pid *
>   * The various task state such as scheduling policy and priority may have
>   * been inherited from a user process, so we reset them to sane values here.
>   *
> - * NOTE that reparent_to_init() gives the caller full capabilities.
> + * NOTE that reparent_kthread() gives the caller full capabilities.
>   */
> -static void reparent_to_init(void)
> +void reparent_kthread(void)
>  {
>   write_lock_irq(&tasklist_lock);
>  
>   ptrace_unlink(current);
>   remove_parent(current);
> - current->parent = child_reaper(current);
> - current->real_parent = child_reaper(current);
> + current->parent = init_pid_ns.child_reaper;
> + current->real_parent = current->parent;
>   add_parent(current);
>  
> - /* Set the exit signal to SIGCHLD so we signal init on exit */
> - current->exit_signal = SIGCHLD;
> + /* make the task auto-reap */
> + current->exit_signal = -1;
>  
>   security_task_reparent_to_init(current);
>   write_unlock_irq(&tasklist_lock);
> @@ -391,7 +391,7 @@ void daemonize(const char *name, ...)
>   current->files = init_task.files;
>   atomic_inc(¤t->files->count);
>  
> - reparent_to_init();
> + reparent_kthread();
>  
>   if (!has_rt_policy(current) && (task_nice(current) < 0))
>   set_user_nice(current, 0);
> --- 2.6.21-rc5/kernel/kthread.c~2_DETACH  2007-04-05 12:18:28.0 
> +0400
> +++ 2.6.21-rc5/kernel/kthread.c   2007-04-10 22:02:31.0 +0400
> @@ -82,6 +82,7 @@ static int kthread(void *_create)
>   sigset_t blocked;
>   int ret = -EINTR;
>  
> + reparent_kthread();
>   kthread_exit_files();
>  
>   /* Copy data: it's on keventd's stack */
> --- 2.6.21-rc5/kernel/stop_machine.c~2_DETACH 2006-10-22 18:24:03.0 
> +0400
> +++ 2.6.21-rc5/kernel/stop_machine.c  2007-04-10 22:04:23.0 +0400
> @@ -33,6 +33,7 @@ static int stopmachine(void *cpu)
>   int irqs_disabled = 0;
>   int prepared = 0;
>  
> + reparent_kthread();
>   set_cpus_allowed(current, cpumask_of_cpu((int)(long)cpu));
>  
>   /* Ack: we are alive */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/3] make kernel threads invisible to /sbin/init

2007-04-10 Thread Oleg Nesterov
1. rename reparent_to_init() to reparent_kthread() and export it

2. use init_pid_ns.child_reaper instead of child_reaper(current)

3. set ->exit_signal = -1, so init can't see us and we don't use
   it to reap the task.

4. add reparent_kthread() to kthread() and stopmachine()

See also

http://marc.info/?t=11758028223&r=1
http://marc.info/?t=9529928483&r=1

Signed-off-by: Oleg Nesterov <[EMAIL PROTECTED]>

 include/linux/sched.h |1 +
 kernel/exit.c |   16 
 kernel/kthread.c  |1 +
 kernel/stop_machine.c |1 +
 4 files changed, 11 insertions(+), 8 deletions(-)

--- 2.6.21-rc5/include/linux/sched.h~2_DETACH   2007-04-05 12:18:28.0 
+0400
+++ 2.6.21-rc5/include/linux/sched.h2007-04-10 21:52:27.0 +0400
@@ -1401,6 +1401,7 @@ extern void exit_itimers(struct signal_s
 
 extern NORET_TYPE void do_group_exit(int);
 
+extern void reparent_kthread(void);
 extern void daemonize(const char *, ...);
 extern int allow_signal(int);
 extern int disallow_signal(int);
--- 2.6.21-rc5/kernel/exit.c~2_DETACH   2007-04-10 21:32:44.0 +0400
+++ 2.6.21-rc5/kernel/exit.c2007-04-10 21:59:41.0 +0400
@@ -255,7 +255,7 @@ static int has_stopped_jobs(struct pid *
 }
 
 /**
- * reparent_to_init - Reparent the calling kernel thread to the init task of 
the pid space that the thread belongs to.
+ * reparent_kthread - Reparent the calling kernel thread to the init task of 
the pid space that the thread belongs to.
  *
  * If a kernel thread is launched as a result of a system call, or if
  * it ever exits, it should generally reparent itself to init so that
@@ -264,20 +264,20 @@ static int has_stopped_jobs(struct pid *
  * The various task state such as scheduling policy and priority may have
  * been inherited from a user process, so we reset them to sane values here.
  *
- * NOTE that reparent_to_init() gives the caller full capabilities.
+ * NOTE that reparent_kthread() gives the caller full capabilities.
  */
-static void reparent_to_init(void)
+void reparent_kthread(void)
 {
write_lock_irq(&tasklist_lock);
 
ptrace_unlink(current);
remove_parent(current);
-   current->parent = child_reaper(current);
-   current->real_parent = child_reaper(current);
+   current->parent = init_pid_ns.child_reaper;
+   current->real_parent = current->parent;
add_parent(current);
 
-   /* Set the exit signal to SIGCHLD so we signal init on exit */
-   current->exit_signal = SIGCHLD;
+   /* make the task auto-reap */
+   current->exit_signal = -1;
 
security_task_reparent_to_init(current);
write_unlock_irq(&tasklist_lock);
@@ -391,7 +391,7 @@ void daemonize(const char *name, ...)
current->files = init_task.files;
atomic_inc(¤t->files->count);
 
-   reparent_to_init();
+   reparent_kthread();
 
if (!has_rt_policy(current) && (task_nice(current) < 0))
set_user_nice(current, 0);
--- 2.6.21-rc5/kernel/kthread.c~2_DETACH2007-04-05 12:18:28.0 
+0400
+++ 2.6.21-rc5/kernel/kthread.c 2007-04-10 22:02:31.0 +0400
@@ -82,6 +82,7 @@ static int kthread(void *_create)
sigset_t blocked;
int ret = -EINTR;
 
+   reparent_kthread();
kthread_exit_files();
 
/* Copy data: it's on keventd's stack */
--- 2.6.21-rc5/kernel/stop_machine.c~2_DETACH   2006-10-22 18:24:03.0 
+0400
+++ 2.6.21-rc5/kernel/stop_machine.c2007-04-10 22:04:23.0 +0400
@@ -33,6 +33,7 @@ static int stopmachine(void *cpu)
int irqs_disabled = 0;
int prepared = 0;
 
+   reparent_kthread();
set_cpus_allowed(current, cpumask_of_cpu((int)(long)cpu));
 
/* Ack: we are alive */

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/