From ff0af2a1660bb122f29713c9b2aff8179f165bb7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 9 Jan 2015 00:13:33 +0100 Subject: [PATCH] core: modernize execution code a bit Among other things, avoid log_struct() unless we really need it. Also, use "r" as variable to store function errors in, instead of "err". "r" is pretty much what we use everywhere else, hence using the same here make sense. FInally, in the child, when we want to log, make sure to open the logging framework first, since it is explicitly closed in preparation for the exec(). --- src/core/execute.c | 396 ++++++++++++++++++++++----------------------- 1 file changed, 198 insertions(+), 198 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 67a69b7bd..12a96a763 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1266,15 +1266,16 @@ static int build_environment( return 0; } -static int exec_child(ExecCommand *command, - const ExecContext *context, - const ExecParameters *params, - ExecRuntime *runtime, - char **argv, - int socket_fd, - int *fds, unsigned n_fds, - char **files_env, - int *error) { +static int exec_child( + ExecCommand *command, + const ExecContext *context, + const ExecParameters *params, + ExecRuntime *runtime, + char **argv, + int socket_fd, + int *fds, unsigned n_fds, + char **files_env, + int *exit_status) { _cleanup_strv_free_ char **our_env = NULL, **pam_env = NULL, **final_env = NULL, **final_argv = NULL; _cleanup_free_ char *mac_selinux_context_net = NULL; @@ -1283,12 +1284,12 @@ static int exec_child(ExecCommand *command, int dont_close[n_fds + 4]; uid_t uid = UID_INVALID; gid_t gid = GID_INVALID; - int i, err; + int i, r; assert(command); assert(context); assert(params); - assert(error); + assert(exit_status); rename_process_from_path(command->path); @@ -1303,10 +1304,10 @@ static int exec_child(ExecCommand *command, if (context->ignore_sigpipe) ignore_signals(SIGPIPE, -1); - err = reset_signal_mask(); - if (err < 0) { - *error = EXIT_SIGNAL_MASK; - return err; + r = reset_signal_mask(); + if (r < 0) { + *exit_status = EXIT_SIGNAL_MASK; + return r; } if (params->idle_pipe) @@ -1315,6 +1316,7 @@ static int exec_child(ExecCommand *command, /* Close sockets very early to make sure we don't * block init reexecution because it cannot bind its * sockets */ + log_forget_fds(); if (socket_fd >= 0) @@ -1332,15 +1334,15 @@ static int exec_child(ExecCommand *command, dont_close[n_dont_close++] = runtime->netns_storage_socket[1]; } - err = close_all_fds(dont_close, n_dont_close); - if (err < 0) { - *error = EXIT_FDS; - return err; + r = close_all_fds(dont_close, n_dont_close); + if (r < 0) { + *exit_status = EXIT_FDS; + return r; } if (!context->same_pgrp) if (setsid() < 0) { - *error = EXIT_SETSID; + *exit_status = EXIT_SETSID; return -errno; } @@ -1349,28 +1351,28 @@ static int exec_child(ExecCommand *command, if (params->confirm_spawn) { char response; - err = ask_for_confirmation(&response, argv); - if (err == -ETIMEDOUT) + r = ask_for_confirmation(&response, argv); + if (r == -ETIMEDOUT) write_confirm_message("Confirmation question timed out, assuming positive response.\n"); - else if (err < 0) - write_confirm_message("Couldn't ask confirmation question, assuming positive response: %s\n", strerror(-err)); + else if (r < 0) + write_confirm_message("Couldn't ask confirmation question, assuming positive response: %s\n", strerror(-r)); else if (response == 's') { write_confirm_message("Skipping execution.\n"); - *error = EXIT_CONFIRM; + *exit_status = EXIT_CONFIRM; return -ECANCELED; } else if (response == 'n') { write_confirm_message("Failing execution.\n"); - *error = 0; + *exit_status = 0; return 0; } } if (context->user) { username = context->user; - err = get_user_creds(&username, &uid, &gid, &home, &shell); - if (err < 0) { - *error = EXIT_USER; - return err; + r = get_user_creds(&username, &uid, &gid, &home, &shell); + if (r < 0) { + *exit_status = EXIT_USER; + return r; } } @@ -1379,29 +1381,29 @@ static int exec_child(ExecCommand *command, if (socket_fd >= 0) fd_nonblock(socket_fd, false); - err = setup_input(context, socket_fd, params->apply_tty_stdin); - if (err < 0) { - *error = EXIT_STDIN; - return err; + r = setup_input(context, socket_fd, params->apply_tty_stdin); + if (r < 0) { + *exit_status = EXIT_STDIN; + return r; } - err = setup_output(context, STDOUT_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin, uid, gid); - if (err < 0) { - *error = EXIT_STDOUT; - return err; + r = setup_output(context, STDOUT_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin, uid, gid); + if (r < 0) { + *exit_status = EXIT_STDOUT; + return r; } - err = setup_output(context, STDERR_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin, uid, gid); - if (err < 0) { - *error = EXIT_STDERR; - return err; + r = setup_output(context, STDERR_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin, uid, gid); + if (r < 0) { + *exit_status = EXIT_STDERR; + return r; } if (params->cgroup_path) { - err = cg_attach_everywhere(params->cgroup_supported, params->cgroup_path, 0, NULL, NULL); - if (err < 0) { - *error = EXIT_CGROUP; - return err; + r = cg_attach_everywhere(params->cgroup_supported, params->cgroup_path, 0, NULL, NULL); + if (r < 0) { + *exit_status = EXIT_CGROUP; + return r; } } @@ -1414,16 +1416,20 @@ static int exec_child(ExecCommand *command, * shouldn't trip up over that. */ sprintf(t, "%i", context->oom_score_adjust); - err = write_string_file("/proc/self/oom_score_adj", t); - if (err < 0 && err != -EPERM && err != EACCES) { - *error = EXIT_OOM_ADJUST; + r = write_string_file("/proc/self/oom_score_adj", t); + if (r == -EPERM || r == EACCES) { + log_open(); + log_unit_debug_errno(params->unit_id, r, "Failed to adjust OOM setting, assuming containerized execution, ignoring: %m"); + log_close(); + } else if (r < 0) { + *exit_status = EXIT_OOM_ADJUST; return -errno; } } if (context->nice_set) if (setpriority(PRIO_PROCESS, 0, context->nice) < 0) { - *error = EXIT_NICE; + *exit_status = EXIT_NICE; return -errno; } @@ -1432,38 +1438,38 @@ static int exec_child(ExecCommand *command, .sched_priority = context->cpu_sched_priority, }; - err = sched_setscheduler(0, - context->cpu_sched_policy | - (context->cpu_sched_reset_on_fork ? - SCHED_RESET_ON_FORK : 0), - ¶m); - if (err < 0) { - *error = EXIT_SETSCHEDULER; + r = sched_setscheduler(0, + context->cpu_sched_policy | + (context->cpu_sched_reset_on_fork ? + SCHED_RESET_ON_FORK : 0), + ¶m); + if (r < 0) { + *exit_status = EXIT_SETSCHEDULER; return -errno; } } if (context->cpuset) if (sched_setaffinity(0, CPU_ALLOC_SIZE(context->cpuset_ncpus), context->cpuset) < 0) { - *error = EXIT_CPUAFFINITY; + *exit_status = EXIT_CPUAFFINITY; return -errno; } if (context->ioprio_set) if (ioprio_set(IOPRIO_WHO_PROCESS, 0, context->ioprio) < 0) { - *error = EXIT_IOPRIO; + *exit_status = EXIT_IOPRIO; return -errno; } if (context->timer_slack_nsec != NSEC_INFINITY) if (prctl(PR_SET_TIMERSLACK, context->timer_slack_nsec) < 0) { - *error = EXIT_TIMERSLACK; + *exit_status = EXIT_TIMERSLACK; return -errno; } if (context->personality != 0xffffffffUL) if (personality(context->personality) < 0) { - *error = EXIT_PERSONALITY; + *exit_status = EXIT_PERSONALITY; return -errno; } @@ -1471,10 +1477,10 @@ static int exec_child(ExecCommand *command, utmp_put_init_process(context->utmp_id, getpid(), getsid(0), context->tty_path); if (context->user && is_terminal_input(context->std_input)) { - err = chown_terminal(STDIN_FILENO, uid); - if (err < 0) { - *error = EXIT_STDIN; - return err; + r = chown_terminal(STDIN_FILENO, uid); + if (r < 0) { + *exit_status = EXIT_STDIN; + return r; } } @@ -1482,10 +1488,10 @@ static int exec_child(ExecCommand *command, if (params->bus_endpoint_fd >= 0 && context->bus_endpoint) { uid_t ep_uid = (uid == UID_INVALID) ? 0 : uid; - err = bus_kernel_set_endpoint_policy(params->bus_endpoint_fd, ep_uid, context->bus_endpoint); - if (err < 0) { - *error = EXIT_BUS_ENDPOINT; - return err; + r = bus_kernel_set_endpoint_policy(params->bus_endpoint_fd, ep_uid, context->bus_endpoint); + if (r < 0) { + *exit_status = EXIT_BUS_ENDPOINT; + return r; } } #endif @@ -1494,17 +1500,17 @@ static int exec_child(ExecCommand *command, * (but only in systemd's own controller hierarchy!) to the * user of the new process. */ if (params->cgroup_path && context->user && params->cgroup_delegate) { - err = cg_set_task_access(SYSTEMD_CGROUP_CONTROLLER, params->cgroup_path, 0644, uid, gid); - if (err < 0) { - *error = EXIT_CGROUP; - return err; + r = cg_set_task_access(SYSTEMD_CGROUP_CONTROLLER, params->cgroup_path, 0644, uid, gid); + if (r < 0) { + *exit_status = EXIT_CGROUP; + return r; } - err = cg_set_group_access(SYSTEMD_CGROUP_CONTROLLER, params->cgroup_path, 0755, uid, gid); - if (err < 0) { - *error = EXIT_CGROUP; - return err; + r = cg_set_group_access(SYSTEMD_CGROUP_CONTROLLER, params->cgroup_path, 0755, uid, gid); + if (r < 0) { + *exit_status = EXIT_CGROUP; + return r; } } @@ -1516,23 +1522,23 @@ static int exec_child(ExecCommand *command, p = strjoin(params->runtime_prefix, "/", *rt, NULL); if (!p) { - *error = EXIT_RUNTIME_DIRECTORY; + *exit_status = EXIT_RUNTIME_DIRECTORY; return -ENOMEM; } - err = mkdir_safe(p, context->runtime_directory_mode, uid, gid); - if (err < 0) { - *error = EXIT_RUNTIME_DIRECTORY; - return err; + r = mkdir_safe(p, context->runtime_directory_mode, uid, gid); + if (r < 0) { + *exit_status = EXIT_RUNTIME_DIRECTORY; + return r; } } } if (params->apply_permissions) { - err = enforce_groups(context, username, gid); - if (err < 0) { - *error = EXIT_GROUP; - return err; + r = enforce_groups(context, username, gid); + if (r < 0) { + *exit_status = EXIT_GROUP; + return r; } } @@ -1540,19 +1546,19 @@ static int exec_child(ExecCommand *command, #ifdef HAVE_PAM if (params->apply_permissions && context->pam_name && username) { - err = setup_pam(context->pam_name, username, uid, context->tty_path, &pam_env, fds, n_fds); - if (err < 0) { - *error = EXIT_PAM; - return err; + r = setup_pam(context->pam_name, username, uid, context->tty_path, &pam_env, fds, n_fds); + if (r < 0) { + *exit_status = EXIT_PAM; + return r; } } #endif if (context->private_network && runtime && runtime->netns_storage_socket[0] >= 0) { - err = setup_netns(runtime->netns_storage_socket); - if (err < 0) { - *error = EXIT_NETWORK; - return err; + r = setup_netns(runtime->netns_storage_socket); + if (r < 0) { + *exit_status = EXIT_NETWORK; + return r; } } @@ -1581,7 +1587,7 @@ static int exec_child(ExecCommand *command, var = strappenda(runtime->var_tmp_dir, "/tmp"); } - err = setup_namespace( + r = setup_namespace( context->read_write_dirs, context->read_only_dirs, context->inaccessible_dirs, @@ -1593,23 +1599,28 @@ static int exec_child(ExecCommand *command, context->protect_system, context->mount_flags); - if (err == -EPERM) - log_unit_warning_errno(params->unit_id, err, "Failed to set up file system namespace due to lack of privileges. Execution sandbox will not be in effect: %m"); - else if (err < 0) { - *error = EXIT_NAMESPACE; - return err; + /* If we couldn't set up the namespace this is + * probably due to a missing capability. In this case, + * silently proceeed. */ + if (r == -EPERM || r == -EACCES) { + log_open(); + log_unit_debug_errno(params->unit_id, r, "Failed to set up namespace, assuming containerized execution, ignoring: %m"); + log_close(); + } else if (r < 0) { + *exit_status = EXIT_NAMESPACE; + return r; } } if (params->apply_chroot) { if (context->root_directory) if (chroot(context->root_directory) < 0) { - *error = EXIT_CHROOT; + *exit_status = EXIT_CHROOT; return -errno; } if (chdir(context->working_directory ? context->working_directory : "/") < 0) { - *error = EXIT_CHDIR; + *exit_status = EXIT_CHDIR; return -errno; } } else { @@ -1618,22 +1629,22 @@ static int exec_child(ExecCommand *command, if (asprintf(&d, "%s/%s", context->root_directory ? context->root_directory : "", context->working_directory ? context->working_directory : "") < 0) { - *error = EXIT_MEMORY; + *exit_status = EXIT_MEMORY; return -ENOMEM; } if (chdir(d) < 0) { - *error = EXIT_CHDIR; + *exit_status = EXIT_CHDIR; return -errno; } } #ifdef HAVE_SELINUX if (params->apply_permissions && mac_selinux_use() && params->selinux_context_net && socket_fd >= 0) { - err = mac_selinux_get_child_mls_label(socket_fd, command->path, context->selinux_context, &mac_selinux_context_net); - if (err < 0) { - *error = EXIT_SELINUX_CONTEXT; - return err; + r = mac_selinux_get_child_mls_label(socket_fd, command->path, context->selinux_context, &mac_selinux_context_net); + if (r < 0) { + *exit_status = EXIT_SELINUX_CONTEXT; + return r; } } #endif @@ -1644,14 +1655,14 @@ static int exec_child(ExecCommand *command, * and the netns fds we don't need anymore. The custom * endpoint fd was needed to upload the policy and can * now be closed as well. */ - err = close_all_fds(fds, n_fds); - if (err >= 0) - err = shift_fds(fds, n_fds); - if (err >= 0) - err = flags_fds(fds, n_fds, context->non_blocking); - if (err < 0) { - *error = EXIT_FDS; - return err; + r = close_all_fds(fds, n_fds); + if (r >= 0) + r = shift_fds(fds, n_fds); + if (r >= 0) + r = flags_fds(fds, n_fds, context->non_blocking); + if (r < 0) { + *exit_status = EXIT_FDS; + return r; } if (params->apply_permissions) { @@ -1661,34 +1672,34 @@ static int exec_child(ExecCommand *command, continue; if (setrlimit_closest(i, context->rlimit[i]) < 0) { - *error = EXIT_LIMITS; + *exit_status = EXIT_LIMITS; return -errno; } } if (context->capability_bounding_set_drop) { - err = capability_bounding_set_drop(context->capability_bounding_set_drop, false); - if (err < 0) { - *error = EXIT_CAPABILITIES; - return err; + r = capability_bounding_set_drop(context->capability_bounding_set_drop, false); + if (r < 0) { + *exit_status = EXIT_CAPABILITIES; + return r; } } #ifdef HAVE_SMACK if (context->smack_process_label) { - err = mac_smack_apply_pid(0, context->smack_process_label); - if (err < 0) { - *error = EXIT_SMACK_PROCESS_LABEL; - return err; + r = mac_smack_apply_pid(0, context->smack_process_label); + if (r < 0) { + *exit_status = EXIT_SMACK_PROCESS_LABEL; + return r; } } #endif if (context->user) { - err = enforce_user(context, uid); - if (err < 0) { - *error = EXIT_USER; - return err; + r = enforce_user(context, uid); + if (r < 0) { + *exit_status = EXIT_USER; + return r; } } @@ -1698,39 +1709,39 @@ static int exec_child(ExecCommand *command, * PR_SET_SECUREBITS unless necessary. */ if (prctl(PR_GET_SECUREBITS) != context->secure_bits) if (prctl(PR_SET_SECUREBITS, context->secure_bits) < 0) { - *error = EXIT_SECUREBITS; + *exit_status = EXIT_SECUREBITS; return -errno; } if (context->capabilities) if (cap_set_proc(context->capabilities) < 0) { - *error = EXIT_CAPABILITIES; + *exit_status = EXIT_CAPABILITIES; return -errno; } if (context->no_new_privileges) if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0) { - *error = EXIT_NO_NEW_PRIVILEGES; + *exit_status = EXIT_NO_NEW_PRIVILEGES; return -errno; } #ifdef HAVE_SECCOMP if (context->address_families_whitelist || !set_isempty(context->address_families)) { - err = apply_address_families(context); - if (err < 0) { - *error = EXIT_ADDRESS_FAMILIES; - return err; + r = apply_address_families(context); + if (r < 0) { + *exit_status = EXIT_ADDRESS_FAMILIES; + return r; } } if (context->syscall_whitelist || !set_isempty(context->syscall_filter) || !set_isempty(context->syscall_archs)) { - err = apply_seccomp(context); - if (err < 0) { - *error = EXIT_SECCOMP; - return err; + r = apply_seccomp(context); + if (r < 0) { + *exit_status = EXIT_SECCOMP; + return r; } } #endif @@ -1740,10 +1751,10 @@ static int exec_child(ExecCommand *command, char *exec_context = mac_selinux_context_net ?: context->selinux_context; if (exec_context) { - err = setexeccon(exec_context); - if (err < 0) { - *error = EXIT_SELINUX_CONTEXT; - return err; + r = setexeccon(exec_context); + if (r < 0) { + *exit_status = EXIT_SELINUX_CONTEXT; + return r; } } } @@ -1751,19 +1762,19 @@ static int exec_child(ExecCommand *command, #ifdef HAVE_APPARMOR if (context->apparmor_profile && mac_apparmor_use()) { - err = aa_change_onexec(context->apparmor_profile); - if (err < 0 && !context->apparmor_profile_ignore) { - *error = EXIT_APPARMOR_PROFILE; + r = aa_change_onexec(context->apparmor_profile); + if (r < 0 && !context->apparmor_profile_ignore) { + *exit_status = EXIT_APPARMOR_PROFILE; return -errno; } } #endif } - err = build_environment(context, n_fds, params->watchdog_usec, home, username, shell, &our_env); - if (err < 0) { - *error = EXIT_MEMORY; - return err; + r = build_environment(context, n_fds, params->watchdog_usec, home, username, shell, &our_env); + if (r < 0) { + *exit_status = EXIT_MEMORY; + return r; } final_env = strv_env_merge(5, @@ -1774,13 +1785,13 @@ static int exec_child(ExecCommand *command, pam_env, NULL); if (!final_env) { - *error = EXIT_MEMORY; + *exit_status = EXIT_MEMORY; return -ENOMEM; } final_argv = replace_env_argv(argv, final_env); if (!final_argv) { - *error = EXIT_MEMORY; + *exit_status = EXIT_MEMORY; return -ENOMEM; } @@ -1801,7 +1812,7 @@ static int exec_child(ExecCommand *command, } } execve(command->path, final_argv, final_env); - *error = EXIT_EXEC; + *exit_status = EXIT_EXEC; return -errno; } @@ -1813,10 +1824,10 @@ int exec_spawn(ExecCommand *command, _cleanup_strv_free_ char **files_env = NULL; int *fds = NULL; unsigned n_fds = 0; - char *line, **argv; - int socket_fd; + _cleanup_free_ char *line = NULL; + int socket_fd, r; + char **argv; pid_t pid; - int err; assert(command); assert(context); @@ -1828,8 +1839,10 @@ int exec_spawn(ExecCommand *command, context->std_output == EXEC_OUTPUT_SOCKET || context->std_error == EXEC_OUTPUT_SOCKET) { - if (params->n_fds != 1) + if (params->n_fds != 1) { + log_unit_error(params->unit_id, "Got more than one socket."); return -EINVAL; + } socket_fd = params->fds[0]; } else { @@ -1838,18 +1851,11 @@ int exec_spawn(ExecCommand *command, n_fds = params->n_fds; } - err = exec_context_load_environment(context, params->unit_id, &files_env); - if (err < 0) { - log_unit_struct(params->unit_id, - LOG_ERR, - LOG_MESSAGE("Failed to load environment files: %s", strerror(-err)), - LOG_ERRNO(-err), - NULL); - return err; - } + r = exec_context_load_environment(context, params->unit_id, &files_env); + if (r < 0) + return log_unit_error_errno(params->unit_id, r, "Failed to load environment files: %m"); argv = params->argv ?: command->argv; - line = exec_command_line(argv); if (!line) return log_oom(); @@ -1859,45 +1865,39 @@ int exec_spawn(ExecCommand *command, "EXECUTABLE=%s", command->path, LOG_MESSAGE("About to execute: %s", line), NULL); - free(line); - pid = fork(); if (pid < 0) - return -errno; + return log_unit_error_errno(params->unit_id, r, "Failed to fork: %m"); if (pid == 0) { - int r; - - err = exec_child(command, - context, - params, - runtime, - argv, - socket_fd, - fds, n_fds, - files_env, - &r); - if (r != 0) { + int exit_status; + + r = exec_child(command, + context, + params, + runtime, + argv, + socket_fd, + fds, n_fds, + files_env, + &exit_status); + if (r < 0) { log_open(); - log_struct(LOG_ERR, - LOG_MESSAGE_ID(SD_MESSAGE_SPAWN_FAILED), - "EXECUTABLE=%s", command->path, - LOG_MESSAGE("Failed at step %s spawning %s: %s", - exit_status_to_string(r, EXIT_STATUS_SYSTEMD), - command->path, strerror(-err)), - LOG_ERRNO(-err), - NULL); - log_close(); + log_unit_struct(params->unit_id, + LOG_ERR, + LOG_MESSAGE_ID(SD_MESSAGE_SPAWN_FAILED), + "EXECUTABLE=%s", command->path, + LOG_MESSAGE("Failed at step %s spawning %s: %s", + exit_status_to_string(exit_status, EXIT_STATUS_SYSTEMD), + command->path, strerror(-r)), + LOG_ERRNO(r), + NULL); } - _exit(r); + _exit(exit_status); } - log_unit_struct(params->unit_id, - LOG_DEBUG, - LOG_MESSAGE("Forked %s as "PID_FMT, - command->path, pid), - NULL); + log_unit_debug(params->unit_id, "Forked %s as "PID_FMT, command->path, pid); /* We add the new process to the cgroup both in the child (so * that we can be sure that no user code is ever executed -- 2.30.2