lkml.org 
[lkml]   [2009]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2009-08-25 02:23    [W:0.480 / U:0.580 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site