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;