Do not close FDs 0, 1, or 2

If they are closed, another file descriptor could be created with these
numbers, and so standard library functions that use them might write to
an unwanted place.  dup2() a file descriptor to /dev/null over them
instead.

Also statically initialize trigger_fd to -1, which is the conventional
value for an invalid file descriptor.

This requires care to avoid closing the file descriptor to /dev/null in
fix_fds(), which took over an hour to debug.

agent/qrexec-agent.c

@@ -71,10 +71,12 @@ static libvchan_t *ctrl_vchan;
 
 static pid_t wait_for_session_pid = -1;
 
-static int trigger_fd;
+static int trigger_fd = -1;
 
 static int terminate_requested;
 
+static int null_fd = -1;
+
 static int meminfo_write_started = 0;
 
 static uid_t myuid;
@@ -156,9 +158,14 @@ static void close_std(void)
 {
     /* close std*, so when child process closes them, qrexec-agent will receive EOF */
     /* this is the main purpose of this reimplementation of /bin/su... */
-    close(0);
-    close(1);
-    close(2);
+    for (int i = 0; i < 3; ++i) {
+        int j;
+        do {
+            j = dup2(null_fd, i);
+        } while (j == -1 && (errno == EINTR || errno == EBUSY));
+        if (j != i)
+            abort();
+    }
 }
 
 static int really_wait(pid_t child)
@@ -921,6 +928,7 @@ static _Noreturn void usage(const char *argv0)
 int main(int argc, char **argv)
 {
     sigset_t selectmask;
+    null_fd = qrexec_open_dev_null();
 
     setup_logging("qrexec-agent");
 

libqrexec/exec.c

@@ -36,6 +36,7 @@
 #include <netdb.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <err.h>
 #include "qrexec.h"
 #include "libqrexec-utils.h"
 #include "private.h"
@@ -47,6 +48,26 @@ void register_exec_func(do_exec_t *func) {
     exec_func = func;
 }
 
+static int null_fd = -1;
+int qrexec_open_dev_null(void) {
+    if (null_fd != -1)
+        abort();
+    null_fd = open("/dev/null", O_RDWR|O_CLOEXEC|O_NOCTTY);
+    if (null_fd < 3) {
+        int problem = errno;
+        if (null_fd == 2 || (fcntl(2, F_GETFD) == -1 && errno == EBADF)) {
+            return 1;
+        }
+        /* stderr is open */
+        if (null_fd == -1) {
+            errno = problem;
+            err(1, "open /dev/null");
+        }
+        errx(1, "File descriptor %d is closed, cannot continue", null_fd);
+    }
+    return null_fd;
+}
+
 void exec_qubes_rpc_if_requested(const char *prog, char *const envp[]) {
     /* avoid calling qubes-rpc-multiplexer through shell */
     if (strncmp(prog, RPC_REQUEST_COMMAND, RPC_REQUEST_COMMAND_LEN) == 0) {
@@ -99,7 +120,16 @@ void fix_fds(int fdin, int fdout, int fderr)
         abort();
     }
 #ifdef SYS_close_range
-    int close_range_res = syscall(SYS_close_range, 3, ~0U, 0);
+    int close_range_res;
+    if (null_fd == -1) {
+        close_range_res = syscall(SYS_close_range, 3, ~0U, 0);
+    } else {
+        assert(null_fd >= 3);
+        close_range_res = syscall(SYS_close_range, null_fd + 1, ~0U, 0);
+        if (null_fd > 3 && close_range_res == 0) {
+            close_range_res = syscall(SYS_close_range, 3, (unsigned)(null_fd - 1), 0);
+        }
+    }
     if (close_range_res == 0)
         return;
     assert(close_range_res == -1);
@@ -108,8 +138,10 @@ void fix_fds(int fdin, int fdout, int fderr)
         abort();
     }
 #endif
-    for (int i = 3; i < 256; ++i)
-        close(i);
+    for (int i = 3; i < 256; ++i) {
+        if (i != null_fd)
+            close(i);
+    }
 }
 
 static int do_fork_exec(const char *user,

libqrexec/libqrexec-utils.h

@@ -162,6 +162,11 @@ void buffer_append(struct buffer *b, const char *data, int len);
 void buffer_remove(struct buffer *b, int len);
 int buffer_len(struct buffer *b);
 void *buffer_data(struct buffer *b);
+/* Open /dev/null and keep it from being closed before the exec func is called.
+ * Returns the newly opened FD. Crashes if called twice. The FD is marked CLOEXEC,
+ * so it doesn't need to be closed before execve(). */
+__attribute__((visibility("default")))
+int qrexec_open_dev_null(void);
 
 int flush_client_data(int fd, struct buffer *buffer);
 int write_stdin(int fd, const char *data, int len, struct buffer *buffer);