Browse Source

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
Marek Marczykowski-Górecki 6 years ago
parent
commit
ea0cd0fdc3
3 changed files with 27 additions and 16 deletions
  1. 3 0
      doc/vm-tools/qrexec-client-vm.rst
  2. 14 14
      qrexec/qrexec-agent-data.c
  3. 10 2
      qrexec/qrexec-client-vm.c

+ 3 - 0
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

+ 14 - 14
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;
 }
 

+ 10 - 2
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;
 }