[BusyBox] [gemorin at debian.org: Bug#212764: 2 race conditions in init implementation]
Bastian Blank
waldi at debian.org
Fri Sep 26 06:50:35 UTC 2003
----- Forwarded message from Guillaume Morin <gemorin at debian.org> -----
Subject: Bug#212764: 2 race conditions in init implementation
Date: Thu, 25 Sep 2003 17:41:58 -0400
From: Guillaume Morin <gemorin at debian.org>
To: Debian Bug Tracking System <submit at bugs.debian.org>
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.
----- End forwarded message -----
Bastian
--
Four thousand throats may be cut in one night by a running man.
-- Klingon Soldier, "Day of the Dove", stardate unknown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://lists.busybox.net/pipermail/busybox/attachments/20030926/fece58ea/attachment-0002.pgp
More information about the busybox
mailing list