From ea0cd0fdc31c54fe046d0ba50983a9a1d3742abd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Tue, 20 Jun 2017 21:40:47 +0200
Subject: [PATCH] qrexec: fix reporting exit code in qrexec-client-vm

1. If local process is started, report its exit code, instaed of remote
one. To get remote exit code, simply start qrexec-client-vm without
third argument (and connect its stdin/stdout with the other process some
other way).

2. Report process terminated by signal.
Don't pretend that process terminated by signal finished successfuly.
Copy shell behaviour of reporting it as 128+signum.

3. Do not wait() for any child process, just the one we expect. In case
of qrexec-client-vm the child process is started differently and
wait()ing on it inside main loop would break its exit code reporting.

Fixes QubesOS/qubes-issues#2861
---
 doc/vm-tools/qrexec-client-vm.rst |  3 +++
 qrexec/qrexec-agent-data.c        | 28 ++++++++++++++--------------
 qrexec/qrexec-client-vm.c         | 12 ++++++++++--
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/doc/vm-tools/qrexec-client-vm.rst b/doc/vm-tools/qrexec-client-vm.rst
index cdb10e5..4cefa9d 100644
--- a/doc/vm-tools/qrexec-client-vm.rst
+++ b/doc/vm-tools/qrexec-client-vm.rst
@@ -67,6 +67,9 @@ If service call is allowed by dom0 and ``qrexec-client-vm`` is started with
 *local_program* argument, it reports the local program exit code. There is no
 way to learn exit code of remote service in this case.
 
+In both cases, if process (local or remote) was terminated by a signal, exit
+status is 128+signal number.
+
 If service call is denied by dom0, ``qrexec-client-vm`` exit with status 126.
 
 AUTHORS
diff --git a/qrexec/qrexec-agent-data.c b/qrexec/qrexec-agent-data.c
index 1d0da97..89ff084 100644
--- a/qrexec/qrexec-agent-data.c
+++ b/qrexec/qrexec-agent-data.c
@@ -48,7 +48,6 @@ int remote_process_status = 0;
 static void sigchld_handler(int __attribute__((__unused__))x)
 {
     child_exited = 1;
-    signal(SIGCHLD, sigchld_handler);
 }
 
 static void sigusr1_handler(int __attribute__((__unused__))x)
@@ -308,20 +307,21 @@ int process_child_io(libvchan_t *data_vchan,
     buffer_init(&stdin_buf);
     while (1) {
         if (child_exited) {
-            pid_t pid;
             int status;
-            while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
-                if (pid == child_process_pid) {
+            if (child_process_pid &&
+                    waitpid(child_process_pid, &status, WNOHANG) > 0) {
+                if (WIFSIGNALED(status))
+                    child_process_status = 128 + WTERMSIG(status);
+                else
                     child_process_status = WEXITSTATUS(status);
-                    if (stdin_fd >= 0) {
-                        /* restore flags */
-                        set_block(stdin_fd);
-                        if (shutdown(stdin_fd, SHUT_WR) < 0) {
-                            if (errno == ENOTSOCK)
-                                close(stdin_fd);
-                        }
-                        stdin_fd = -1;
+                if (stdin_fd >= 0) {
+                    /* restore flags */
+                    set_block(stdin_fd);
+                    if (shutdown(stdin_fd, SHUT_WR) < 0) {
+                        if (errno == ENOTSOCK)
+                            close(stdin_fd);
                     }
+                    stdin_fd = -1;
                 }
             }
             child_exited = 0;
@@ -341,8 +341,6 @@ int process_child_io(libvchan_t *data_vchan,
         if (!libvchan_data_ready(data_vchan) &&
                 !libvchan_is_open(data_vchan) &&
                 !buffer_len(&stdin_buf)) {
-            if (child_process_pid == 0)
-                child_process_status = remote_process_status;
             break;
         }
         /* child signaled desire to use single socket for both stdin and stdout */
@@ -471,6 +469,8 @@ int process_child_io(libvchan_t *data_vchan,
         close(stderr_fd);
         stderr_fd = -1;
     }
+    if (child_process_pid == 0)
+        return remote_process_status;
     return child_process_status;
 }
 
diff --git a/qrexec/qrexec-client-vm.c b/qrexec/qrexec-client-vm.c
index b023045..69bfb96 100644
--- a/qrexec/qrexec-client-vm.c
+++ b/qrexec/qrexec-client-vm.c
@@ -167,8 +167,16 @@ int main(int argc, char **argv)
     }
 
     close(trigger_fd);
-    if (start_local_process)
-        waitpid(child_pid, &i, 0);
+    if (start_local_process) {
+        if (waitpid(child_pid, &i, 0) != -1) {
+            if (WIFSIGNALED(i))
+                ret = 128 + WTERMSIG(i);
+            else
+                ret = WEXITSTATUS(i);
+        } else {
+            perror("wait for local process");
+        }
+    }
 
     return ret;
 }