From 4dbcc319cae75233cc334556ded595636115ed47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20L=C3=BCke?= Date: Sat, 25 Apr 2026 00:18:56 +0900 Subject: [PATCH 1/3] fork-notify: Use callback instead of argv NULL code path with return In 012d87c1fc/cc8f398202 it was made possible for fork_notify() to return in the child but at that point all FDs were closed and the _cleanup path from the return causes assertion failures due to invalid FDs in notify_event_source/event, leading to a vmspawn failing to start with a SIGABRT logged in coredump. Instead of TAKE_PTR on a bunch of things which is fragile, rather avoid the return and instead add an explicit callback handler and guarantee to exit directly after it. A userdata argument is also added but not used yet I think it's quite normal to have for a callback. --- src/shared/fork-notify.c | 44 ++++++++++++++++------------------------ src/shared/fork-notify.h | 4 +++- src/vmspawn/vmspawn.c | 2 +- 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/shared/fork-notify.c b/src/shared/fork-notify.c index 066ee29115..0a34c491b7 100644 --- a/src/shared/fork-notify.c +++ b/src/shared/fork-notify.c @@ -92,9 +92,10 @@ static int on_child_notify(sd_event_source *s, int fd, uint32_t revents, void *u return 0; } -int fork_notify(char * const *argv, PidRef *ret_pidref) { +int fork_notify(char * const *argv, fork_notify_handler_t child_handler, void *child_userdata, PidRef *ret_pidref) { int r; + assert(argv); assert(ret_pidref); if (!is_main_thread()) @@ -123,7 +124,7 @@ int fork_notify(char * const *argv, PidRef *ret_pidref) { if (r < 0) return r; - if (DEBUG_LOGGING && argv) { + if (DEBUG_LOGGING) { _cleanup_free_ char *l = quote_command_line(argv, SHELL_ESCAPE_EMPTY); log_debug("Invoking '%s' as child.", strnull(l)); } @@ -145,10 +146,11 @@ int fork_notify(char * const *argv, PidRef *ret_pidref) { _exit(EXIT_MEMORY); } - if (!argv) { - *ret_pidref = TAKE_PIDREF(child); - return 0; /* Let the caller run custom code in the child */ - } + /* After fork and before exec one can execute custom code in this function + * but since all open FDs were closed only limited actions are safe (e.g., setenv), + * akin to how in a signal handler only certain things are safe. */ + if (child_handler) + child_handler(child_userdata); r = invoke_callout_binary(argv[0], argv); log_debug_errno(r, "Failed to invoke %s: %m", argv[0]); @@ -173,7 +175,7 @@ int fork_notify(char * const *argv, PidRef *ret_pidref) { *ret_pidref = TAKE_PIDREF(child); - return 1; /* In the parent */ + return 0; } static void fork_notify_terminate_internal(PidRef *pidref) { @@ -241,7 +243,14 @@ int journal_fork(RuntimeScope scope, char * const* units, OutputMode output, Pid if (strv_extendf(&argv, "--output=%s", output_mode_to_string(output)) < 0) return log_oom_debug(); - return fork_notify(argv, ret_pidref); + return fork_notify(argv, /* child_handler= */ NULL, /* child_userdata= */ NULL, ret_pidref); +} + +static void set_journal_remote_config(void *userdata) { + if (setenv("SYSTEMD_JOURNAL_REMOTE_CONFIG_FILE", "/dev/null", /* overwrite= */ true) < 0) { + log_debug_errno(errno, "Failed to set $SYSTEMD_JOURNAL_REMOTE_CONFIG_FILE: %m"); + _exit(EXIT_MEMORY); + } } int fork_journal_remote( @@ -313,22 +322,5 @@ int fork_journal_remote( strv_extendf(&argv, "--max-files=%" PRIu64, max_files) < 0) return log_oom(); - r = fork_notify(/* argv= */ NULL, ret_pidref); - if (r < 0) - return r; - if (r == 0) { - /* In the child */ - if (setenv("SYSTEMD_JOURNAL_REMOTE_CONFIG_FILE", - "/dev/null", - /* overwrite= */ true) < 0) { - log_debug_errno(errno, "Failed to set $SYSTEMD_JOURNAL_REMOTE_CONFIG_FILE: %m"); - _exit(EXIT_MEMORY); - } - - r = invoke_callout_binary(argv[0], argv); - log_debug_errno(r, "Failed to invoke %s: %m", argv[0]); - _exit(EXIT_EXEC); - } - - return 0; + return fork_notify(argv, set_journal_remote_config, /* child_userdata= */ NULL, ret_pidref); } diff --git a/src/shared/fork-notify.h b/src/shared/fork-notify.h index cc241beff9..cf22f48ba2 100644 --- a/src/shared/fork-notify.h +++ b/src/shared/fork-notify.h @@ -4,7 +4,9 @@ #include "output-mode.h" #include "shared-forward.h" -int fork_notify(char * const *argv, PidRef *ret_pidref); +typedef void (*fork_notify_handler_t)(void *userdata); + +int fork_notify(char * const *argv, fork_notify_handler_t child_handler, void *child_userdata, PidRef *ret_pidref); void fork_notify_terminate(PidRef *pidref); diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index 1a349a1d63..d7784e1e9a 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -1565,7 +1565,7 @@ static int start_tpm( if (r < 0) return log_oom(); - r = fork_notify(argv, ret_pidref); + r = fork_notify(argv, /* child_handler= */ NULL, /* child_userdata= */ NULL, ret_pidref); if (r < 0) return r; From 41cf6891213dd78c5af3c2b8ff05194c52efa62f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20L=C3=BCke?= Date: Sat, 25 Apr 2026 01:42:36 +0900 Subject: [PATCH 2/3] nsresource: fix buffer overrun reported by ASAN This came up when running systemd-vmspawn with ASAN to fix another bug and thus I had to fix this overrun here first: The dispatch tables were missing the terminator, add it. --- src/shared/nsresource.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared/nsresource.c b/src/shared/nsresource.c index 7a1c33446b..b783cb80b7 100644 --- a/src/shared/nsresource.c +++ b/src/shared/nsresource.c @@ -372,6 +372,7 @@ int nsresource_add_netif_veth( static const sd_json_dispatch_field dispatch_table[] = { { "hostInterfaceName", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(InterfaceParams, host_interface_name), SD_JSON_MANDATORY }, { "namespaceInterfaceName", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(InterfaceParams, namespace_interface_name), SD_JSON_MANDATORY }, + {} }; _cleanup_(interface_params_done) InterfaceParams p = {}; @@ -437,6 +438,7 @@ int nsresource_add_netif_tap( static const sd_json_dispatch_field dispatch_table[] = { { "hostInterfaceName", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(InterfaceParams, host_interface_name), SD_JSON_MANDATORY }, { "interfaceFileDescriptor", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint, offsetof(InterfaceParams, interface_fd_index), SD_JSON_MANDATORY }, + {} }; _cleanup_(interface_params_done) InterfaceParams p = {}; From 70d75c22d700098dea39151b12bba9729bed0f67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20L=C3=BCke?= Date: Sat, 25 Apr 2026 01:49:09 +0900 Subject: [PATCH 3/3] machine-register: don't unref borrowed varlink reply ASAN showed a use-after-free error for systemd-vmspawn's machine_register call because the reply got accessed and freed again through _cleanup. The same problem exists in two verb_machine_control_one/unregister_machine. Fix these call sites to not set up _cleanup. --- src/machine/machinectl.c | 2 +- src/shared/machine-register.c | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index e8a30a8959..ae5473aa99 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -1242,7 +1242,7 @@ static int verb_machine_control_one(const char *machine_name, const char *method if (r < 0) return log_error_errno(r, "Failed to connect to machine control socket: %m"); - _cleanup_(sd_json_variant_unrefp) sd_json_variant *reply = NULL; + sd_json_variant *reply = NULL; const char *error_id = NULL; r = sd_varlink_call(vl, method, /* parameters= */ NULL, &reply, &error_id); if (r < 0) diff --git a/src/shared/machine-register.c b/src/shared/machine-register.c index a161d1b050..9637793fe5 100644 --- a/src/shared/machine-register.c +++ b/src/shared/machine-register.c @@ -170,8 +170,7 @@ int register_machine( } if (r < 0) return log_debug_errno(r, "Failed to connect to machined on %s: %m", strna(p)); - - _cleanup_(sd_json_variant_unrefp) sd_json_variant *reply = NULL; + sd_json_variant *reply = NULL; const char *error_id = NULL; r = sd_varlink_callbo( vl, @@ -306,7 +305,7 @@ int unregister_machine(sd_bus *bus, const char *machine_name, RuntimeScope scope if (r >= 0) r = sd_varlink_connect_address(&vl, p); if (r >= 0) { - _cleanup_(sd_json_variant_unrefp) sd_json_variant *reply = NULL; + sd_json_variant *reply = NULL; const char *error_id = NULL; r = sd_varlink_callbo( vl,