vmspawn: correctly escape ',' in certain values passed to qemu

diff --git a/src/vmspawn/meson.build b/src/vmspawn/meson.build
index a7b34bdbab0a9069e87a25504f55b1eac5db9353..10810afa81d0967e3edab1e16fd7e866318acae1 100644 (file)
--- a/src/vmspawn/meson.build
+++ b/src/vmspawn/meson.build
@@ -18,6 +18,10 @@ vmspawn_libs = [
         libshared,
 ]
 
+vmspawn_test_template = test_template + {
+        'link_with' : [vmspawn_libs],
+}
+
 executables += [
         executable_template + {
                 'name' : 'systemd-vmspawn',
@@ -26,5 +30,9 @@ executables += [
                 'sources' : files('vmspawn.c'),
                 'link_with' : vmspawn_libs,
                 'dependencies' : [libblkid]
-        }
+        },
+        vmspawn_test_template + {
+                'conditions': ['ENABLE_VMSPAWN'],
+                'sources' : files('test-vmspawn-util.c'),
+        },
 ]

diff --git a/src/vmspawn/test-vmspawn-util.c b/src/vmspawn/test-vmspawn-util.c
new file mode 100644 (file)
index 0000000..67e5c4c
--- /dev/null
+++ b/src/vmspawn/test-vmspawn-util.c
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include <stdbool.h>
+
+#include "alloc-util.h"
+#include "string-util.h"
+#include "vmspawn-util.h"
+#include "tests.h"
+
+#define _ESCAPE_QEMU_VALUE_CHECK(str, correct, varname) \
+        do {                                            \
+                _cleanup_free_ char* varname = NULL;    \
+                varname = escape_qemu_value(str);       \
+                assert(varname);                        \
+                assert_se(streq(varname, correct));     \
+        } while (0)
+
+#define ESCAPE_QEMU_VALUE_CHECK(str, correct) \
+        _ESCAPE_QEMU_VALUE_CHECK(str, correct, conf##__COUNTER__)
+
+TEST(escape_qemu_value) {
+        ESCAPE_QEMU_VALUE_CHECK("abcde", "abcde");
+        ESCAPE_QEMU_VALUE_CHECK("a,bcde", "a,,bcde");
+        ESCAPE_QEMU_VALUE_CHECK(",,,", ",,,,,,");
+        ESCAPE_QEMU_VALUE_CHECK("", "");
+}
+
+DEFINE_TEST_MAIN(LOG_INFO);

diff --git a/src/vmspawn/vmspawn-util.c b/src/vmspawn/vmspawn-util.c
index 42c7bbfac62f91e02dc573bf2a4de54c156c8030..9ebaa9a223602552de2a1be8df23490c1de73965 100644 (file)
--- a/src/vmspawn/vmspawn-util.c
+++ b/src/vmspawn/vmspawn-util.c
@@ -7,6 +7,7 @@
 #include "architecture.h"
 #include "conf-files.h"
 #include "errno-util.h"
+#include "escape.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "json.h"
@@ -433,3 +434,36 @@ int vsock_fix_child_cid(int vhost_device_fd, unsigned *machine_cid, const char *
 
         return log_debug_errno(SYNTHETIC_ERRNO(EADDRNOTAVAIL), "Failed to assign a CID to the guest vsock");
 }
+
+char* escape_qemu_value(const char *s) {
+        const char *f;
+        char *e, *t;
+        size_t n;
+
+        assert(s);
+
+        /* QEMU requires that commas in arguments be escaped by doubling up the commas.
+         * See https://www.qemu.org/docs/master/system/qemu-manpage.html#options
+         * for more information.
+         *
+         * This function performs this escaping, returning an allocated string with the escaped value, or NULL if allocation failed. */
+
+        n = strlen(s);
+
+        if (n > (SIZE_MAX - 1) / 2)
+                return NULL;
+
+        e = new(char, n*2 + 1);
+        if (!e)
+                return NULL;
+
+        for (f = s, t = e; f < s + n; f++) {
+                *t++ = *f;
+                if (*f == ',')
+                        *t++ = ',';
+        }
+
+        *t = 0;
+
+        return e;
+}

diff --git a/src/vmspawn/vmspawn-util.h b/src/vmspawn/vmspawn-util.h
index 90cb97ec005966af403a2e69b9b7dfb9c62b36ba..1e9da8530cfd9685ee014bcad8eb820da994564f 100644 (file)
--- a/src/vmspawn/vmspawn-util.h
+++ b/src/vmspawn/vmspawn-util.h
@@ -87,3 +87,5 @@ int load_ovmf_config(const char *path, OvmfConfig **ret);
 int find_ovmf_config(int search_sb, OvmfConfig **ret);
 int find_qemu_binary(char **ret_qemu_binary);
 int vsock_fix_child_cid(int vsock_fd, unsigned *machine_cid, const char *machine);
+
+char* escape_qemu_value(const char *s);

diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c
index 07ea2abd3a85038db644094a208cf77616cc1151..fa6c5e2b3e4619f119ec06f6cd76a024e63d4c8d 100644 (file)
--- a/src/vmspawn/vmspawn.c
+++ b/src/vmspawn/vmspawn.c
@@ -1207,13 +1207,18 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
         if (r < 0)
                 return log_oom();
 
-        r = strv_extendf(&cmdline, "if=pflash,format=%s,readonly=on,file=%s", ovmf_config_format(ovmf_config), ovmf_config->path);
+        _cleanup_free_ char *escaped_ovmf_config_path = escape_qemu_value(ovmf_config->path);
+        if (!escaped_ovmf_config_path)
+                return log_oom();
+
+        r = strv_extendf(&cmdline, "if=pflash,format=%s,readonly=on,file=%s", ovmf_config_format(ovmf_config), escaped_ovmf_config_path);
         if (r < 0)
                 return log_oom();
 
         _cleanup_(unlink_and_freep) char *ovmf_vars_to = NULL;
         if (ovmf_config->supports_sb) {
                 const char *ovmf_vars_from = ovmf_config->vars;
+                _cleanup_free_ char *escaped_ovmf_vars_to = NULL;
                 _cleanup_close_ int source_fd = -EBADF, target_fd = -EBADF;
 
                 r = tempfn_random_child(NULL, "vmspawn-", &ovmf_vars_to);
@@ -1245,7 +1250,11 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
                 if (r < 0)
                         return log_oom();
 
-                r = strv_extendf(&cmdline, "file=%s,if=pflash,format=%s", ovmf_vars_to, ovmf_config_format(ovmf_config));
+                escaped_ovmf_vars_to = escape_qemu_value(ovmf_vars_to);
+                if (!escaped_ovmf_vars_to)
+                        return log_oom();
+
+                r = strv_extendf(&cmdline, "file=%s,if=pflash,format=%s", escaped_ovmf_vars_to, ovmf_config_format(ovmf_config));
                 if (r < 0)
                         return log_oom();
         }
@@ -1265,13 +1274,19 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
         }
 
         if (arg_image) {
+                _cleanup_free_ char *escaped_image = NULL;
+
                 assert(!arg_directory);
 
                 r = strv_extend(&cmdline, "-drive");
                 if (r < 0)
                         return log_oom();
 
-                r = strv_extendf(&cmdline, "if=none,id=mkosi,file=%s,format=raw", arg_image);
+                escaped_image = escape_qemu_value(arg_image);
+                if (!escaped_image)
+                        log_oom();
+
+                r = strv_extendf(&cmdline, "if=none,id=mkosi,file=%s,format=raw", escaped_image);
                 if (r < 0)
                         return log_oom();
 
@@ -1283,16 +1298,21 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
         }
 
         if (arg_directory) {
-                _cleanup_free_ char *sock_path = NULL, *sock_name = NULL;
+                _cleanup_free_ char *sock_path = NULL, *sock_name = NULL, *escaped_sock_path = NULL;
+
                 r = start_virtiofsd(bus, trans_scope, arg_directory, /* uidmap= */ true, &sock_path, &sock_name);
                 if (r < 0)
                         return r;
 
+                escaped_sock_path = escape_qemu_value(sock_path);
+                if (!escaped_sock_path)
+                        log_oom();
+
                 r = strv_extend(&cmdline, "-chardev");
                 if (r < 0)
                         return log_oom();
 
-                r = strv_extendf(&cmdline, "socket,id=%1$s,path=%2$s/%1$s", sock_name, sock_path);
+                r = strv_extendf(&cmdline, "socket,id=%1$s,path=%2$s/%1$s", sock_name, escaped_sock_path);
                 if (r < 0)
                         return log_oom();
 
@@ -1314,16 +1334,20 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
                 return log_oom();
 
         FOREACH_ARRAY(mount, arg_runtime_mounts.mounts, arg_runtime_mounts.n_mounts) {
-                _cleanup_free_ char *sock_path = NULL, *sock_name = NULL, *clean_target = NULL;
+                _cleanup_free_ char *sock_path = NULL, *sock_name = NULL, *clean_target = NULL, *escaped_sock_path = NULL;
                 r = start_virtiofsd(bus, trans_scope, mount->source, /* uidmap= */ false, &sock_path, &sock_name);
                 if (r < 0)
                         return r;
 
+                escaped_sock_path = escape_qemu_value(sock_path);
+                if (!escaped_sock_path)
+                        log_oom();
+
                 r = strv_extend(&cmdline, "-chardev");
                 if (r < 0)
                         return log_oom();
 
-                r = strv_extendf(&cmdline, "socket,id=%1$s,path=%2$s/%1$s", sock_name, sock_path);
+                r = strv_extendf(&cmdline, "socket,id=%1$s,path=%2$s/%1$s", sock_name, escaped_sock_path);
                 if (r < 0)
                         return log_oom();
 
@@ -1346,7 +1370,7 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
         }
 
         if (ARCHITECTURE_SUPPORTS_SMBIOS) {
-                _cleanup_free_ char *kcl = strv_join(arg_kernel_cmdline_extra, " ");
+                _cleanup_free_ char *kcl = strv_join(arg_kernel_cmdline_extra, " "), *escaped_kcl = NULL;
                 if (!kcl)
                         return log_oom();
 
@@ -1356,11 +1380,15 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
                                 return log_oom();
                 } else {
                         if (ARCHITECTURE_SUPPORTS_SMBIOS) {
+                                escaped_kcl = escape_qemu_value(kcl);
+                                if (!escaped_kcl)
+                                        log_oom();
+
                                 r = strv_extend(&cmdline, "-smbios");
                                 if (r < 0)
                                         return log_oom();
 
-                                r = strv_extendf(&cmdline, "type=11,value=io.systemd.stub.kernel-cmdline-extra=%s", kcl);
+                                r = strv_extendf(&cmdline, "type=11,value=io.systemd.stub.kernel-cmdline-extra=%s", escaped_kcl);
                                 if (r < 0)
                                         return log_oom();
                         } else
@@ -1393,6 +1421,8 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
 
         _cleanup_free_ const char *tpm_state_tempdir = NULL;
         if (swtpm) {
+                _cleanup_free_ char *escaped_state_dir = NULL;
+
                 r = start_tpm(bus, trans_scope, swtpm, &tpm_state_tempdir);
                 if (r < 0) {
                         /* only bail if the user asked for a tpm */
@@ -1401,11 +1431,15 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
                         log_debug_errno(r, "Failed to start tpm, ignoring: %m");
                 }
 
+                escaped_state_dir = escape_qemu_value(tpm_state_tempdir);
+                if (!escaped_state_dir)
+                        log_oom();
+
                 r = strv_extend(&cmdline, "-chardev");
                 if (r < 0)
                         return log_oom();
 
-                r = strv_extendf(&cmdline, "socket,id=chrtpm,path=%s/sock", tpm_state_tempdir);
+                r = strv_extendf(&cmdline, "socket,id=chrtpm,path=%s/sock", escaped_state_dir);
                 if (r < 0)
                         return log_oom();