logwrapper: switch to signal handlers for SIGCHLD

Switch to a signal handler to monitor SIGCHLD.
Also, clean up the exit path and log a warning when the
previous SIGCHLD sigaction is not default or ignore.

Change-Id: I7ef7daabee18d80e9e82fba3943cf1d03b504ae3

1 files changed, 134 insertions, 75 deletions

diff --git a/logwrapper/logwrap.c b/logwrapper/logwrap.c
index c2b36be..9cd3fe4 100644
--- a/logwrapper/logwrap.c
+++ b/logwrapper/logwrap.c
@@ -16,7 +16,7 @@
 
 #include <string.h>
 #include <sys/types.h>
-#include <sys/signalfd.h>
+#include <sys/socket.h>
 #include <signal.h>
 #include <poll.h>
 #include <sys/wait.h>
@@ -26,6 +26,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <libgen.h>
+#include <stdbool.h>
 
 #include <logwrap/logwrap.h>
 #include "private/android_filesystem_config.h"
@@ -33,83 +34,104 @@
 
 #define ARRAY_SIZE(x)	(sizeof(x) / sizeof(*(x)))
 
-static int fatal(const char *msg) {
+static int signal_fd_write;
+
+static void fatal(const char *msg) {
     fprintf(stderr, "%s", msg);
     ALOG(LOG_ERROR, "logwrapper", "%s", msg);
-    return -1;
 }
 
 static int parent(const char *tag, int parent_read, int signal_fd, pid_t pid,
         int *chld_sts) {
-    int status;
+    int status = 0;
     char buffer[4096];
     struct pollfd poll_fds[] = {
         [0] = {
-            .fd = parent_read,
+            .fd = signal_fd,
             .events = POLLIN,
         },
         [1] = {
-            .fd = signal_fd,
+            .fd = parent_read,
             .events = POLLIN,
         },
     };
+    int rc = 0;
+    sigset_t chldset;
 
     int a = 0;  // start index of unprocessed data
     int b = 0;  // end index of unprocessed data
     int sz;
+    bool remote_hung = false;
+    bool found_child = false;
 
     char *btag = basename(tag);
     if (!btag) btag = (char*) tag;
 
-    while (1) {
-        if (poll(poll_fds, ARRAY_SIZE(poll_fds), -1) <= 0) {
-            return fatal("poll failed\n");
+    sigemptyset(&chldset);
+    sigaddset(&chldset, SIGCHLD);
+    sigprocmask(SIG_UNBLOCK, &chldset, NULL);
+
+    while (!found_child) {
+        if (poll(poll_fds, remote_hung ? 1 : 2, -1) < 0) {
+            if (errno == EINTR)
+                continue;
+            fatal("poll failed\n");
+            rc = -1;
+            goto err_poll;
         }
 
-        if (poll_fds[0].revents & POLLIN) {
-            sz = read(parent_read, &buffer[b], sizeof(buffer) - 1 - b);
+        if (!remote_hung) {
+            if (poll_fds[1].revents & POLLIN) {
+                sz = read(parent_read, &buffer[b], sizeof(buffer) - 1 - b);
+
+                sz += b;
+                // Log one line at a time
+                for (b = 0; b < sz; b++) {
+                    if (buffer[b] == '\r') {
+                        buffer[b] = '\0';
+                    } else if (buffer[b] == '\n') {
+                        buffer[b] = '\0';
+                        ALOG(LOG_INFO, btag, "%s", &buffer[a]);
+                        a = b + 1;
+                    }
+                }
 
-            sz += b;
-            // Log one line at a time
-            for (b = 0; b < sz; b++) {
-                if (buffer[b] == '\r') {
-                    buffer[b] = '\0';
-                } else if (buffer[b] == '\n') {
+                if (a == 0 && b == sizeof(buffer) - 1) {
+                    // buffer is full, flush
                     buffer[b] = '\0';
                     ALOG(LOG_INFO, btag, "%s", &buffer[a]);
-                    a = b + 1;
+                    b = 0;
+                } else if (a != b) {
+                    // Keep left-overs
+                    b -= a;
+                    memmove(buffer, &buffer[a], b);
+                    a = 0;
+                } else {
+                    a = 0;
+                    b = 0;
                 }
             }
 
-            if (a == 0 && b == sizeof(buffer) - 1) {
-                // buffer is full, flush
-                buffer[b] = '\0';
-                ALOG(LOG_INFO, btag, "%s", &buffer[a]);
-                b = 0;
-            } else if (a != b) {
-                // Keep left-overs
-                b -= a;
-                memmove(buffer, &buffer[a], b);
-                a = 0;
-            } else {
-                a = 0;
-                b = 0;
+            if (poll_fds[1].revents & POLLHUP) {
+                remote_hung = true;
             }
         }
 
-        if (poll_fds[1].revents & POLLIN) {
-            struct signalfd_siginfo sfd_info;
-            pid_t wpid;
+        if (poll_fds[0].revents & POLLIN) {
+            char tmp[32];
+            int ret;
 
-            // Clear all pending signals before reading the child's status
-            while (read(signal_fd, &sfd_info, sizeof(sfd_info)) > 0) {
-                if ((pid_t)sfd_info.ssi_pid != pid)
-                    ALOG(LOG_WARN, "logwrapper", "cleared SIGCHLD for pid %u\n",
-                            sfd_info.ssi_pid);
+            read(signal_fd, tmp, sizeof(tmp));
+            while (!found_child) {
+                do {
+                    ret = waitpid(-1, &status, WNOHANG);
+                } while (ret < 0 && errno == EINTR);
+
+                if (ret <= 0)
+                    break;
+
+                found_child = (pid == ret);
             }
-            wpid = waitpid(pid, &status, WNOHANG);
-            if (wpid > 0)
-                break;
         }
     }
 
@@ -121,19 +143,20 @@ static int parent(const char *tag, int parent_read, int signal_fd, pid_t pid,
 
     if (WIFEXITED(status)) {
         if (WEXITSTATUS(status))
-            ALOG(LOG_INFO, "logwrapper", "%s terminated by exit(%d)", tag,
+            ALOG(LOG_INFO, "logwrapper", "%s terminated by exit(%d)", btag,
                     WEXITSTATUS(status));
     } else if (WIFSIGNALED(status)) {
-        ALOG(LOG_INFO, "logwrapper", "%s terminated by signal %d", tag,
+        ALOG(LOG_INFO, "logwrapper", "%s terminated by signal %d", btag,
                 WTERMSIG(status));
     } else if (WIFSTOPPED(status)) {
-        ALOG(LOG_INFO, "logwrapper", "%s stopped by signal %d", tag,
+        ALOG(LOG_INFO, "logwrapper", "%s stopped by signal %d", btag,
                 WSTOPSIG(status));
     }
     if (chld_sts != NULL)
         *chld_sts = status;
 
-    return 0;
+err_poll:
+    return rc;
 }
 
 static void child(int argc, char* argv[]) {
@@ -149,40 +172,54 @@ static void child(int argc, char* argv[]) {
     }
 }
 
+void sigchld_handler(int sig) {
+    write(signal_fd_write, &sig, 1);
+}
+
 int logwrap(int argc, char* argv[], int *status) {
     pid_t pid;
-
     int parent_ptty;
     int child_ptty;
     char *child_devname = NULL;
-    sigset_t chldset;
+    struct sigaction chldact;
+    struct sigaction oldchldact;
+    sigset_t blockset;
+    sigset_t oldset;
+    int sockets[2];
+    int rc = 0;
 
     /* Use ptty instead of socketpair so that STDOUT is not buffered */
     parent_ptty = open("/dev/ptmx", O_RDWR);
     if (parent_ptty < 0) {
-        return fatal("Cannot create parent ptty\n");
+        fatal("Cannot create parent ptty\n");
+        rc = -1;
+        goto err_open;
     }
 
     if (grantpt(parent_ptty) || unlockpt(parent_ptty) ||
             ((child_devname = (char*)ptsname(parent_ptty)) == 0)) {
-        return fatal("Problem with /dev/ptmx\n");
+        fatal("Problem with /dev/ptmx\n");
+        rc = -1;
+        goto err_ptty;
     }
 
-    sigemptyset(&chldset);
-    sigaddset(&chldset, SIGCHLD);
-    sigprocmask(SIG_BLOCK, &chldset, NULL);
+    sigemptyset(&blockset);
+    sigaddset(&blockset, SIGCHLD);
+    sigprocmask(SIG_BLOCK, &blockset, &oldset);
 
     pid = fork();
     if (pid < 0) {
-        close(parent_ptty);
-        sigprocmask(SIG_UNBLOCK, &chldset, NULL);
-        return fatal("Failed to fork\n");
+        fatal("Failed to fork\n");
+        rc = -1;
+        goto err_fork;
     } else if (pid == 0) {
+        sigprocmask(SIG_SETMASK, &oldset, NULL);
         close(parent_ptty);
-        sigprocmask(SIG_UNBLOCK, &chldset, NULL);
+
         child_ptty = open(child_devname, O_RDWR);
         if (child_ptty < 0) {
-            return fatal("Problem with child ptty\n");
+            fatal("Problem with child ptty\n");
+            return -1;
         }
 
         // redirect stdout and stderr
@@ -191,35 +228,57 @@ int logwrap(int argc, char* argv[], int *status) {
         close(child_ptty);
 
         child(argc - 1, &argv[1]);
-        return fatal("This should never happen\n");
-
+        fatal("This should never happen\n");
+        return -1;
     } else {
-        int rc;
-        int fd;
+        memset(&chldact, 0, sizeof(chldact));
+        chldact.sa_handler = sigchld_handler;
+        chldact.sa_flags = SA_NOCLDSTOP;
+
+        sigaction(SIGCHLD, &chldact, &oldchldact);
+        if ((!(oldchldact.sa_flags & SA_SIGINFO) &&
+                oldchldact.sa_handler != SIG_DFL &&
+                oldchldact.sa_handler != SIG_IGN) ||
+                ((oldchldact.sa_flags & SA_SIGINFO) &&
+                oldchldact.sa_sigaction != NULL)) {
+            ALOG(LOG_WARN, "logwrapper", "logwrap replaced the SIGCHLD "
+                    "handler and might cause interaction issues");
+        }
 
-        fd = signalfd(-1, &chldset, SFD_NONBLOCK);
-        if (fd == -1) {
+        rc = socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);
+        if (rc == -1) {
             char msg[40];
 
-            snprintf(msg, sizeof(msg), "signalfd failed: %d\n", errno);
+            snprintf(msg, sizeof(msg), "socketpair failed: %d\n", errno);
 
-            close(parent_ptty);
-            sigprocmask(SIG_UNBLOCK, &chldset, NULL);
-            return fatal(msg);
+            fatal(msg);
+            goto err_socketpair;
         }
 
+        fcntl(sockets[0], F_SETFD, FD_CLOEXEC);
+        fcntl(sockets[0], F_SETFL, O_NONBLOCK);
+        fcntl(sockets[1], F_SETFD, FD_CLOEXEC);
+        fcntl(sockets[1], F_SETFL, O_NONBLOCK);
+
+        signal_fd_write = sockets[0];
+
         // switch user and group to "log"
         // this may fail if we are not root,
         // but in that case switching user/group is unnecessary
         setgid(AID_LOG);
         setuid(AID_LOG);
 
-        rc = parent(argv[1], parent_ptty, fd, pid, status);
-        close(parent_ptty);
-        close(fd);
-
-        sigprocmask(SIG_UNBLOCK, &chldset, NULL);
-
-        return rc;
+        rc = parent(argv[1], parent_ptty, sockets[1], pid, status);
     }
+
+    close(sockets[0]);
+    close(sockets[1]);
+err_socketpair:
+    sigaction(SIGCHLD, &oldchldact, NULL);
+err_fork:
+    sigprocmask(SIG_SETMASK, &oldset, NULL);
+err_ptty:
+    close(parent_ptty);
+err_open:
+    return rc;
 }