summary |
shortlog |
log |
commit | commitdiff |
tree
raw |
patch |
inline | side by side (from parent 1:
e2ad488)
Currently both the generic signal handler in signal.c and the signal
handler for the stat command handler in command.c call daemon_log()
via PARA_EMERG_LOG(). This is problematic because daemon_log()
takes the log mutex and the signal might arrive while daemon_log()
is executing. If this race condition is hit, the process deadlocks
because daemon_log() tries to acquire a mutex which it already holds.
All three types of server processes (main, afs and command handler)
are susceptible to this bug, but regardless of which process happens
to hit the race window, the server process hangs waiting on the mutex,
and no longer accepts connections.
Fix this by removing the problematic log call in the generic case and
by printing it out of interrupt context in the command handler case.
This bug was introduced together with the log mutex five years ago.
Fixes: ced0c17d1a3ee0336dc7b35e69faff131dabecac
int send_afs_status(struct command_context *cc, int parser_friendly);
static bool subcmd_should_die;
int send_afs_status(struct command_context *cc, int parser_friendly);
static bool subcmd_should_die;
+/*
+ * Don't call PARA_XXX_LOG() here as we might already hold the log mutex. See
+ * generic_signal_handler() for details.
+ */
static void command_handler_sighandler(int s)
{
static void command_handler_sighandler(int s)
{
- if (s != SIGTERM)
- return;
- PARA_EMERG_LOG("terminating on signal %d\n", SIGTERM);
- subcmd_should_die = true;
+ if (s == SIGTERM)
+ subcmd_should_die = true;
* open a race window similar to the one described above.
*/
pselect(1, NULL, NULL, NULL, &ts, &set);
* open a race window similar to the one described above.
*/
pselect(1, NULL, NULL, NULL, &ts, &set);
+ if (subcmd_should_die) {
+ PARA_EMERG_LOG("terminating on SIGTERM\n");
ret = -E_SERVER_CRASH;
if (getppid() == 1)
goto out;
ret = -E_SERVER_CRASH;
if (getppid() == 1)
goto out;
errno = save_errno;
return;
}
errno = save_errno;
return;
}
- if (ret < 0)
- PARA_EMERG_LOG("%s\n", strerror(errno));
- else
- PARA_EMERG_LOG("short write to signal pipe\n");
+ /*
+ * This is a fatal error which should never happen. We must not call
+ * PARA_XXX_LOG() here because this might acquire the log mutex which
+ * is already taken by the main program if the interrupt occurs while a
+ * log message is being printed. The mutex will not be released as long
+ * as this signal handler is running, so a deadlock ensues.
+ */