dgaudet 98/02/14 02:54:43
Modified: src CHANGES
src/main http_main.c
Log:
The addition of child_init_modules() and child_exit_modules() has made child
initialization a critical section. We can't service a USR1, TERM, or HUP
request during this period because all our data structures could be in
some unknown state. So block_alarms() the whole way through.
Fix a segfault caused by terminating before allocating pchild. This won't
occur with the above fix in place, but seems a reasonable precaution should
there be a need to call clean_child_exit() before pchild is allocated.
Fix various cases where exit() was called rather than clean_child_exit().
Revision Changes Path
1.630 +3 -0 apache-1.3/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.629
retrieving revision 1.630
diff -u -r1.629 -r1.630
--- CHANGES 1998/02/14 03:26:56 1.629
+++ CHANGES 1998/02/14 10:54:39 1.630
@@ -1,5 +1,8 @@
Changes with Apache 1.3b6
+ *) Cleaned up some race conditions in unix child_main during
+ initialization. [Dean Gaudet]
+
*) SECURITY: "UserDir /abspath" without a * in the path would allow
remote users to access "/~.." and bypass access restrictions
(but note /~../.. was handled properly).
1.288 +40 -22 apache-1.3/src/main/http_main.c
Index: http_main.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/main/http_main.c,v
retrieving revision 1.287
retrieving revision 1.288
diff -u -r1.287 -r1.288
--- http_main.c 1998/02/14 10:43:20 1.287
+++ http_main.c 1998/02/14 10:54:41 1.288
@@ -320,6 +320,18 @@
scoreboard *scoreboard_image = NULL;
+static APACHE_TLS int volatile exit_after_unblock = 0;
+
+/* a clean exit from a child with proper cleanup */
+static void __attribute__((noreturn)) clean_child_exit(int code)
+{
+ if (pchild) {
+ child_exit_modules(pchild, server_conf);
+ destroy_pool(pchild);
+ }
+ exit(code);
+}
+
#if defined(USE_FCNTL_SERIALIZED_ACCEPT) ||
defined(USE_FLOCK_SERIALIZED_ACCEPT)
static void expand_lock_fname(pool *p)
{
@@ -472,12 +484,12 @@
if (sigprocmask(SIG_BLOCK, &accept_block_mask, &accept_previous_mask)) {
perror("sigprocmask(SIG_BLOCK)");
- exit (1);
+ clean_child_exit(1);
}
if ((err = pthread_mutex_lock(accept_mutex))) {
errno = err;
perror("pthread_mutex_lock");
- exit(1);
+ clean_child_exit(1);
}
have_accept_mutex = 1;
}
@@ -489,7 +501,7 @@
if ((err = pthread_mutex_unlock(accept_mutex))) {
errno = err;
perror("pthread_mutex_unlock");
- exit(1);
+ clean_child_exit(1);
}
/* There is a slight race condition right here... if we were to die right
* now, we'd do another pthread_mutex_unlock. Now, doing that would let
@@ -507,7 +519,7 @@
have_accept_mutex = 0;
if (sigprocmask(SIG_SETMASK, &accept_previous_mask, NULL)) {
perror("sigprocmask(SIG_SETMASK)");
- exit (1);
+ clean_child_exit(1);
}
}
@@ -592,7 +604,7 @@
{
if (semop(sem_id, &op_on, 1) < 0) {
perror("accept_mutex_on");
- exit(1);
+ clean_child_exit(1);
}
}
@@ -600,7 +612,7 @@
{
if (semop(sem_id, &op_off, 1) < 0) {
perror("accept_mutex_off");
- exit(1);
+ clean_child_exit(1);
}
}
@@ -653,7 +665,7 @@
"fcntl: F_SETLKW: Error getting accept lock, exiting! "
"Perhaps you need to use the LockFile directive to place "
"your lock file on a local disk!");
- exit(1);
+ clean_child_exit(1);
}
}
@@ -669,7 +681,7 @@
"fcntl: F_SETLKW: Error freeing accept lock, exiting! "
"Perhaps you need to use the LockFile directive to place "
"your lock file on a local disk!");
- exit(1);
+ clean_child_exit(1);
}
}
@@ -693,7 +705,7 @@
if (lock_fd == -1) {
aplog_error(APLOG_MARK, APLOG_EMERG, server_conf,
"Child cannot open lock file: %s\n", lock_fname);
- exit(1);
+ clean_child_exit(1);
}
}
@@ -724,7 +736,7 @@
if (ret < 0) {
aplog_error(APLOG_MARK, APLOG_EMERG, server_conf,
"flock: LOCK_EX: Error getting accept lock. Exiting!");
- exit(1);
+ clean_child_exit(1);
}
}
@@ -733,7 +745,7 @@
if (flock(lock_fd, LOCK_UN) < 0) {
aplog_error(APLOG_MARK, APLOG_EMERG, server_conf,
"flock: LOCK_UN: Error freeing accept lock. Exiting!");
- exit(1);
+ clean_child_exit(1);
}
}
@@ -789,15 +801,6 @@
static APACHE_TLS const char *volatile timeout_name = NULL;
static APACHE_TLS int volatile alarms_blocked = 0;
static APACHE_TLS int volatile alarm_pending = 0;
-static APACHE_TLS int volatile exit_after_unblock = 0;
-
-/* a clean exit from a child with proper cleanup */
-static void __attribute__((noreturn)) clean_child_exit(int code)
-{
- child_exit_modules(pchild, server_conf);
- destroy_pool(pchild);
- exit(code);
-}
void timeout(int sig)
{ /* Also called on SIGPIPE */
@@ -1641,7 +1644,7 @@
if (scoreboard_fd == -1) {
perror(scoreboard_fname);
fprintf(stderr, "Cannot open scoreboard file:\n");
- exit(1);
+ clean_child_exit(1);
}
#else
#ifdef __EMX__
@@ -2905,6 +2908,18 @@
struct sockaddr sa_client;
listen_rec *lr;
+ /* All of initialization is a critical section, we don't care if we're
+ * told to HUP or USR1 before we're done initializing. For example,
+ * we could be half way through child_init_modules() when a restart
+ * signal arrives, and we'd have no real way to recover gracefully
+ * and exit properly.
+ *
+ * I suppose a module could take forever to initialize, but that would
+ * be either a broken module, or a broken configuration (i.e. network
+ * problems, file locking problems, whatever). -djg
+ */
+ block_alarms();
+
my_pid = getpid();
csd = -1;
dupped_csd = -1;
@@ -2938,7 +2953,7 @@
if (!geteuid() && setuid(user_id) == -1) {
aplog_error(APLOG_MARK, APLOG_ALERT, server_conf,
"setuid: unable to change uid");
- exit(1);
+ clean_child_exit(1);
}
#endif
@@ -2966,6 +2981,9 @@
DosSetSignalExceptionFocus(0, &ulTimes);
}
#endif
+
+ /* done with the initialization critical section */
+ unblock_alarms();
while (1) {
BUFF *conn_io;
