Re: [PATCH 1/2] wait/ptrace: always assume __WALL if the child is traced

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 21 Oct 2015 19:41:50 +0200 Oleg Nesterov <oleg@redhat.com> wrote:

> On 10/20, Andrew Morton wrote:
> >
> > On Tue, 20 Oct 2015 19:17:54 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > The following program (simplified version of generated by syzkaller)
> > >
> > > 	#include <pthread.h>
> > > 	#include <unistd.h>
> > > 	#include <sys/ptrace.h>
> > > 	#include <stdio.h>
> > > 	#include <signal.h>
> > >
> > > 	void *thread_func(void *arg)
> > > 	{
> > > 		ptrace(PTRACE_TRACEME, 0,0,0);
> > > 		return 0;
> > > 	}
> > >
> > > 	int main(void)
> > > 	{
> > > 		pthread_t thread;
> > >
> > > 		if (fork())
> > > 			return 0;
> > >
> > > 		while (getppid() != 1)
> > > 			;
> > >
> > > 		pthread_create(&thread, NULL, thread_func, NULL);
> > > 		pthread_join(thread, NULL);
> > > 		return 0;
> > > 	}
> > >
> > > creates the unreapable zombie if /sbin/init doesn't use __WALL.
> > >
> > > This is not a kernel bug, at least in a sense that everything works as
> > > expected: debugger should reap a traced sub-thread before it can reap
> > > the leader, but without __WALL/__WCLONE do_wait() ignores sub-threads.
> > >
> > > Unfortunately, it seems that /sbin/init in most (all?) distributions
> > > doesn't use it and we have to change the kernel to avoid the problem.
> >
> > Well, to fix this a distro needs to roll out a new kernel.  Or a new
> > init(8).  Is there any reason to believe that distributing/deploying a
> > new kernel is significantly easier for everyone? Because fixing init
> > sounds like a much preferable solution to this problem.
> 
> I will be happy if we decide that this is userpace problem and we should
> not fix the kernel. I simply do not know.

The kernel patch sounds pretty sketchy - something we should avoid
doing if at all possible.

> However, please look at 2/2 which imho makes sense regardless and looks
> "obviously safe". Without this patch waitid() can not use __WALL, so if
> /sbin/init uses waitid() then the userspace fix won't be one-liner. And
> at least Fedora22 and Ubuntu use waitid().

2/2 does look sensible (needs a better changelog if it's to be a
standalone thing), but if we're expecting distros to fix this with an
updated init(8) only, then they can't assume that the kernel's waitid()
has been altered.  So init will need the not-one-liner version of the
fix.

> So personally I'd prefer 2/2 + fix-init, not sure if this can work...

I'm just guessing here.  Are you (or someone) able to find out which
approach the distros will prefer, and why?

And what has to be done to init(8) to fix this bug when running current
kernels?