Fixed signal races in shadow tools.

Some of the supplied tools use functions which are not signal-safe.

Most of the times it's exit() vs. _exit().

In other times it's how the standard output or standard error is
handled. FILE-related functions shall be avoided, therefore I replaced
them with write().

Also there is no need to call closelog(). At worst, it allows to
trigger a deadlock by issuing different signal types at bad timings.
But as these fixes are about race conditions, expect bad timings in
general for these bugs to be triggered. :)
This commit is contained in:
Tobias Stoeckmann 2016-07-02 18:11:09 +02:00
parent a4dee3d1ad
commit dd50014055
5 changed files with 20 additions and 15 deletions

View File

@ -58,7 +58,7 @@ static void process_flags (int argc, char **argv);
*/ */
static RETSIGTYPE catch_signals (unused int sig) static RETSIGTYPE catch_signals (unused int sig)
{ {
exit (10); _exit (10);
} }
/* /*

View File

@ -169,9 +169,8 @@ static RETSIGTYPE catch_signals (int killed)
} }
if (0 != killed) { if (0 != killed) {
(void) putchar ('\n'); (void) write (STDOUT_FILENO, "\n", 1);
(void) fflush (stdout); _exit (killed);
exit (killed);
} }
} }

View File

@ -103,7 +103,7 @@ static bool hflg = false;
static bool preauth_flag = false; static bool preauth_flag = false;
static bool amroot; static bool amroot;
static unsigned int timeout; static char tmsg[256];
/* /*
* External identifiers. * External identifiers.
@ -416,8 +416,8 @@ static void init_env (void)
static RETSIGTYPE alarm_handler (unused int sig) static RETSIGTYPE alarm_handler (unused int sig)
{ {
fprintf (stderr, _("\nLogin timed out after %u seconds.\n"), timeout); write (STDERR_FILENO, tmsg, strlen (tmsg));
exit (0); _exit (0);
} }
#ifdef USE_PAM #ifdef USE_PAM
@ -532,6 +532,7 @@ int main (int argc, char **argv)
bool is_console; bool is_console;
#endif #endif
int err; int err;
unsigned int timeout;
const char *cp; const char *cp;
const char *tmp; const char *tmp;
char fromhost[512]; char fromhost[512];
@ -698,8 +699,10 @@ int main (int argc, char **argv)
top: top:
/* only allow ALARM sec. for login */ /* only allow ALARM sec. for login */
(void) signal (SIGALRM, alarm_handler);
timeout = getdef_unum ("LOGIN_TIMEOUT", ALARM); timeout = getdef_unum ("LOGIN_TIMEOUT", ALARM);
snprintf (tmsg, sizeof tmsg,
_("\nLogin timed out after %u seconds.\n"), timeout);
(void) signal (SIGALRM, alarm_handler);
if (timeout > 0) { if (timeout > 0) {
(void) alarm (timeout); (void) alarm (timeout);
} }

View File

@ -105,6 +105,8 @@ static char caller_name[BUFSIZ];
static bool change_environment = true; static bool change_environment = true;
#ifdef USE_PAM #ifdef USE_PAM
static char kill_msg[256];
static char wait_msg[256];
static pam_handle_t *pamh = NULL; static pam_handle_t *pamh = NULL;
static int caught = 0; static int caught = 0;
/* PID of the child, in case it needs to be killed */ /* PID of the child, in case it needs to be killed */
@ -161,8 +163,7 @@ static RETSIGTYPE die (int killed)
} }
if (killed != 0) { if (killed != 0) {
closelog (); _exit (128+killed);
exit (128+killed);
} }
} }
@ -182,12 +183,11 @@ static RETSIGTYPE kill_child (int unused(s))
{ {
if (0 != pid_child) { if (0 != pid_child) {
(void) kill (-pid_child, SIGKILL); (void) kill (-pid_child, SIGKILL);
(void) fputs (_(" ...killed.\n"), stderr); (void) write (STDERR_FILENO, kill_msg, strlen (kill_msg));
} else { } else {
(void) fputs (_(" ...waiting for child to terminate.\n"), (void) write (STDERR_FILENO, wait_msg, strlen (wait_msg));
stderr);
} }
exit (255); _exit (255);
} }
#endif /* USE_PAM */ #endif /* USE_PAM */
@ -373,6 +373,9 @@ static void prepare_pam_close_session (void)
stderr); stderr);
(void) kill (-pid_child, caught); (void) kill (-pid_child, caught);
snprintf (kill_msg, _(" ...killed.\n"));
snprintf (wait_msg, _(" ...waiting for child to terminate.\n"));
(void) signal (SIGALRM, kill_child); (void) signal (SIGALRM, kill_child);
(void) alarm (2); (void) alarm (2);

View File

@ -70,7 +70,7 @@ static RETSIGTYPE catch_signals (int);
static RETSIGTYPE catch_signals (unused int sig) static RETSIGTYPE catch_signals (unused int sig)
{ {
exit (1); _exit (1);
} }
/* /*