From: Sam Leonard Date: Wed, 14 Feb 2024 15:38:31 +0000 (+0000) Subject: vmspawn: correctly escape ',' in certain values passed to qemu X-Git-Tag: v256-rc1~787^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=018cc9eaf649eb04ebf3d353bf0a665a9deb0f1e;p=thirdparty%2Fsystemd.git vmspawn: correctly escape ',' in certain values passed to qemu --- diff --git a/src/vmspawn/meson.build b/src/vmspawn/meson.build index a7b34bdbab0..10810afa81d 100644 --- 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 index 00000000000..67e5c4cefcb --- /dev/null +++ b/src/vmspawn/test-vmspawn-util.c @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include + +#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 42c7bbfac62..9ebaa9a2236 100644 --- 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 90cb97ec005..1e9da8530cf 100644 --- 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 07ea2abd3a8..fa6c5e2b3e4 100644 --- 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();