• Oleg Nesterov's avatar
    clone(): fix race between copy_process() and de_thread() · 4ab6c083
    Oleg Nesterov authored
    Spotted by Hiroshi Shimamoto who also provided the test-case below.
    
    copy_process() uses signal->count as a reference counter, but it is not.
    This test case
    
    	#include <sys/types.h>
    	#include <sys/wait.h>
    	#include <unistd.h>
    	#include <stdio.h>
    	#include <errno.h>
    	#include <pthread.h>
    
    	void *null_thread(void *p)
    	{
    		for (;;)
    			sleep(1);
    
    		return NULL;
    	}
    
    	void *exec_thread(void *p)
    	{
    		execl("/bin/true", "/bin/true", NULL);
    
    		return null_thread(p);
    	}
    
    	int main(int argc, char **argv)
    	{
    		for (;;) {
    			pid_t pid;
    			int ret, status;
    
    			pid = fork();
    			if (pid < 0)
    				break;
    
    			if (!pid) {
    				pthread_t tid;
    
    				pthread_create(&tid, NULL, exec_thread, NULL);
    				for (;;)
    					pthread_create(&tid, NULL, null_thread, NULL);
    			}
    
    			do {
    				ret = waitpid(pid, &status, 0);
    			} while (ret == -1 && errno == EINTR);
    		}
    
    		return 0;
    	}
    
    quickly creates an unkillable task.
    
    If copy_process(CLONE_THREAD) races with de_thread()
    copy_signal()->atomic(signal->count) breaks the signal->notify_count
    logic, and the execing thread can hang forever in kernel space.
    
    Change copy_process() to increment count/live only when we know for sure
    we can't fail.  In this case the forked thread will take care of its
    reference to signal correctly.
    
    If copy_process() fails, check CLONE_THREAD flag.  If it it set - do
    nothing, the counters were not changed and current belongs to the same
    thread group.  If it is not set, ->signal must be released in any case
    (and ->count must be == 1), the forked child is the only thread in the
    thread group.
    
    We need more cleanups here, in particular signal->count should not be used
    by de_thread/__exit_signal at all.  This patch only fixes the bug.
    Reported-by: default avatarHiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
    Tested-by: default avatarHiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
    Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
    Acked-by: default avatarRoland McGrath <roland@redhat.com>
    Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    Cc: <stable@kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    4ab6c083
fork.c 41.3 KB