diff --git a/TODO b/TODO index cc7a332a8b..37323e3a19 100644 --- a/TODO +++ b/TODO @@ -19,6 +19,8 @@ Janitorial Clean-ups: Features: +* if /usr/bin/swapoff fails due to OOM, log a friendly explanatory message about it + * build short web pages out of each catalog entry, build them along with man pages, and include hyperlinks to them in the journal output diff --git a/man/systemd.kill.xml b/man/systemd.kill.xml index 6a1c67d406..5e4d430031 100644 --- a/man/systemd.kill.xml +++ b/man/systemd.kill.xml @@ -61,28 +61,25 @@ KillMode= - Specifies how processes of this unit shall be - killed. One of - , - , - , + Specifies how processes of this unit shall be killed. One of + , , , . - If set to , all remaining - processes in the control group of this unit will be killed on - unit stop (for services: after the stop command is executed, - as configured with ExecStop=). If set to - , only the main process itself is - killed. If set to , the - SIGTERM signal (see below) is sent to the - main process while the subsequent SIGKILL - signal (see below) is sent to all remaining processes of the - unit's control group. If set to , no - process is killed. In this case, only the stop command will be - executed on unit stop, but no process will be killed otherwise. - Processes remaining alive after stop are left in their control - group and the control group continues to exist after stop - unless it is empty. + If set to , all remaining processes in the control group of this + unit will be killed on unit stop (for services: after the stop command is executed, as configured + with ExecStop=). If set to , the + SIGTERM signal (see below) is sent to the main process while the subsequent + SIGKILL signal (see below) is sent to all remaining processes of the unit's + control group. If set to , only the main process itself is killed (not + recommended!). If set to , no process is killed (strongly recommended + against!). In this case, only the stop command will be executed on unit stop, but no process will be + killed otherwise. Processes remaining alive after stop are left in their control group and the + control group continues to exist after stop unless empty. + + Note that it is not recommended to set KillMode= to + process or even none, as this allows processes to escape + the service manager's lifecycle and resource management, and to remain running even while their + service is considered stopped and is assumed to not consume any resources. Processes will first be terminated via SIGTERM (unless the signal to send is changed via KillSignal= or RestartKillSignal=). Optionally, diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 352798fd6a..96a3f12103 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -117,7 +117,6 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_device_policy, cgroup_device_policy, CGrou DEFINE_CONFIG_PARSE_ENUM(config_parse_exec_keyring_mode, exec_keyring_mode, ExecKeyringMode, "Failed to parse keyring mode"); DEFINE_CONFIG_PARSE_ENUM(config_parse_exec_utmp_mode, exec_utmp_mode, ExecUtmpMode, "Failed to parse utmp mode"); DEFINE_CONFIG_PARSE_ENUM(config_parse_job_mode, job_mode, JobMode, "Failed to parse job mode"); -DEFINE_CONFIG_PARSE_ENUM(config_parse_kill_mode, kill_mode, KillMode, "Failed to parse kill mode"); DEFINE_CONFIG_PARSE_ENUM(config_parse_notify_access, notify_access, NotifyAccess, "Failed to parse notify access specifier"); DEFINE_CONFIG_PARSE_ENUM(config_parse_protect_home, protect_home, ProtectHome, "Failed to parse protect home value"); DEFINE_CONFIG_PARSE_ENUM(config_parse_protect_system, protect_system, ProtectSystem, "Failed to parse protect system value"); @@ -631,6 +630,48 @@ int config_parse_exec_coredump_filter( return 0; } +int config_parse_kill_mode( + const char* unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + KillMode *k = data, m; + + assert(filename); + assert(lvalue); + assert(rvalue); + assert(data); + + if (isempty(rvalue)) { + *k = KILL_CONTROL_GROUP; + return 0; + } + + m = kill_mode_from_string(rvalue); + if (m < 0) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Failed to parse kill mode specification, ignoring: %s", rvalue); + return 0; + } + + if (m == KILL_NONE) + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Unit configured to use KillMode=none. " + "This is unsafe, as it disables systemd's process life-cycle management for the service. " + "Please update your service to use a safer KillMode=, such as 'mixed' or 'control-group'. " + "Support for KillMode=none is deprecated and will eventually be removed."); + + *k = m; + return 0; +} + int config_parse_exec( const char *unit, const char *filename, diff --git a/src/core/mount.c b/src/core/mount.c index 48500b4932..10613d11ae 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -863,6 +863,8 @@ static void mount_enter_dead(Mount *m, MountResult f) { m->result = f; unit_log_result(UNIT(m), m->result == MOUNT_SUCCESS, mount_result_to_string(m->result)); + unit_warn_leftover_processes(UNIT(m), unit_log_leftover_process_stop); + mount_set_state(m, m->result != MOUNT_SUCCESS ? MOUNT_FAILED : MOUNT_DEAD); m->exec_runtime = exec_runtime_unref(m->exec_runtime, true); @@ -1008,7 +1010,7 @@ static void mount_enter_mounting(Mount *m) { (void) mkdir_p_label(m->where, m->directory_mode); unit_warn_if_dir_nonempty(UNIT(m), m->where); - unit_warn_leftover_processes(UNIT(m)); + unit_warn_leftover_processes(UNIT(m), unit_log_leftover_process_start); m->control_command_id = MOUNT_EXEC_MOUNT; m->control_command = m->exec_command + MOUNT_EXEC_MOUNT; diff --git a/src/core/service.c b/src/core/service.c index a824f2d6a0..340b655059 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -438,14 +438,17 @@ static int service_add_fd_store(Service *s, int fd, const char *name, bool do_po } } - fs = new0(ServiceFDStore, 1); + fs = new(ServiceFDStore, 1); if (!fs) return -ENOMEM; - fs->fd = fd; - fs->service = s; - fs->do_poll = do_poll; - fs->fdname = strdup(name ?: "stored"); + *fs = (ServiceFDStore) { + .fd = fd, + .service = s, + .do_poll = do_poll, + .fdname = strdup(name ?: "stored"), + }; + if (!fs->fdname) { free(fs); return -ENOMEM; @@ -1746,6 +1749,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) unit_log_failure(UNIT(s), service_result_to_string(s->result)); end_state = SERVICE_FAILED; } + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_stop); if (!allow_restart) log_unit_debug(UNIT(s), "Service restart not allowed."); @@ -2072,15 +2076,16 @@ static int service_adverse_to_leftover_processes(Service *s) { assert(s); /* KillMode=mixed and control group are used to indicate that all process should be killed off. - * SendSIGKILL is used for services that require a clean shutdown. These are typically database - * service where a SigKilled process would result in a lengthy recovery and who's shutdown or - * startup time is quite variable (so Timeout settings aren't of use). + * SendSIGKILL= is used for services that require a clean shutdown. These are typically database + * service where a SigKilled process would result in a lengthy recovery and who's shutdown or startup + * time is quite variable (so Timeout settings aren't of use). * * Here we take these two factors and refuse to start a service if there are existing processes * within a control group. Databases, while generally having some protection against multiple - * instances running, lets not stress the rigor of these. Also ExecStartPre parts of the service + * instances running, lets not stress the rigor of these. Also ExecStartPre= parts of the service * aren't as rigoriously written to protect aganst against multiple use. */ - if (unit_warn_leftover_processes(UNIT(s)) && + + if (unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_start) > 0 && IN_SET(s->kill_context.kill_mode, KILL_MIXED, KILL_CONTROL_GROUP) && !s->kill_context.send_sigkill) return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(EBUSY), diff --git a/src/core/socket.c b/src/core/socket.c index 5e8f317bfc..218b4b245d 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2035,6 +2035,8 @@ static void socket_enter_dead(Socket *s, SocketResult f) { else unit_log_failure(UNIT(s), socket_result_to_string(s->result)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_stop); + socket_set_state(s, s->result != SOCKET_SUCCESS ? SOCKET_FAILED : SOCKET_DEAD); s->exec_runtime = exec_runtime_unref(s->exec_runtime, true); @@ -2237,7 +2239,7 @@ static void socket_enter_start_pre(Socket *s) { socket_unwatch_control_pid(s); - unit_warn_leftover_processes(UNIT(s)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_start); s->control_command_id = SOCKET_EXEC_START_PRE; s->control_command = s->exec_command[SOCKET_EXEC_START_PRE]; diff --git a/src/core/swap.c b/src/core/swap.c index 0b42236aca..20179de2d2 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -701,6 +701,7 @@ static void swap_enter_dead(Swap *s, SwapResult f) { s->result = f; unit_log_result(UNIT(s), s->result == SWAP_SUCCESS, swap_result_to_string(s->result)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_stop); swap_set_state(s, s->result != SWAP_SUCCESS ? SWAP_FAILED : SWAP_DEAD); s->exec_runtime = exec_runtime_unref(s->exec_runtime, true); @@ -788,7 +789,7 @@ static void swap_enter_activating(Swap *s) { assert(s); - unit_warn_leftover_processes(UNIT(s)); + unit_warn_leftover_processes(UNIT(s), unit_log_leftover_process_start); s->control_command_id = SWAP_EXEC_ACTIVATE; s->control_command = s->exec_command + SWAP_EXEC_ACTIVATE; diff --git a/src/core/unit.c b/src/core/unit.c index c5eb72163b..b287c77117 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4971,11 +4971,11 @@ int unit_kill_context( if (!pid_set) return -ENOMEM; - cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, - SIGHUP, - CGROUP_IGNORE_SELF, - pid_set, - NULL, NULL); + (void) cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, + SIGHUP, + CGROUP_IGNORE_SELF, + pid_set, + NULL, NULL); } } } @@ -5861,14 +5861,20 @@ int unit_prepare_exec(Unit *u) { return 0; } -static int log_leftover(pid_t pid, int sig, void *userdata) { +static bool ignore_leftover_process(const char *comm) { + return comm && comm[0] == '('; /* Most likely our own helper process (PAM?), ignore */ +} + +int unit_log_leftover_process_start(pid_t pid, int sig, void *userdata) { _cleanup_free_ char *comm = NULL; (void) get_process_comm(pid, &comm); - if (comm && comm[0] == '(') /* Most likely our own helper process (PAM?), ignore */ + if (ignore_leftover_process(comm)) return 0; + /* During start we print a warning */ + log_unit_warning(userdata, "Found left-over process " PID_FMT " (%s) in control group while starting unit. Ignoring.\n" "This usually indicates unclean termination of a previous run, or service implementation deficiencies.", @@ -5877,7 +5883,24 @@ static int log_leftover(pid_t pid, int sig, void *userdata) { return 1; } -int unit_warn_leftover_processes(Unit *u) { +int unit_log_leftover_process_stop(pid_t pid, int sig, void *userdata) { + _cleanup_free_ char *comm = NULL; + + (void) get_process_comm(pid, &comm); + + if (ignore_leftover_process(comm)) + return 0; + + /* During stop we only print an informational message */ + + log_unit_info(userdata, + "Unit process " PID_FMT " (%s) remains running after unit stopped.", + pid, strna(comm)); + + return 1; +} + +int unit_warn_leftover_processes(Unit *u, cg_kill_log_func_t log_func) { assert(u); (void) unit_pick_cgroup_path(u); @@ -5885,7 +5908,7 @@ int unit_warn_leftover_processes(Unit *u) { if (!u->cgroup_path) return 0; - return cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, 0, 0, NULL, log_leftover, u); + return cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, 0, 0, NULL, log_func, u); } bool unit_needs_console(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index bee68607fe..6659406c2d 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -846,7 +846,9 @@ void unit_unlink_state_files(Unit *u); int unit_prepare_exec(Unit *u); -int unit_warn_leftover_processes(Unit *u); +int unit_log_leftover_process_start(pid_t pid, int sig, void *userdata); +int unit_log_leftover_process_stop(pid_t pid, int sig, void *userdata); +int unit_warn_leftover_processes(Unit *u, cg_kill_log_func_t log_func); bool unit_needs_console(Unit *u); diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index 5df5743823..3ad139b242 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -788,19 +788,25 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { if (!fpath) return log_oom(); - service = new0(SysvStub, 1); + log_warning("SysV service '%s' lacks a native systemd unit file. " + "Automatically generating a unit file for compatibility. " + "Please update package to include a native systemd unit file, in order to make it more safe and robust.", fpath); + + service = new(SysvStub, 1); if (!service) return log_oom(); - service->sysv_start_priority = -1; - service->name = TAKE_PTR(name); - service->path = TAKE_PTR(fpath); + *service = (SysvStub) { + .sysv_start_priority = -1, + .name = TAKE_PTR(name), + .path = TAKE_PTR(fpath), + }; r = hashmap_put(all_services, service->name, service); if (r < 0) return log_oom(); - service = NULL; + TAKE_PTR(service); } } diff --git a/units/console-getty.service.m4 b/units/console-getty.service.m4 index 3c553240a2..7c3992e8e0 100644 --- a/units/console-getty.service.m4 +++ b/units/console-getty.service.m4 @@ -30,7 +30,9 @@ UtmpIdentifier=cons TTYPath=/dev/console TTYReset=yes TTYVHangup=yes +m4_ifdef(`ENABLE_LOGIND',, KillMode=process +)m4_dnl IgnoreSIGPIPE=no SendSIGHUP=yes diff --git a/units/container-getty@.service.m4 b/units/container-getty@.service.m4 index 087ab7f9b1..e4f9ae5791 100644 --- a/units/container-getty@.service.m4 +++ b/units/container-getty@.service.m4 @@ -36,6 +36,8 @@ UtmpIdentifier=pts/%I TTYPath=/dev/pts/%I TTYReset=yes TTYVHangup=yes +m4_ifdef(`ENABLE_LOGIND',, KillMode=process +)m4_dnl IgnoreSIGPIPE=no SendSIGHUP=yes diff --git a/units/getty@.service.m4 b/units/getty@.service.m4 index 80e793bb73..087d686666 100644 --- a/units/getty@.service.m4 +++ b/units/getty@.service.m4 @@ -47,7 +47,9 @@ TTYPath=/dev/%I TTYReset=yes TTYVHangup=yes TTYVTDisallocate=yes +m4_ifdef(`ENABLE_LOGIND',, KillMode=process +)m4_dnl IgnoreSIGPIPE=no SendSIGHUP=yes diff --git a/units/serial-getty@.service.m4 b/units/serial-getty@.service.m4 index 757b86ab2f..cab89fb426 100644 --- a/units/serial-getty@.service.m4 +++ b/units/serial-getty@.service.m4 @@ -40,7 +40,9 @@ UtmpIdentifier=%I TTYPath=/dev/%I TTYReset=yes TTYVHangup=yes +m4_ifdef(`ENABLE_LOGIND',, KillMode=process +)m4_dnl IgnoreSIGPIPE=no SendSIGHUP=yes