• Oleg Nesterov's avatar
    wait_task_zombie: fix 2/3 races vs forget_original_parent() · 2f4e6e2a
    Oleg Nesterov authored
    Two threads, T1 and T2.  T2 ptraces P, and P is not a child of ptracer's
    thread group.  P exits and goes to TASK_ZOMBIE.
    
    T1 does wait_task_zombie(P):
    
    	P->exit_state = TASK_DEAD;
    	...
    	read_unlock(&tasklist_lock);
    
    					T2 does exit(), takes tasklist,
    					forget_original_parent() does
    					__ptrace_unlink(P) but doesn't
    					call do_notify_parent(P) because
    					p->exit_state == EXIT_DEAD.
    
    Now, P is not visible to our process: __ptrace_unlink() removed it from
    ->children. We should send notification to P->parent and release P if and
    only if SIGCHLD is ignored.
    
    And we have 3 bugs:
    
    1. P->parent does do_wait() and gets -ECHILD (P is on ->parent->children,
       but its state is TASK_DEAD).
    
    2. // wait_task_zombie() continues
    
    	if (put_user(...)) {
    		// TODO: is this safe?
    		p->exit_state = EXIT_ZOMBIE;
    		return;
    	}
    
       we return without notification/release, task_struct leaked.
    
       Solution: ignore -EFAULT and proceed. It is an application's bug if
       we can't fill infop/stat_addr (in case of VM_FAULT_OOM we have much
       more problems).
    
    3. // wait_task_zombie() continues
    
    	if (p->real_parent != p->parent) {
    		// Not taken, it was untraced'ed
    		...
    	}
    
    	release_task(p);
    
       we released the task which we shouldn't.
    
       Solution: check ->real_parent != ->parent before, under tasklist_lock,
       but use ptrace_unlink() instead of __ptrace_unlink() to check ->ptrace.
    
    This patch hopefully solves 2 and 3, the 1st bug will be fixed later, we need
    some cleanups in forget_original_parent/reparent_thread.
    
    However, the first race is very unlikely and not critical, so I hope it makes
    sense to fix 1 and 2 for now.
    
    4. Small cleanup: don't "restore" EXIT_ZOMBIE unless we know we are not going
       to realease the child.
    Signed-off-by: default avatarOleg Nesterov <oleg@tv-sign.ru>
    Cc: Ingo Molnar <mingo@elte.hu>
    Cc: Roland McGrath <roland@redhat.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    2f4e6e2a
exit.c 43.6 KB