From 5fa01ac0369f0f225ab1e1f90f6b7058cc4deaaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 10 Aug 2023 10:59:55 +0200 Subject: [PATCH] manager: fix error handling after failure to set up child MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit exec_child() is supposed to set *exit_status when returning failure. Unfortunately, we didn't do that in two cases. The result would be: - a bogus error message "Failed at step SUCCESS spawning foo: …", - a bogus success exit status. Bugs introduced in 390902012c5177b6b01bc634b2e9c704073d9e7d and ad21e542b20f0fb292d1958d3a759bf3403522c2. The code is reworked to add some asserts and not set exit_status in the caller so that it's clearer (also to the compiler) that it needs to be set. --- src/core/execute.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index a81a7d57d4..4b6e927e66 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4958,6 +4958,7 @@ static int exec_child( *exit_status = EXIT_SUCCESS; return 0; } + *exit_status = EXIT_CONFIRM; return log_unit_error_errno(unit, SYNTHETIC_ERRNO(ECANCELED), "Execution cancelled by the user"); @@ -5128,14 +5129,18 @@ static int exec_child( r = set_coredump_filter(context->coredump_filter); if (ERRNO_IS_NEG_PRIVILEGE(r)) log_unit_debug_errno(unit, r, "Failed to adjust coredump_filter, ignoring: %m"); - else if (r < 0) + else if (r < 0) { + *exit_status = EXIT_LIMITS; return log_unit_error_errno(unit, r, "Failed to adjust coredump_filter: %m"); + } } if (context->nice_set) { r = setpriority_closest(context->nice); - if (r < 0) + if (r < 0) { + *exit_status = EXIT_NICE; return log_unit_error_errno(unit, r, "Failed to set up process scheduling priority (nice level): %m"); + } } if (context->cpu_sched_set) { @@ -5578,11 +5583,11 @@ static int exec_child( LOG_UNIT_MESSAGE(unit, "Executable %s missing, skipping: %m", command->path), "EXECUTABLE=%s", command->path); + *exit_status = EXIT_SUCCESS; return 0; } *exit_status = EXIT_EXEC; - return log_unit_struct_errno(unit, LOG_INFO, r, "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR, LOG_UNIT_INVOCATION_ID(unit), @@ -6044,7 +6049,7 @@ int exec_spawn(Unit *unit, return log_unit_error_errno(unit, errno, "Failed to fork: %m"); if (pid == 0) { - int exit_status = EXIT_SUCCESS; + int exit_status; r = exec_child(unit, command, @@ -6062,9 +6067,8 @@ int exec_spawn(Unit *unit, &exit_status); if (r < 0) { - const char *status = - exit_status_to_string(exit_status, - EXIT_STATUS_LIBC | EXIT_STATUS_SYSTEMD); + const char *status = ASSERT_PTR( + exit_status_to_string(exit_status, EXIT_STATUS_LIBC | EXIT_STATUS_SYSTEMD)); log_unit_struct_errno(unit, LOG_ERR, r, "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR, @@ -6072,7 +6076,8 @@ int exec_spawn(Unit *unit, LOG_UNIT_MESSAGE(unit, "Failed at step %s spawning %s: %m", status, command->path), "EXECUTABLE=%s", command->path); - } + } else + assert(exit_status == EXIT_SUCCESS); _exit(exit_status); }