Messages in this thread | | | Date | Tue, 25 Aug 2009 09:10:30 +0900 | From | Hiroshi Shimamoto <> | Subject | Re: [PATCH v3] fix race copy_process() vs de_thread() |
| |
Oleg Nesterov wrote: > Spotted by Hiroshi Shimamoto who also provided the test-case below. > > copy_process() pathes use 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 the 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: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Nice fix! Tested-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
Thanks, Hiroshi
| |