Re: [PATCH] Re: Zombies with 2.4.15pre5 (exit.c)

From: OGAWA Hirofumi (hirofumi@mail.parknet.co.jp)
Date: Tue Nov 20 2001 - 13:40:49 EST


Hi,

Dave McCracken <dmccr@us.ibm.com> writes:

> The intent of the original patch was to make the task unfindable to other
> waiters, which fixed a race condition in sys_wait4(). My assumption was
> that the task was about to be cleaned up in release_task(). What I missed
> was that there are a couple of code paths that don't release the task, but
> assume it'll be cleaned up later.

I think the original patch don't fix race condition, because
tasklist_lock is read_lock(). Furthermore, the threads which did not
receive process status continues waiting, even when there is no child
process.

I wrote the following patch. But, I'm not sure whether it is right.

Thanks

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

--- linux-head/kernel/exit.c Tue Nov 20 23:32:58 2001 +++ wait/kernel/exit.c Tue Nov 20 17:56:12 2001 @@ -529,23 +529,27 @@ retval = ru ? getrusage(p, RUSAGE_BOTH, ru) : 0; if (!retval && stat_addr) retval = put_user((p->exit_code << 8) | 0x7f, stat_addr); - if (!retval) { - p->exit_code = 0; - retval = p->pid; + if (retval) + goto end_wait4; + + /* exactly one thread return the process status */ + task_lock(p); + if (p->exit_code == 0) { + task_unlock(p); + goto repeat; } + p->exit_code = 0; + task_unlock(p); + retval = p->pid; goto end_wait4; case TASK_ZOMBIE: - /* Make sure no other waiter picks this task up */ - p->state = TASK_DEAD; - - current->times.tms_cutime += p->times.tms_utime + p->times.tms_cutime; - current->times.tms_cstime += p->times.tms_stime + p->times.tms_cstime; read_unlock(&tasklist_lock); retval = ru ? getrusage(p, RUSAGE_BOTH, ru) : 0; if (!retval && stat_addr) retval = put_user(p->exit_code, stat_addr); if (retval) - goto end_wait4; + goto end_wait4; + retval = p->pid; if (p->p_opptr != p->p_pptr) { write_lock_irq(&tasklist_lock); @@ -554,8 +558,20 @@ SET_LINKS(p); do_notify_parent(p, SIGCHLD); write_unlock_irq(&tasklist_lock); - } else - release_task(p); + goto end_wait4; + } + + /* exactly one thread return the process status */ + task_lock(p); + if (p->pid == 0) { + task_unlock(p); + goto repeat; + } + p->pid = 0; + task_unlock(p); + current->times.tms_cutime += p->times.tms_utime + p->times.tms_cutime; + current->times.tms_cstime += p->times.tms_stime + p->times.tms_cstime; + release_task(p); goto end_wait4; default: continue; --- linux-head/include/linux/sched.h Tue Nov 20 23:32:58 2001 +++ wait/include/linux/sched.h Tue Nov 20 17:38:11 2001 @@ -88,7 +88,6 @@ #define TASK_UNINTERRUPTIBLE 2 #define TASK_ZOMBIE 4 #define TASK_STOPPED 8 -#define TASK_DEAD 16 #define __set_task_state(tsk, state_value) \ do { (tsk)->state = (state_value); } while (0) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Nov 23 2001 - 21:00:24 EST