SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

vmspawn: minor fixes (#42634)

Browse Source
This commit is contained in:
Luca Boccassi
2026-06-19 19:39:45 +01:00
committed by GitHub
parent 416a715b49 ba1203562e
commit 2bc35aab8d
6 changed files with 172 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/vmspawn/meson.build
+5 -1
View File
@@ -7,7 +7,6 @@ endif
vmspawn_sources = files(
'vmspawn.c',
'vmspawn-bind-volume.c',
'vmspawn-qemu-config.c',
'vmspawn-qmp.c',
'vmspawn-varlink.c',
'vmspawn-settings.c',
@@ -16,6 +15,7 @@ vmspawn_sources = files(
)
vmspawn_extract_sources = files(
'vmspawn-util.c',
'vmspawn-qemu-config.c',
)
executables += [
@@ -29,4 +29,8 @@ executables += [
'sources' : files('test-vmspawn-util.c'),
'objects' : ['systemd-vmspawn'],
},
test_template + {
'sources' : files('test-vmspawn-qemu-config.c'),
'objects' : ['systemd-vmspawn'],
},
]
src/vmspawn/test-vmspawn-qemu-config.c
+120
View File
@@ -0,0 +1,120 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#include "alloc-util.h"
#include "memstream-util.h"
#include "tests.h"
#include "vmspawn-qemu-config.h"
/* Render a single "key = value" pair through qemu_config_key() and return both the function's return code
* and (on success, when ret is set) the rendered bytes. */
static int render_key(char **ret, const char *key, const char *value) {
_cleanup_(memstream_done) MemStream m = {};
FILE *f;
int r;
assert_se(f = memstream_init(&m));
r = qemu_config_key(f, key, value);
if (r < 0)
return r;
if (ret)
assert_se(memstream_finalize(&m, ret, NULL) >= 0);
return r;
}
static int render_section(char **ret, const char *type, const char *id) {
_cleanup_(memstream_done) MemStream m = {};
FILE *f;
int r;
assert_se(f = memstream_init(&m));
r = qemu_config_section(f, type, id);
if (r < 0)
return r;
if (ret)
assert_se(memstream_finalize(&m, ret, NULL) >= 0);
return r;
}
TEST(qemu_config_key_valid) {
_cleanup_free_ char *out = NULL;
ASSERT_OK(render_key(&out, "readonly", "on"));
ASSERT_STREQ(out, " readonly = \"on\"\n");
/* Keys may carry the full identifier charset. */
out = mfree(out);
ASSERT_OK(render_key(&out, "confidential-guest-support", "snp0"));
ASSERT_STREQ(out, " confidential-guest-support = \"snp0\"\n");
}
TEST(qemu_config_value_permits_path_bytes) {
_cleanup_free_ char *out = NULL;
/* Legitimate path bytes, including backslash, must pass through a quoted value verbatim. */
ASSERT_OK(render_key(&out, "file", "/usr/share/edk2/ovmf/OVMF_CODE.fd"));
ASSERT_STREQ(out, " file = \"/usr/share/edk2/ovmf/OVMF_CODE.fd\"\n");
out = mfree(out);
ASSERT_OK(render_key(&out, "file", "/weird/but\\legal/path"));
ASSERT_STREQ(out, " file = \"/weird/but\\legal/path\"\n");
/* '=', spaces, brackets etc. are all fine inside a quoted value. */
out = mfree(out);
ASSERT_OK(render_key(&out, "cpus", "1,sockets=1"));
ASSERT_STREQ(out, " cpus = \"1,sockets=1\"\n");
}
TEST(qemu_config_value_rejects_quote_and_newline) {
/* These two bytes are the only ones that can break out of the quoted token; both must be refused. */
ASSERT_ERROR(render_key(NULL, "file", "ab\"cd"), EINVAL);
ASSERT_ERROR(render_key(NULL, "file", "ab\ncd"), EINVAL);
}
TEST(qemu_config_key_name_rejects_structure) {
/* Key names are emitted unquoted, so they must be plain identifiers. */
ASSERT_ERROR(render_key(NULL, "fo=o", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "fo\no", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "fo\"o", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "fo o", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "foo]", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "/foo", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "", "v"), EINVAL);
}
TEST(qemu_config_section_valid) {
_cleanup_free_ char *out = NULL;
ASSERT_OK(render_section(&out, "drive", "ovmf-code"));
ASSERT_STREQ(out, "\n[drive \"ovmf-code\"]\n");
out = mfree(out);
ASSERT_OK(render_section(&out, "smp-opts", NULL));
ASSERT_STREQ(out, "\n[smp-opts]\n");
}
TEST(qemu_config_section_type_rejects_structure) {
/* The section type is emitted unquoted as "[type]" — a ']' or newline here would let a caller
* close the header early or open a new section. */
ASSERT_ERROR(render_section(NULL, "drive]", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "dr\nive", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "dr ive", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "dr\"ive", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "", "id"), EINVAL);
}
TEST(qemu_config_section_id_rejects_structure) {
/* The id is quoted, but ']' and backslash are still hardened against future runtime data. */
ASSERT_ERROR(render_section(NULL, "drive", "i\"d"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", "i\nd"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", "i]d"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", "i\\d"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", ""), EINVAL);
}
DEFINE_TEST_MAIN(LOG_DEBUG);
src/vmspawn/vmspawn-qemu-config.c
+17 -20
View File
@@ -5,22 +5,19 @@
#include "alloc-util.h"
#include "errno-util.h"
#include "log.h"
#include "string-util.h"
#include "vmspawn-qemu-config.h"
static bool qemu_config_type_valid(const char *type) {
return !strchr(type, '\n');
}
static bool qemu_config_id_valid(const char *id) {
return !strpbrk(id, "\"\n");
}
static bool qemu_config_key_name_valid(const char *key) {
return !strpbrk(key, "=\n");
/* Enforce QEMU's identifier grammar, so runtime data can never inject config structure. */
static bool qemu_config_identifier_valid(const char *s) {
return !isempty(s) && in_charset(s, ALPHANUMERICAL ".-_");
}
/* Values are written quoted ('key = "%s"') and QEMU reads them literally between the quotes (no escapes),
* only '"' and newline can break out. Nevertheless run the value through string_is_safe(), which is
* stricter than QEMU's own parser. Relax if a valid use case occurs. */
static bool qemu_config_value_valid(const char *value) {
return !strpbrk(value, "\"\n");
return string_is_safe(value, STRING_ALLOW_EMPTY | STRING_ALLOW_BACKSLASHES | STRING_ALLOW_GLOBS);
}
int qemu_config_key(FILE *f, const char *key, const char *value) {
@@ -28,13 +25,13 @@ int qemu_config_key(FILE *f, const char *key, const char *value) {
assert(key);
assert(value);
if (!qemu_config_key_name_valid(key))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config key '%s' contains '=' or newline.", key);
if (!qemu_config_identifier_valid(key))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config key '%s' is not a valid identifier.", key);
if (!qemu_config_value_valid(value))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config value '%s' contains quote or newline.", value);
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config value '%s' contains unsafe characters.", value);
if (fprintf(f, " %s = \"%s\"\n", key, value) < 0)
return -errno_or_else(EIO);
return errno_or_else(EIO);
return 0;
}
@@ -64,12 +61,12 @@ int qemu_config_section_impl(FILE *f, const char *type, const char *id, ...) {
assert(f);
assert(type);
if (!qemu_config_type_valid(type))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section type '%s' contains newline.", type);
if (!qemu_config_identifier_valid(type))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section type '%s' is not a valid identifier.", type);
if (id) {
if (!qemu_config_id_valid(id))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section id '%s' contains quote or newline.", id);
if (!qemu_config_identifier_valid(id))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section id '%s' is not a valid identifier.", id);
fprintf(f, "\n[%s \"%s\"]\n", type, id);
} else
fprintf(f, "\n[%s]\n", type);
@@ -91,7 +88,7 @@ int qemu_config_section_impl(FILE *f, const char *type, const char *id, ...) {
va_end(ap);
if (ferror(f))
return -errno_or_else(EIO);
return errno_or_else(EIO);
return 0;
}
src/vmspawn/vmspawn-util.c
+4 -4
View File
@@ -32,11 +32,11 @@ static const char* const architecture_to_qemu_table[_ARCHITECTURE_MAX] = {
[ARCHITECTURE_X86_64] = "x86_64", /* differs from our name */
[ARCHITECTURE_X86] = "i386", /* differs from our name */
[ARCHITECTURE_LOONGARCH64] = "loongarch64",
[ARCHITECTURE_MIPS64_LE] = "mips", /* differs from our name */
[ARCHITECTURE_MIPS_LE] = "mips", /* differs from our name */
[ARCHITECTURE_MIPS64_LE] = "mips64el", /* differs from our name */
[ARCHITECTURE_MIPS_LE] = "mipsel", /* differs from our name */
[ARCHITECTURE_PARISC] = "hppa", /* differs from our name */
[ARCHITECTURE_PPC64_LE] = "ppc", /* differs from our name */
[ARCHITECTURE_PPC64] = "ppc", /* differs from our name */
[ARCHITECTURE_PPC64_LE] = "ppc64", /* differs from our name; qemu-system-ppc64 runs LE guests too */
[ARCHITECTURE_PPC64] = "ppc64", /* differs from our name */
[ARCHITECTURE_PPC] = "ppc",
[ARCHITECTURE_RISCV32] = "riscv32",
[ARCHITECTURE_RISCV64] = "riscv64",
src/vmspawn/vmspawn-varlink.c
+3 -2
View File
@@ -577,10 +577,11 @@ VmspawnVarlinkContext* vmspawn_varlink_context_free(VmspawnVarlinkContext *ctx)
if (!ctx)
return NULL;
sd_varlink_server_unref(ctx->varlink_server);
vmspawn_qmp_bridge_free(ctx->bridge);
ctx->varlink_server = sd_varlink_server_unref(ctx->varlink_server);
drain_event_subscribers(&ctx->subscribed);
ctx->bridge = vmspawn_qmp_bridge_free(ctx->bridge);
return mfree(ctx);
}
src/vmspawn/vmspawn.c
+23 -11
View File
@@ -3172,8 +3172,8 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
if (r < 0)
return log_error_errno(r, "Failed to create ephemeral snapshot of '%s': %m", arg_directory);
arg_directory = strdup(snapshot_directory);
if (!arg_directory)
r = free_and_strdup(&arg_directory, snapshot_directory);
if (r < 0)
return log_oom();
}
@@ -3305,6 +3305,11 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
if (existing) {
_cleanup_free_ char *combined = NULL;
if (existing->size >= INT_MAX)
return log_error_errno(SYNTHETIC_ERRNO(EFBIG),
"Existing fstab.extra credential is too large (%zu bytes).",
existing->size);
if (existing->size > 0 && existing->data[existing->size - 1] != '\n')
r = asprintf(&combined, "%.*s\n%s", (int) existing->size, existing->data, fstab_extra);
else
@@ -3378,15 +3383,15 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
return log_error_errno(r, "Failed to start tpm: %m");
log_debug_errno(r, "Failed to start tpm, ignoring: %m");
} else {
_cleanup_(sd_event_source_unrefp) sd_event_source *source = NULL;
r = event_add_child_pidref(event, &source, &child, WEXITED, on_child_exit, /* userdata= */ NULL);
if (r < 0)
return r;
pidref_done(&child);
children[n_children++] = TAKE_PTR(source);
}
_cleanup_(sd_event_source_unrefp) sd_event_source *source = NULL;
r = event_add_child_pidref(event, &source, &child, WEXITED, on_child_exit, /* userdata= */ NULL);
if (r < 0)
return r;
pidref_done(&child);
children[n_children++] = TAKE_PTR(source);
}
if (tpm_socket_address) {
@@ -4022,7 +4027,14 @@ static int determine_kernel(void) {
int r;
if (!arg_linux && arg_directory) {
/* A kernel is required for directory type images so attempt to find one under /boot and /efi */
/* A kernel is required for directory type images so attempt to find one under /boot and /efi.
* Reject a user --initrd= first: discovery would overwrite (and leak) it, and the
* --initrd=/--linux= consistency check in verify_arguments() is bypassed once discovery
* sets arg_linux. */
if (!strv_isempty(arg_initrds))
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
"--initrd= cannot be used with --directory= unless --linux= is also specified.");
r = discover_boot_entry(arg_directory, &arg_linux, &arg_initrds);
if (r < 0)
return log_error_errno(r, "Failed to locate UKI in directory type image, please specify one with --linux=.");