Avoid leaking vchans

Leaking vchans will cause Xenstore entries to also leak, which is bad. If it happens in a VM it will cause quota problems, while if it happens in dom0 it will just slow down Xenstore.
QubesOS · Apr 28, 2024 · 76790cb · 76790cb
1 parent e06514b
commit 76790cb
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 14 deletions.
diff --git a/daemon/qrexec-client.c b/daemon/qrexec-client.c
@@ -292,6 +292,7 @@ int main(int argc, char **argv)
             if (qubes_wait_for_vchan_connection_with_timeout(
                         data_vchan, fd, true, connection_timeout) < 0) {
                 LOG(ERROR, "qrexec connection timeout");
+                libvchan_close(data_vchan);
                 rc = QREXEC_EXIT_PROBLEM;
                 goto cleanup;
             }

diff --git a/daemon/qrexec-daemon-common.c b/daemon/qrexec-daemon-common.c
@@ -479,6 +479,7 @@ int run_qrexec_to_dom0(const struct service_params *svc_params,
     if (qubes_wait_for_vchan_connection_with_timeout(
                 data_vchan, wait_fd, false, connection_timeout) < 0) {
         LOG(ERROR, "qrexec connection timeout");
+        libvchan_close(data_vchan);
         return QREXEC_EXIT_PROBLEM;
     }
 
@@ -496,23 +497,25 @@ int run_qrexec_to_dom0(const struct service_params *svc_params,
 
 int handshake_and_go(struct handshake_params *params)
 {
+    int rc = QREXEC_EXIT_PROBLEM;
     if (params->data_vchan == NULL || !libvchan_is_open(params->data_vchan)) {
         LOG(ERROR, "Failed to open data vchan connection");
-        return QREXEC_EXIT_PROBLEM;
+        goto cleanup;
     }
-    int rc;
     int data_protocol_version = handle_agent_handshake(params->data_vchan,
                                                        params->remote_send_first);
-    if (data_protocol_version < 0) {
-        rc = QREXEC_EXIT_PROBLEM;
-    } else if (params->prepare_ret != 0) {
-        rc = params->prepare_ret == -1 ? QREXEC_EXIT_SERVICE_NOT_FOUND : QREXEC_EXIT_PROBLEM;
-        handle_failed_exec(params->data_vchan, params->remote_send_first, rc);
-    } else {
-        params->data_protocol_version = data_protocol_version;
-        rc = select_loop(params);
+    if (data_protocol_version >= 0) {
+        if (params->prepare_ret != 0) {
+            rc = params->prepare_ret == -1 ? QREXEC_EXIT_SERVICE_NOT_FOUND : QREXEC_EXIT_PROBLEM;
+            handle_failed_exec(params->data_vchan, params->remote_send_first, rc);
+        } else {
+            params->data_protocol_version = data_protocol_version;
+            rc = select_loop(params);
+        }
     }
-    libvchan_close(params->data_vchan);
+cleanup:
+    if (params->data_vchan != NULL)
+        libvchan_close(params->data_vchan);
     params->data_vchan = NULL;
     return rc;
 }
diff --git a/libqrexec/exec.c b/libqrexec/exec.c
@@ -120,12 +120,14 @@ static int do_fork_exec(const char *user,
             (stderr_fd && socketpair(AF_UNIX, SOCK_STREAM, 0, errpipe)) ||
             socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, statuspipe)) {
         PERROR("socketpair");
-        exit(1);
+        /* FD leaks do not matter, we exit soon anyway */
+        return -2;
     }
     switch (*pid = fork()) {
         case -1:
             PERROR("fork");
-            exit(-1);
+            /* ditto */
+            return -2;
         case 0: {
             int status;
             if (signal(SIGPIPE, SIG_DFL) == SIG_ERR)

diff --git a/libqrexec/libqrexec-utils.h b/libqrexec/libqrexec-utils.h
@@ -182,7 +182,7 @@ int execute_parsed_qubes_rpc_command(
  */
 __attribute__((visibility("default")))
 int find_qrexec_service(
-        const struct qrexec_parsed_command *cmd,
+        struct qrexec_parsed_command *cmd,
         int *socket_fd, struct buffer *stdin_buffer);
 
 /** Suggested buffer size for the path buffer of find_qrexec_service. */