From 1e05472dff94675ce60b16558f1524309aaaf80c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 28 May 2024 02:55:52 +0200 Subject: [PATCH 1/3] daemon: attempt starting target vm only if not running yet Do not call admin.vm.Start method on every (autostart) call. Do it only if the target qrexec daemon is not running - or rather, when connecting to it fails. This makes the happy path of a call a bit quicker (3-5ms in some basic tests), but more importantly it avoids needlessly checking with libvirt if the VM is running - which would block if some other slow operation is running at the time (like, starting another VM). --- daemon/qrexec-daemon-common.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/daemon/qrexec-daemon-common.c b/daemon/qrexec-daemon-common.c index fa4a89ce..209870af 100644 --- a/daemon/qrexec-daemon-common.c +++ b/daemon/qrexec-daemon-common.c @@ -135,7 +135,8 @@ bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id, return false; } } - if (autostart) { + int s = disposable ? -2 : connect_unix_socket(target); + if (s == -2 && autostart) { buf = qubesd_call(target, "admin.vm.Start", "", &resp_len); if (buf == NULL) // error already printed by qubesd_call return false; @@ -150,9 +151,9 @@ bool qrexec_execute_vm(const char *target, bool autostart, int remote_domain_id, return false; } free(buf); + s = connect_unix_socket(target); } - int s = connect_unix_socket(target); - if (s == -1) + if (s < 0) goto kill; int data_domain; int data_port; @@ -199,6 +200,11 @@ int connect_unix_socket_by_id(unsigned int domid) return connect_unix_socket(id_str); } +/* Returns: + * - FD on success, + * - -2 target qrexec daemon is not running, + * - -1 on other errors + */ int connect_unix_socket(const char *domname) { int s, len, res; @@ -217,12 +223,15 @@ int connect_unix_socket(const char *domname) if (res >= (int)sizeof(remote.sun_path)) { LOG(ERROR, "%s/qrexec.%s is too long for AF_UNIX socket path", socket_dir, domname); + close(s); return -1; } len = (size_t)res + 1 + offsetof(struct sockaddr_un, sun_path); if (connect(s, (struct sockaddr *) &remote, len) == -1) { + int saved_errno = errno; LOG(ERROR, "connect %s", remote.sun_path); - return -1; + close(s); + return (saved_errno == ENOENT || saved_errno == ECONNREFUSED) ? -2 : -1; } if (handle_daemon_handshake(s) < 0) return -1; From 6760cfa978e864aae6df938b518f8ff6d05caa1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 28 May 2024 03:07:25 +0200 Subject: [PATCH 2/3] Make stderr line-buffered for logging qrexec_logv uses several separate fprintf() calls for a single message. Since stderr is unbuffered by default, this sometimes leads to splitting one log message with another. Avoid the issue by turning stderr into line-buffered mode. Documentation for setlinebuf() (or more precisely - for underlying setvbuf()) says it can be called only after opening the stream but before any other operation. Since setup_logging() is called as the first thing in main(), it should be okay. --- libqrexec/log.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libqrexec/log.c b/libqrexec/log.c index f3f2283b..f569d3f9 100644 --- a/libqrexec/log.c +++ b/libqrexec/log.c @@ -88,5 +88,10 @@ void setup_logging(const char *program_name) { errx(125, "File descriptor %d is closed, cannot continue", i); } + /* qrexec_logv() uses several separate fprintf calls, print them at once */ + errno = 0; + if (setvbuf(stderr, NULL, _IOLBF, 0)) + warn("Failed to enable line buffering on stderr"); + qrexec_program_name = program_name; } From c87b9e7e1e899cb0aa2c57504f0dfb1a23c74010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 28 May 2024 23:17:11 +0200 Subject: [PATCH 3/3] Remove redundant log line On successful service call there is later a message from handle_process_common anyway. --- libqrexec/exec.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libqrexec/exec.c b/libqrexec/exec.c index ab3f9f96..0dec6844 100644 --- a/libqrexec/exec.c +++ b/libqrexec/exec.c @@ -624,7 +624,6 @@ static int qubes_tcp_connect(const char *host, const char *port) close(sockfd); } else { rc = sockfd; - LOG(DEBUG, "Connection succeeded"); } freeaddrs: freeaddrinfo(addrs);