Skip to content

Commit

Permalink
manager: fix error handling after failure to set up child
Browse files Browse the repository at this point in the history
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 3909020 and
ad21e54.

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.
  • Loading branch information
keszybz committed Aug 16, 2023
1 parent 7ab2471 commit 5fa01ac
Showing 1 changed file with 13 additions and 8 deletions.
21 changes: 13 additions & 8 deletions src/core/execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand All @@ -6062,17 +6067,17 @@ 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,
LOG_UNIT_INVOCATION_ID(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);
}
Expand Down

0 comments on commit 5fa01ac

Please sign in to comment.