]> granicus.if.org Git - shadow/commitdiff
Fixed signal races in shadow tools.
authorTobias Stoeckmann <tobias@stoeckmann.org>
Sat, 2 Jul 2016 16:11:09 +0000 (18:11 +0200)
committerTobias Stoeckmann <tobias@stoeckmann.org>
Sat, 2 Jul 2016 16:11:09 +0000 (18:11 +0200)
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. :)

src/expiry.c
src/gpasswd.c
src/login.c
src/su.c
src/sulogin.c

index 4ae47703744eed3862f11cbab615fec916e9f8eb..41add942cc221d5a0aa4d751a68449a57873f535 100644 (file)
@@ -58,7 +58,7 @@ static void process_flags (int argc, char **argv);
  */
 static RETSIGTYPE catch_signals (unused int sig)
 {
-       exit (10);
+       _exit (10);
 }
 
 /*
index 27ad9599c1123e21241622098099f656de542d3b..c4a492b105ec46d9cc46c1ad029e50a60c684fa5 100644 (file)
@@ -169,9 +169,8 @@ static RETSIGTYPE catch_signals (int killed)
        }
 
        if (0 != killed) {
-               (void) putchar ('\n');
-               (void) fflush (stdout);
-               exit (killed);
+               (void) write (STDOUT_FILENO, "\n", 1);
+               _exit (killed);
        }
 }
 
index d610844d0e83a36404f5ddb599c8bdad9d0f1f71..2d2e704e9931d4692745d5a37f69a526bf50705a 100644 (file)
@@ -103,7 +103,7 @@ static bool hflg = false;
 static bool preauth_flag = false;
 
 static bool amroot;
-static unsigned int timeout;
+static char tmsg[256];
 
 /*
  * External identifiers.
@@ -416,8 +416,8 @@ static void init_env (void)
 
 static RETSIGTYPE alarm_handler (unused int sig)
 {
-       fprintf (stderr, _("\nLogin timed out after %u seconds.\n"), timeout);
-       exit (0);
+       write (STDERR_FILENO, tmsg, strlen (tmsg));
+       _exit (0);
 }
 
 #ifdef USE_PAM
@@ -532,6 +532,7 @@ int main (int argc, char **argv)
        bool is_console;
 #endif
        int err;
+       unsigned int timeout;
        const char *cp;
        const char *tmp;
        char fromhost[512];
@@ -698,8 +699,10 @@ int main (int argc, char **argv)
 
       top:
        /* only allow ALARM sec. for login */
-       (void) signal (SIGALRM, alarm_handler);
        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) {
                (void) alarm (timeout);
        }
index 37042172e4bb2f78fd76d8c42f19a81b80e2ee51..2c27e98ea091208d9734dfc55b93b1101d399b13 100644 (file)
--- a/src/su.c
+++ b/src/su.c
@@ -105,6 +105,8 @@ static char caller_name[BUFSIZ];
 static bool change_environment = true;
 
 #ifdef USE_PAM
+static char kill_msg[256];
+static char wait_msg[256];
 static pam_handle_t *pamh = NULL;
 static int caught = 0;
 /* PID of the child, in case it needs to be killed */
@@ -161,8 +163,7 @@ static RETSIGTYPE die (int killed)
        }
 
        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) {
                (void) kill (-pid_child, SIGKILL);
-               (void) fputs (_(" ...killed.\n"), stderr);
+               (void) write (STDERR_FILENO, kill_msg, strlen (kill_msg));
        } else {
-               (void) fputs (_(" ...waiting for child to terminate.\n"),
-                             stderr);
+               (void) write (STDERR_FILENO, wait_msg, strlen (wait_msg));
        }
-       exit (255);
+       _exit (255);
 }
 #endif                         /* USE_PAM */
 
@@ -373,6 +373,9 @@ static void prepare_pam_close_session (void)
                              stderr);
                (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) alarm (2);
 
index ccbf2c5d2da804a82f9fb8360f7d1c6d14858178..4264099bc48c8c3992bac413bccc138ea11b4939 100644 (file)
@@ -70,7 +70,7 @@ static RETSIGTYPE catch_signals (int);
 
 static RETSIGTYPE catch_signals (unused int sig)
 {
-       exit (1);
+       _exit (1);
 }
 
 /*