Bug#212764: 2 race conditions in init implementation
Package: busybox-cvs
Version: 0.60.99.cvs20030819-3 (not installed)
Severity: important
Tags: patch
Hi,
I have found two races conditions in the run() function of busybox's
init implementation (static pid_t run(const struct init_action *a)).
The runtime behavior is that the call to wait in waitfor() blocks
forever. I can easily reproduce it on SMP system. Actually, it should
only easily reproducable on 2 processors systems.
- 1st race condition
around line 450
sigemptyset(&nmask);
sigaddset(&nmask, SIGCHLD);
sigprocmask(SIG_BLOCK, &nmask, &omask);
if ((pid = fork()) == 0) {
/* exec the command here and exit*/
}
sigprocmask(SIG_SETMASK, &omask, NULL);
return pid;
/* the parent will call wait() after returning */
There is a race here. If the child exits before the SIGCHLD is
unblocked, wait() will block forever.
I have fixed the bug by replacing the wait() call in waitfor() with a
waitpid() call and handle the error.
- 2nd race condition
on line 486
/* Now fork off another process to just hang around */
if ((pid = fork()) < 0) {
message(LOG | CONSOLE, "Can't fork!");
_exit(1);
}
if (pid > 0) {
/* We are the parent -- wait till the child is done */
signal(SIGINT, SIG_IGN);
signal(SIGTSTP, SIG_IGN);
signal(SIGQUIT, SIG_IGN);
signal(SIGCHLD, SIG_DFL);
/* Wait for child to exit */
while ((tmp_pid = waitpid(pid, &junk, 0)) != pid);
If the child exits before the call to signal(SIGCHLD, SIG_DFL), another
sigchld handler might be executed or the signal may be ignored. In both
cases, waitpid() won't work, so this condition should be tested. I have
modified that in my patch.
There is another possible fix. You can move signal(SIGCHLD, SIG_DFL)
before the fork on line 457 ... Your call, this may be cleaner. The
signal calls are still very racy but it should not really matter.
--- /home/guillaum/tmp/burp/busybox-cvs-0.60.99.cvs20030819/init/init.c 2003-08-19 11:59:13.000000000 +0000
+++ busybox-cvs-0.60.99.cvs20030819/init/init.c 2003-09-25 21:24:41.000000000 +0000
@@ -496,7 +496,11 @@
signal(SIGCHLD, SIG_DFL);
/* Wait for child to exit */
- while ((tmp_pid = waitpid(pid, &junk, 0)) != pid);
+ while ((tmp_pid = waitpid(pid, &junk, 0)) != pid) {
+ if (tmp_pid == -1 && errno == ECHILD)
+ break;
+ // FIXME handle other errors
+ }
/* See if stealing the controlling tty back is necessary */
pgrp = tcgetpgrp(0);
@@ -620,12 +624,14 @@
pid = run(a);
while (1) {
- wpid = wait(&status);
- if (wpid > 0 && wpid != pid) {
- continue;
- }
+ wpid = waitpid(pid,&status,0);
if (wpid == pid)
break;
+ if (wpid == -1 && errno == ECHILD)
+ /* we missed its termination */
+ break;
+ /* FIXME other errors should maybe trigger an error, but allow
+ * the program to continue */
}
return wpid;
}
HTH.
Guillaume.
-- System Information:
Debian Release: testing/unstable
Architecture: powerpc
Kernel: Linux siri 2.4.22-ben2 #19 ven sep 12 09:08:08 EDT 2003 ppc
Locale: LANG=C, LC_CTYPE=C (ignored: LC_ALL set to fr_FR@euro)
--
Guillaume Morin <guillaume@morinfr.org>
I love you, I love you like a chocolate cake, like the trains, like the sea.
See me loving you, please. (Dionysos)
Reply to: