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

OGAWA Hirofumi
2001-11-20T18:44:33Z

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)