Avoid using alarm(2) for timeouts

libvchan has much better ways of dealing with timeouts now, so use them
instead of calling an async signal unsafe function from a signal
handler.  Both qrexec-agent-data.c and qrexec-daemon-common.c
reimplemented timeouts, so write common code in libqrexec for both to
use.

Author: DemiMarie

agent/qrexec-agent-data.c

@@ -150,69 +150,6 @@ static int handle_just_exec(struct qrexec_parsed_command *cmd)
     return 0;
 }
 
-static const long BILLION_NANOSECONDS = 1000000000L;
-
-static int wait_for_vchan_connection_with_timeout(
-        libvchan_t *conn, int wait_fd, bool is_server, time_t timeout) {
-    struct timespec end_tp, now_tp, timeout_tp;
-
-    if (timeout && clock_gettime(CLOCK_MONOTONIC, &end_tp)) {
-        PERROR("clock_gettime");
-        return -1;
-    }
-    assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS);
-    end_tp.tv_sec += timeout;
-    while (true) {
-        bool did_timeout = true;
-        struct pollfd fds = { .fd = wait_fd, .events = POLLIN | POLLHUP, .revents = 0 };
-
-        /* calculate how much time left until connection timeout expire */
-        if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) {
-            PERROR("clock_gettime");
-            return -1;
-        }
-        assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS);
-        if (now_tp.tv_sec <= end_tp.tv_sec) {
-            timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec;
-            timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec;
-            if (timeout_tp.tv_nsec < 0) {
-                timeout_tp.tv_nsec += BILLION_NANOSECONDS;
-                timeout_tp.tv_sec--;
-            }
-            did_timeout = timeout_tp.tv_sec < 0;
-        }
-        switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) {
-            case -1:
-                if (errno == EINTR)
-                    break;
-                LOG(ERROR, "vchan connection error");
-                return -1;
-            case 0:
-                LOG(ERROR, "vchan connection timeout");
-                return -1;
-            case 1:
-                break;
-            default:
-                abort();
-        }
-        if (fds.revents & POLLIN) {
-            if (is_server) {
-                libvchan_wait(conn);
-                return 0;
-            } else {
-                int connect_ret = libvchan_client_init_async_finish(conn, true);
-
-                if (connect_ret < 0) {
-                    LOG(ERROR, "vchan connection error");
-                    return -1;
-                } else if (connect_ret == 0) {
-                    return 0;
-                }
-            }
-        }
-    }
-}
-
 
 /* Behaviour depends on type parameter:
  *  MSG_JUST_EXEC - connect to vchan server, fork+exec process given by cmdline
@@ -253,7 +190,8 @@ static int handle_new_process_common(
         LOG(ERROR, "Data vchan connection failed");
         exit(1);
     }
-    if (wait_for_vchan_connection_with_timeout(data_vchan, wait_fd, false, connection_timeout) < 0) {
+    if (qubes_wait_for_vchan_connection_with_timeout(
+                data_vchan, wait_fd, false, connection_timeout) < 0) {
         LOG(ERROR, "Data vchan connection failed");
         exit(1);
     }
@@ -381,7 +319,7 @@ int handle_data_client(
         LOG(ERROR, "Data vchan connection failed");
         exit(1);
     }
-    if (wait_for_vchan_connection_with_timeout(
+    if (qubes_wait_for_vchan_connection_with_timeout(
             data_vchan, libvchan_fd_for_select(data_vchan), true, connection_timeout) < 0) {
         LOG(ERROR, "Data vchan connection failed");
         exit(1);

daemon/qrexec-client.c

@@ -143,57 +143,6 @@ static void parse_connect(char *str, char **request_id,
     exit(1);
 }
 
-static const long BILLION_NANOSECONDS = 1000000000L;
-
-static void wait_for_vchan_client_with_timeout(libvchan_t *conn, time_t timeout) {
-    struct timespec end_tp, now_tp, timeout_tp;
-
-    if (timeout && clock_gettime(CLOCK_MONOTONIC, &end_tp)) {
-        PERROR("clock_gettime");
-        exit(1);
-    }
-    assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS);
-    end_tp.tv_sec += timeout;
-    int const fd = libvchan_fd_for_select(conn);
-    while (conn && libvchan_is_open(conn) == VCHAN_WAITING) {
-        if (timeout) {
-            bool did_timeout = true;
-            struct pollfd fds = { .fd = fd, .events = POLLIN | POLLHUP, .revents = 0 };
-
-            /* calculate how much time left until connection timeout expire */
-            if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) {
-                PERROR("clock_gettime");
-                exit(1);
-            }
-            assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS);
-            if (now_tp.tv_sec <= end_tp.tv_sec) {
-                timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec;
-                timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec;
-                if (timeout_tp.tv_nsec < 0) {
-                    timeout_tp.tv_nsec += BILLION_NANOSECONDS;
-                    timeout_tp.tv_sec--;
-                }
-                did_timeout = timeout_tp.tv_sec < 0;
-            }
-            switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) {
-            case -1:
-                if (errno == EINTR)
-                    break;
-                LOG(ERROR, "vchan connection error");
-                exit(1);
-            case 0:
-                LOG(ERROR, "vchan connection timeout");
-                exit(1);
-            case 1:
-                break;
-            default:
-                abort();
-            }
-        }
-        libvchan_wait(conn);
-    }
-}
-
 static size_t compute_service_length(const char *const remote_cmdline, const char *const prog_name) {
     const size_t service_length = strlen(remote_cmdline) + 1;
     if (service_length < 2 || service_length > MAX_QREXEC_CMD_LEN) {
@@ -365,9 +314,16 @@ int main(int argc, char **argv)
                     VCHAN_BUFFER_SIZE, VCHAN_BUFFER_SIZE);
             if (!data_vchan) {
                 LOG(ERROR, "Failed to start data vchan server");
-                exit(1);
+                rc = QREXEC_EXIT_PROBLEM;
+                goto cleanup;
+            }
+            int fd = libvchan_fd_for_select(data_vchan);
+            if (qubes_wait_for_vchan_connection_with_timeout(
+                        data_vchan, fd, true, connection_timeout) < 0) {
+                LOG(ERROR, "qrexec connection timeout");
+                rc = QREXEC_EXIT_PROBLEM;
+                goto cleanup;
             }
-            wait_for_vchan_client_with_timeout(data_vchan, connection_timeout);
             struct handshake_params params = {
                 .data_vchan = data_vchan,
                 .stdin_buffer = &stdin_buffer,

daemon/qrexec-daemon-common.c

@@ -330,12 +330,6 @@ static int select_loop(struct handshake_params *params)
     return (params->exit_with_code ? exit_code : 0);
 }
 
-static void sigalrm_handler(int x __attribute__((__unused__)))
-{
-    LOG(ERROR, "vchan connection timeout");
-    _exit(1);
-}
-
 int run_qrexec_to_dom0(const struct service_params *svc_params,
                         int src_domain_id,
                         const char *src_domain_name,
@@ -374,15 +368,18 @@ int run_qrexec_to_dom0(const struct service_params *svc_params,
     } else {
         prepare_ret = prepare_local_fds(command, &stdin_buffer);
     }
-    void (*old_handler)(int);
-
-    /* libvchan_client_init is blocking and does not support connection
-     * timeout, so use alarm(2) for that... */
-    old_handler = signal(SIGALRM, sigalrm_handler);
-    alarm(connection_timeout);
-    data_vchan = libvchan_client_init(data_domain, data_port);
-    alarm(0);
-    signal(SIGALRM, old_handler);
+    int wait_fd;
+    data_vchan = libvchan_client_init_async(data_domain, data_port, &wait_fd);
+    if (!data_vchan) {
+        LOG(ERROR, "Cannot create data vchan connection");
+        return QREXEC_EXIT_PROBLEM;
+    }
+    if (qubes_wait_for_vchan_connection_with_timeout(
+                data_vchan, wait_fd, false, connection_timeout) < 0) {
+        LOG(ERROR, "qrexec connection timeout");
+        return QREXEC_EXIT_PROBLEM;
+    }
+
     struct handshake_params params = {
         .data_vchan = data_vchan,
         .stdin_buffer = &stdin_buffer,

fuzz/Makefile

@@ -16,7 +16,7 @@ CXXFLAGS += -O1 -fno-omit-frame-pointer -gline-tables-only \
 		-fsanitize-address-use-after-scope -fsanitize=fuzzer
 endif
 
-_LIBQREXEC_OBJS = remote.o write-stdin.o ioall.o txrx-vchan.o buffer.o replace.o exec.o log.o unix-server.o toml.o process_io.o
+_LIBQREXEC_OBJS = remote.o write-stdin.o ioall.o txrx-vchan.o buffer.o replace.o exec.o log.o unix-server.o toml.o process_io.o vchan_timeout.o
 LIBQREXEC_OBJS = $(patsubst %.o,libqrexec-%.o,$(_LIBQREXEC_OBJS))
 
 FUZZERS = qubesrpc_parse_fuzzer qrexec_remote_fuzzer qrexec_daemon_fuzzer

fuzz/fuzz.c

@@ -140,7 +140,18 @@ ssize_t fuzz_write(int fd, const void *buf, size_t count) {
     return count;
 }
 
+typedef int EVTCHN;
 fuzz_file_t *fuzz_libvchan_client_init(int domain, int port) {
     /* not implemented yet */
     abort();
 }
+
+fuzz_file_t *fuzz_libvchan_client_init_async(int domain, int port, EVTCHN *watch_fd) {
+    /* not implemented yet */
+    abort();
+}
+
+int fuzz_libvchan_client_init_async_finish(fuzz_file_t *ctrl, bool blocking) {
+    /* not implemented yet */
+    abort();
+}

fuzz/fuzz.h

@@ -31,6 +31,8 @@ int fuzz_libvchan_is_open(fuzz_file_t *file);
 int fuzz_libvchan_data_ready(fuzz_file_t *file);
 int fuzz_libvchan_buffer_space(fuzz_file_t *file);
 fuzz_file_t *fuzz_libvchan_client_init(int domain, int port);
+fuzz_file_t *fuzz_libvchan_client_init_async(int domain, int port, int *fd);
+int fuzz_libvchan_client_init_async_finish(fuzz_file_t *file, bool blocking);
 
 ssize_t fuzz_read(int fd, void *buf, size_t count);
 ssize_t fuzz_write(int fd, const void *buf, size_t count);

fuzz/mock-fuzz.h

@@ -16,6 +16,8 @@
 #define libvchan_data_ready fuzz_libvchan_data_ready
 #define libvchan_buffer_space fuzz_libvchan_buffer_space
 #define libvchan_client_init fuzz_libvchan_client_init
+#define libvchan_client_init_async fuzz_libvchan_client_init_async
+#define libvchan_client_init_async_finish fuzz_libvchan_client_init_async_finish
 
 #define read fuzz_read
 #define write fuzz_write

libqrexec/Makefile

@@ -21,7 +21,7 @@ endif
 
 
 all: libqrexec-utils.so
-libqrexec-utils.so.$(SO_VER): unix-server.o ioall.o buffer.o exec.o txrx-vchan.o write-stdin.o replace.o remote.o process_io.o log.o toml.o
+libqrexec-utils.so.$(SO_VER): unix-server.o ioall.o buffer.o exec.o txrx-vchan.o write-stdin.o replace.o remote.o process_io.o log.o toml.o vchan_timeout.o
 	$(CC) $(LDFLAGS) -Wl,-soname,$@ -o $@ $^ $(VCHANLIBS)
 
 libqrexec-utils.so: libqrexec-utils.so.$(SO_VER)

libqrexec/libqrexec-utils.h

@@ -32,6 +32,7 @@
 #include <poll.h>
 #include <sys/socket.h>
 
+#include <libvchan.h>
 #include <qrexec.h>
 
 #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
@@ -347,4 +348,15 @@ void *qubes_read_all_to_malloc(int fd, size_t initial_buffer_size, size_t max_by
  */
 bool qubes_sendmsg_all(struct msghdr *msg, int sock);
 
+/**
+ * Wait for a vchan connection with a timeout.
+ *
+ * @param conn the vchan
+ * @param wait_fd The FD set by libvchan_client_init_async() for clients,
+ *                or the FD returned by libvchan_fd_for_select() for servers.
+ * @param is_server Is this a server or a client vchan?
+ * @param timeout The timeout to use.
+ */
+int qubes_wait_for_vchan_connection_with_timeout(
+        libvchan_t *conn, int wait_fd, bool is_server, time_t timeout);
 #endif /* LIBQREXEC_UTILS_H */

libqrexec/vchan_timeout.c

@@ -0,0 +1,67 @@
+#include <libqrexec-utils.h>
+#include <time.h>
+#include <assert.h>
+#include <stdlib.h>
+
+static const long BILLION_NANOSECONDS = 1000000000L;
+
+int qubes_wait_for_vchan_connection_with_timeout(
+        libvchan_t *conn, int wait_fd, bool is_server, time_t timeout) {
+    struct timespec end_tp, now_tp, timeout_tp;
+
+    if (clock_gettime(CLOCK_MONOTONIC, &end_tp)) {
+        PERROR("clock_gettime");
+        return -1;
+    }
+    assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS);
+    end_tp.tv_sec += timeout;
+    for (;;) {
+        bool did_timeout = true;
+        struct pollfd fds = { .fd = wait_fd, .events = POLLIN | POLLHUP, .revents = 0 };
+
+        /* calculate how much time left until connection timeout expire */
+        if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) {
+            PERROR("clock_gettime");
+            return -1;
+        }
+        assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS);
+        if (now_tp.tv_sec <= end_tp.tv_sec) {
+            timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec;
+            timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec;
+            if (timeout_tp.tv_nsec < 0) {
+                timeout_tp.tv_nsec += BILLION_NANOSECONDS;
+                timeout_tp.tv_sec--;
+            }
+            did_timeout = timeout_tp.tv_sec < 0;
+        }
+        switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) {
+            case -1:
+                if (errno == EINTR)
+                    break;
+                LOG(ERROR, "vchan connection error");
+                return -1;
+            case 0:
+                LOG(ERROR, "vchan connection timeout");
+                return -1;
+            case 1:
+                break;
+            default:
+                abort();
+        }
+        if (fds.revents & POLLIN) {
+            if (is_server) {
+                libvchan_wait(conn);
+                return 0;
+            } else {
+                int connect_ret = libvchan_client_init_async_finish(conn, true);
+
+                if (connect_ret < 0) {
+                    LOG(ERROR, "vchan connection error");
+                    return -1;
+                } else if (connect_ret == 0) {
+                    return 0;
+                }
+            }
+        }
+    }
+}