logwrapper: prevent logwrap from hanging when child dies

Sometimes the read on the PTY can wait indefinitely if the child
dies. By using a poll statement that monitors both the output
of the child and its state we prevent this from happening.

Change-Id: I51d5556c66f039bca673145ca72db262977e1689

1 files changed, 108 insertions, 44 deletions

diff --git a/logwrapper/logwrap.c b/logwrapper/logwrap.c
index 302a739dd..c2b36be29 100644
--- a/logwrapper/logwrap.c
+++ b/logwrapper/logwrap.c
@@ -16,6 +16,9 @@
 
 #include <string.h>
 #include <sys/types.h>
+#include <sys/signalfd.h>
+#include <signal.h>
+#include <poll.h>
 #include <sys/wait.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -28,15 +31,28 @@
 #include "private/android_filesystem_config.h"
 #include "cutils/log.h"
 
+#define ARRAY_SIZE(x)	(sizeof(x) / sizeof(*(x)))
+
 static int fatal(const char *msg) {
     fprintf(stderr, "%s", msg);
     ALOG(LOG_ERROR, "logwrapper", "%s", msg);
     return -1;
 }
 
-void parent(const char *tag, int parent_read, int *chld_sts) {
+static int parent(const char *tag, int parent_read, int signal_fd, pid_t pid,
+        int *chld_sts) {
     int status;
     char buffer[4096];
+    struct pollfd poll_fds[] = {
+        [0] = {
+            .fd = parent_read,
+            .events = POLLIN,
+        },
+        [1] = {
+            .fd = signal_fd,
+            .events = POLLIN,
+        },
+    };
 
     int a = 0;  // start index of unprocessed data
     int b = 0;  // end index of unprocessed data
@@ -45,60 +61,82 @@ void parent(const char *tag, int parent_read, int *chld_sts) {
     char *btag = basename(tag);
     if (!btag) btag = (char*) tag;
 
-    while ((sz = read(parent_read, &buffer[b], sizeof(buffer) - 1 - b)) > 0) {
+    while (1) {
+        if (poll(poll_fds, ARRAY_SIZE(poll_fds), -1) <= 0) {
+            return fatal("poll failed\n");
+        }
 
-        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 (poll_fds[0].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;
+                }
+            }
+
+            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 & POLLIN) {
+            struct signalfd_siginfo sfd_info;
+            pid_t wpid;
 
+            // 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);
+            }
+            wpid = waitpid(pid, &status, WNOHANG);
+            if (wpid > 0)
+                break;
+        }
     }
+
     // Flush remaining data
     if (a != b) {
         buffer[b] = '\0';
         ALOG(LOG_INFO, btag, "%s", &buffer[a]);
     }
 
-    if (wait(&status) != -1) {  // Wait for child
-        if (WIFEXITED(status) && WEXITSTATUS(status))
+    if (WIFEXITED(status)) {
+        if (WEXITSTATUS(status))
             ALOG(LOG_INFO, "logwrapper", "%s terminated by exit(%d)", tag,
                     WEXITSTATUS(status));
-        else if (WIFSIGNALED(status))
-            ALOG(LOG_INFO, "logwrapper", "%s terminated by signal %d", tag,
-                    WTERMSIG(status));
-        else if (WIFSTOPPED(status))
-            ALOG(LOG_INFO, "logwrapper", "%s stopped by signal %d", tag,
-                    WSTOPSIG(status));
-        if (chld_sts != NULL)
-            *chld_sts = status;
-    } else
-        ALOG(LOG_INFO, "logwrapper", "%s wait() failed: %s (%d)", tag,
-                strerror(errno), errno);
+    } else if (WIFSIGNALED(status)) {
+        ALOG(LOG_INFO, "logwrapper", "%s terminated by signal %d", tag,
+                WTERMSIG(status));
+    } else if (WIFSTOPPED(status)) {
+        ALOG(LOG_INFO, "logwrapper", "%s stopped by signal %d", tag,
+                WSTOPSIG(status));
+    }
+    if (chld_sts != NULL)
+        *chld_sts = status;
+
+    return 0;
 }
 
-void child(int argc, char* argv[]) {
+static void child(int argc, char* argv[]) {
     // create null terminated argv_child array
     char* argv_child[argc + 1];
     memcpy(argv_child, argv, argc * sizeof(char *));
@@ -111,12 +149,13 @@ void child(int argc, char* argv[]) {
     }
 }
 
-int logwrap(int argc, char* argv[], int *chld_sts) {
+int logwrap(int argc, char* argv[], int *status) {
     pid_t pid;
 
     int parent_ptty;
     int child_ptty;
     char *child_devname = NULL;
+    sigset_t chldset;
 
     /* Use ptty instead of socketpair so that STDOUT is not buffered */
     parent_ptty = open("/dev/ptmx", O_RDWR);
@@ -129,13 +168,19 @@ int logwrap(int argc, char* argv[], int *chld_sts) {
         return fatal("Problem with /dev/ptmx\n");
     }
 
+    sigemptyset(&chldset);
+    sigaddset(&chldset, SIGCHLD);
+    sigprocmask(SIG_BLOCK, &chldset, NULL);
+
     pid = fork();
     if (pid < 0) {
         close(parent_ptty);
+        sigprocmask(SIG_UNBLOCK, &chldset, NULL);
         return fatal("Failed to fork\n");
     } else if (pid == 0) {
-        child_ptty = open(child_devname, O_RDWR);
         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");
         }
@@ -146,16 +191,35 @@ int logwrap(int argc, char* argv[], int *chld_sts) {
         close(child_ptty);
 
         child(argc - 1, &argv[1]);
+        return fatal("This should never happen\n");
 
     } else {
+        int rc;
+        int fd;
+
+        fd = signalfd(-1, &chldset, SFD_NONBLOCK);
+        if (fd == -1) {
+            char msg[40];
+
+            snprintf(msg, sizeof(msg), "signalfd failed: %d\n", errno);
+
+            close(parent_ptty);
+            sigprocmask(SIG_UNBLOCK, &chldset, NULL);
+            return fatal(msg);
+        }
+
         // switch user and group to "log"
-        // this may fail if we are not root, 
-        // but in that case switching user/group is unnecessary 
+        // this may fail if we are not root,
+        // but in that case switching user/group is unnecessary
         setgid(AID_LOG);
         setuid(AID_LOG);
 
-        parent(argv[1], parent_ptty, chld_sts);
-    }
+        rc = parent(argv[1], parent_ptty, fd, pid, status);
+        close(parent_ptty);
+        close(fd);
 
-    return 0;
+        sigprocmask(SIG_UNBLOCK, &chldset, NULL);
+
+        return rc;
+    }
 }