Issue #103, #203: Child process termination (partial fix)

When browser tab/window is closed during active session, child process
stays alive forever (even if shellinaboxd is terminated).

This fix works only if shellinaboxd is started without root privileges.
Droping them at runtime doesn't help either. Issue is related to PAM
session management process.

If we start shellinaboxd with root priviliges this fix will not affect
anything.

* When session timeouts cleanup procedure is triggered. Procedure is executed
in launcher process, because this is parent of child (service) process.
There we execute checks, if we have correct child pid (stored in session) and
than we can terminate process.
* Added debug information about cleaning up child process

https://code.google.com/p/shellinabox/issues/detail?id=103
https://code.google.com/p/shellinabox/issues/detail?id=203

shellinabox/launcher.c

@@ -520,6 +520,7 @@ int launchChild(int service, struct Session *session, const char *url) {
   ssize_t len          = sizeof(struct LaunchRequest) + strlen(u) + 1;
   check(request        = calloc(len, 1));
   request->service     = service;
+  request->terminate   = -1;
   request->width       = session->width;
   request->height      = session->height;
   strncat(request->peerName, httpGetPeerName(session->http),
@@ -547,6 +548,7 @@ int launchChild(int service, struct Session *session, const char *url) {
     return -1;
   }
   check(bytes == sizeof(pid));
+  check(session->pid = pid);
   struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
   check(cmsg);
   check(cmsg->cmsg_level == SOL_SOCKET);
@@ -555,6 +557,34 @@ int launchChild(int service, struct Session *session, const char *url) {
   return pid;
 }
 
+int terminateChild(struct Session *session) {
+  if (launcher < 0) {
+    errno              = EINVAL;
+    return -1;
+  }
+
+  if (session->pid < 1) {
+    debug("Child pid for termination not valid!");
+    return -1;
+  }
+
+  // Send terminate request to launcher process
+  struct LaunchRequest *request;
+  ssize_t len          = sizeof(struct LaunchRequest);
+  check(request        = calloc(len, 1));
+  request->terminate   = session->pid;
+  if (NOINTR(write(launcher, request, len)) != len) {
+    debug("Child %d termination request failed!", request->terminate);
+    free(request);
+    return -1;
+  }
+
+  free(request);
+  session->pid         = 0;
+  session->cleanup     = 0;
+  return 0;
+}
+
 struct Utmp {
   const char   pid[32];
   int          pty;
@@ -1616,7 +1646,7 @@ static void launcherDaemon(int fd) {
     int   status;
     pid_t pid;
     while (NOINTR(pid = waitpid(-1, &status, WNOHANG)) > 0) {
-      debug("Child %d exited with exit code %d\n", pid, WEXITSTATUS(status));
+      debug("Child %d exited with exit code %d", pid, WEXITSTATUS(status));
       if (WIFEXITED(status) || WIFSIGNALED(status)) {
         char key[32];
         snprintf(&key[0], sizeof(key), "%d", pid);
@@ -1627,6 +1657,21 @@ static void launcherDaemon(int fd) {
       continue;
     }
 
+    // Check if we received terminate request from parent process and
+    // try to terminate child, if child is still running
+    if (request.terminate > 0) {
+      errno = 0;
+      NOINTR(pid = waitpid(request.terminate, &status, WNOHANG));
+      if (pid == 0 && errno == 0) {
+        if (kill(request.terminate, SIGTERM) == 0) {
+          debug("Terminating child %d (kill)", request.terminate);
+        } else {
+          debug("Terminating child failed [%s]", strerror(errno));
+        }
+	  }
+      continue;
+    }
+
     char *url;
     check(url                 = calloc(request.urlLength + 1, 1));
   readURL:
@@ -1637,7 +1682,7 @@ static void launcherDaemon(int fd) {
       break;
     }
     while (NOINTR(pid = waitpid(-1, &status, WNOHANG)) > 0) {
-      debug("Child %d exited with exit code %d\n", pid, WEXITSTATUS(status));
+      debug("Child %d exited with exit code %d", pid, WEXITSTATUS(status));
       if (WIFEXITED(status) || WIFSIGNALED(status)) {
         char key[32];
         snprintf(&key[0], sizeof(key), "%d", pid);
@@ -1682,6 +1727,7 @@ static void launcherDaemon(int fd) {
           childProcesses      = newHashMap(destroyUtmpHashEntry, NULL);
         }
         addToHashMap(childProcesses, utmp->pid, (char *)utmp);
+        debug("Child %d launched", pid);
       } else {
         int fds[2];
         if (!pipe(fds)) {

shellinabox/launcher.h

@@ -54,6 +54,7 @@
 struct LaunchRequest {
   int   service;
   int   width, height;
+  pid_t terminate;
   char  peerName[128];
   int   urlLength;
   char  url[0];
@@ -61,6 +62,7 @@ struct LaunchRequest {
 
 int  supportsPAM(void);
 int  launchChild(int service, struct Session *session, const char *url);
+int  terminateChild(struct Session *session);
 void setWindowSize(int pty, int width, int height);
 int  forkLauncher(void);
 void terminateLauncher(void);

shellinabox/session.c

@@ -121,6 +121,8 @@ void initSession(struct Session *session, const char *sessionKey,
   session->height         = 0;
   session->buffered       = NULL;
   session->len            = 0;
+  session->pid            = 0;
+  session->cleanup        = 0;
 }
 
 struct Session *newSession(const char *sessionKey, Server *server, URL *url,

shellinabox/session.h

@@ -63,6 +63,8 @@ struct Session {
   int              height;
   char             *buffered;
   int              len;
+  pid_t            pid;
+  int              cleanup;
 };
 
 void addToGraveyard(struct Session *session);

shellinabox/shellinaboxd.c

@@ -274,8 +274,11 @@ static int completePendingRequest(struct Session *session,
 }
 
 static void sessionDone(void *arg) {
-  debug("Child terminated");
   struct Session *session = (struct Session *)arg;
+  debug("Session %s done", session->sessionKey);
+  if (session->cleanup) {
+    terminateChild(session);
+  }
   session->done           = 1;
   addToGraveyard(session);
   completePendingRequest(session, "", 0, INT_MAX);
@@ -301,6 +304,7 @@ static int handleSession(struct ServerConnection *connection, void *arg,
   if (bytes || timedOut) {
     if (!session->http && timedOut) {
       debug("Timeout. Closing session.");
+      session->cleanup = 1;
       return 0;
     }
     check(!session->done);