From ecedd90fcdf647f9a7b56b4934b65e30b2979b04 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Apr 2012 23:24:47 +0200 Subject: service: place control command in subcgroup control/ Previously, we were brutally and onconditionally killing all processes in a service's cgroup before starting the service anew, in order to ensure that StartPre lines cannot be misused to spawn long-running processes. On logind-less systems this has the effect that restarting sshd necessarily calls all active ssh sessions, which is usually not desirable. With this patch control processes for a service are placed in a sub-cgroup called "control/". When starting a service anew we simply kill this cgroup, but not the main cgroup, in order to avoid killing any long-running non-control processes from previous runs. https://bugzilla.redhat.com/show_bug.cgi?id=805942 --- TODO | 2 - src/core/cgroup.c | 52 +++++++++++++--- src/core/cgroup.h | 8 +-- src/core/execute.c | 5 +- src/core/execute.h | 1 + src/core/mount.c | 7 ++- src/core/service.c | 175 +++++++++++++++++++++++++++++------------------------ src/core/socket.c | 7 ++- src/core/swap.c | 7 ++- 9 files changed, 163 insertions(+), 101 deletions(-) diff --git a/TODO b/TODO index fa0905657..12ce158ec 100644 --- a/TODO +++ b/TODO @@ -19,8 +19,6 @@ Features: * cg_create_and_attach() should fail for non-available controllers -* place start-pre/start-post/... scripts in sub cgrouprs - * make gtk-doc optional (like kmod?) * udev: find a way to tell udev to not cancel firmware requests in initramfs diff --git a/src/core/cgroup.c b/src/core/cgroup.c index ef9b02f46..7a5f673a7 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -108,26 +108,43 @@ void cgroup_bonding_trim_list(CGroupBonding *first, bool delete_root) { cgroup_bonding_trim(b, delete_root); } -int cgroup_bonding_install(CGroupBonding *b, pid_t pid) { + +int cgroup_bonding_install(CGroupBonding *b, pid_t pid, const char *cgroup_suffix) { + char *p = NULL; + const char *path; int r; assert(b); assert(pid >= 0); - if ((r = cg_create_and_attach(b->controller, b->path, pid)) < 0) + if (cgroup_suffix) { + p = join(b->path, "/", cgroup_suffix, NULL); + if (!p) + return -ENOMEM; + + path = p; + } else + path = b->path; + + r = cg_create_and_attach(b->controller, path, pid); + free(p); + + if (r < 0) return r; b->realized = true; return 0; } -int cgroup_bonding_install_list(CGroupBonding *first, pid_t pid) { +int cgroup_bonding_install_list(CGroupBonding *first, pid_t pid, const char *cgroup_suffix) { CGroupBonding *b; int r; - LIST_FOREACH(by_unit, b, first) - if ((r = cgroup_bonding_install(b, pid)) < 0 && b->essential) + LIST_FOREACH(by_unit, b, first) { + r = cgroup_bonding_install(b, pid, cgroup_suffix); + if (r < 0 && b->essential) return r; + } return 0; } @@ -176,7 +193,11 @@ int cgroup_bonding_set_task_access_list(CGroupBonding *first, mode_t mode, uid_t return 0; } -int cgroup_bonding_kill(CGroupBonding *b, int sig, bool sigcont, Set *s) { +int cgroup_bonding_kill(CGroupBonding *b, int sig, bool sigcont, Set *s, const char *cgroup_suffix) { + char *p = NULL; + const char *path; + int r; + assert(b); assert(sig >= 0); @@ -184,10 +205,22 @@ int cgroup_bonding_kill(CGroupBonding *b, int sig, bool sigcont, Set *s) { if (!b->ours) return 0; - return cg_kill_recursive(b->controller, b->path, sig, sigcont, true, false, s); + if (cgroup_suffix) { + p = join(b->path, "/", cgroup_suffix, NULL); + if (!p) + return -ENOMEM; + + path = p; + } else + path = b->path; + + r = cg_kill_recursive(b->controller, path, sig, sigcont, true, false, s); + free(p); + + return r; } -int cgroup_bonding_kill_list(CGroupBonding *first, int sig, bool sigcont, Set *s) { +int cgroup_bonding_kill_list(CGroupBonding *first, int sig, bool sigcont, Set *s, const char *cgroup_suffix) { CGroupBonding *b; Set *allocated_set = NULL; int ret = -EAGAIN, r; @@ -200,7 +233,8 @@ int cgroup_bonding_kill_list(CGroupBonding *first, int sig, bool sigcont, Set *s return -ENOMEM; LIST_FOREACH(by_unit, b, first) { - if ((r = cgroup_bonding_kill(b, sig, sigcont, s)) < 0) { + r = cgroup_bonding_kill(b, sig, sigcont, s, cgroup_suffix); + if (r < 0) { if (r == -EAGAIN || r == -ESRCH) continue; diff --git a/src/core/cgroup.h b/src/core/cgroup.h index de248fbca..95f09e001 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -56,8 +56,8 @@ int cgroup_bonding_realize_list(CGroupBonding *first); void cgroup_bonding_free(CGroupBonding *b, bool trim); void cgroup_bonding_free_list(CGroupBonding *first, bool trim); -int cgroup_bonding_install(CGroupBonding *b, pid_t pid); -int cgroup_bonding_install_list(CGroupBonding *first, pid_t pid); +int cgroup_bonding_install(CGroupBonding *b, pid_t pid, const char *suffix); +int cgroup_bonding_install_list(CGroupBonding *first, pid_t pid, const char *suffix); int cgroup_bonding_set_group_access(CGroupBonding *b, mode_t mode, uid_t uid, gid_t gid); int cgroup_bonding_set_group_access_list(CGroupBonding *b, mode_t mode, uid_t uid, gid_t gid); @@ -65,8 +65,8 @@ int cgroup_bonding_set_group_access_list(CGroupBonding *b, mode_t mode, uid_t ui int cgroup_bonding_set_task_access(CGroupBonding *b, mode_t mode, uid_t uid, gid_t gid, int sticky); int cgroup_bonding_set_task_access_list(CGroupBonding *b, mode_t mode, uid_t uid, gid_t gid, int sticky); -int cgroup_bonding_kill(CGroupBonding *b, int sig, bool sigcont, Set *s); -int cgroup_bonding_kill_list(CGroupBonding *first, int sig, bool sigcont, Set *s); +int cgroup_bonding_kill(CGroupBonding *b, int sig, bool sigcont, Set *s, const char *suffix); +int cgroup_bonding_kill_list(CGroupBonding *first, int sig, bool sigcont, Set *s, const char *suffix); void cgroup_bonding_trim(CGroupBonding *first, bool delete_root); void cgroup_bonding_trim_list(CGroupBonding *first, bool delete_root); diff --git a/src/core/execute.c b/src/core/execute.c index 271c57f56..c59f7e2da 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -962,6 +962,7 @@ int exec_spawn(ExecCommand *command, bool confirm_spawn, CGroupBonding *cgroup_bondings, CGroupAttribute *cgroup_attributes, + const char *cgroup_suffix, pid_t *ret) { pid_t pid; @@ -1154,7 +1155,7 @@ int exec_spawn(ExecCommand *command, } if (cgroup_bondings) { - err = cgroup_bonding_install_list(cgroup_bondings, 0); + err = cgroup_bonding_install_list(cgroup_bondings, 0, cgroup_suffix); if (err < 0) { r = EXIT_CGROUP; goto fail_child; @@ -1505,7 +1506,7 @@ int exec_spawn(ExecCommand *command, * sure that when we kill the cgroup the process will be * killed too). */ if (cgroup_bondings) - cgroup_bonding_install_list(cgroup_bondings, pid); + cgroup_bonding_install_list(cgroup_bondings, pid, cgroup_suffix); log_debug("Forked %s as %lu", command->path, (unsigned long) pid); diff --git a/src/core/execute.h b/src/core/execute.h index fc4c71e53..428bb7c9b 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -192,6 +192,7 @@ int exec_spawn(ExecCommand *command, bool confirm_spawn, struct CGroupBonding *cgroup_bondings, struct CGroupAttribute *cgroup_attributes, + const char *cgroup_suffix, pid_t *ret); void exec_command_done(ExecCommand *c); diff --git a/src/core/mount.c b/src/core/mount.c index 6c768a3d0..760ffcdbf 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -805,6 +805,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { UNIT(m)->manager->confirm_spawn, UNIT(m)->cgroup_bondings, UNIT(m)->cgroup_attributes, + NULL, &pid)) < 0) goto fail; @@ -875,7 +876,8 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { if ((r = set_put(pid_set, LONG_TO_PTR(m->control_pid))) < 0) goto fail; - if ((r = cgroup_bonding_kill_list(UNIT(m)->cgroup_bondings, sig, true, pid_set)) < 0) { + r = cgroup_bonding_kill_list(UNIT(m)->cgroup_bondings, sig, true, pid_set, NULL); + if (r < 0) { if (r != -EAGAIN && r != -ESRCH && r != -ENOENT) log_warning("Failed to kill control group: %s", strerror(-r)); } else if (r > 0) @@ -1833,7 +1835,8 @@ static int mount_kill(Unit *u, KillWho who, KillMode mode, int signo, DBusError goto finish; } - if ((q = cgroup_bonding_kill_list(UNIT(m)->cgroup_bondings, signo, false, pid_set)) < 0) + q = cgroup_bonding_kill_list(UNIT(m)->cgroup_bondings, signo, false, pid_set, NULL); + if (q < 0) if (q != -EAGAIN && q != -ESRCH && q != -ENOENT) r = q; } diff --git a/src/core/service.c b/src/core/service.c index 1c04ed338..59dd71294 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1686,6 +1686,7 @@ static int service_spawn( bool apply_chroot, bool apply_tty_stdin, bool set_notify_socket, + bool is_control, pid_t *_pid) { pid_t pid; @@ -1767,6 +1768,7 @@ static int service_spawn( UNIT(s)->manager->confirm_spawn, UNIT(s)->cgroup_bondings, UNIT(s)->cgroup_attributes, + is_control ? "control" : NULL, &pid); if (r < 0) @@ -1886,15 +1888,17 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { if ((s->control_command = s->exec_command[SERVICE_EXEC_STOP_POST])) { s->control_command_id = SERVICE_EXEC_STOP_POST; - if ((r = service_spawn(s, - s->control_command, - true, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - true, - false, - &s->control_pid)) < 0) + r = service_spawn(s, + s->control_command, + true, + false, + !s->permissions_start_only, + !s->root_directory_start_only, + true, + false, + true, + &s->control_pid); + if (r < 0) goto fail; @@ -1952,7 +1956,8 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f if ((r = set_put(pid_set, LONG_TO_PTR(s->control_pid))) < 0) goto fail; - if ((r = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, sig, true, pid_set)) < 0) { + r = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, sig, true, pid_set, NULL); + if (r < 0) { if (r != -EAGAIN && r != -ESRCH && r != -ENOENT) log_warning("Failed to kill control group: %s", strerror(-r)); } else if (r > 0) @@ -2001,15 +2006,17 @@ static void service_enter_stop(Service *s, ServiceResult f) { if ((s->control_command = s->exec_command[SERVICE_EXEC_STOP])) { s->control_command_id = SERVICE_EXEC_STOP; - if ((r = service_spawn(s, - s->control_command, - true, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - false, - false, - &s->control_pid)) < 0) + r = service_spawn(s, + s->control_command, + true, + false, + !s->permissions_start_only, + !s->root_directory_start_only, + false, + false, + true, + &s->control_pid); + if (r < 0) goto fail; service_set_state(s, SERVICE_STOP); @@ -2054,15 +2061,17 @@ static void service_enter_start_post(Service *s) { if ((s->control_command = s->exec_command[SERVICE_EXEC_START_POST])) { s->control_command_id = SERVICE_EXEC_START_POST; - if ((r = service_spawn(s, - s->control_command, - true, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - false, - false, - &s->control_pid)) < 0) + r = service_spawn(s, + s->control_command, + true, + false, + !s->permissions_start_only, + !s->root_directory_start_only, + false, + false, + true, + &s->control_pid); + if (r < 0) goto fail; service_set_state(s, SERVICE_START_POST); @@ -2094,7 +2103,7 @@ static void service_enter_start(Service *s) { /* We want to ensure that nobody leaks processes from * START_PRE here, so let's go on a killing spree, People * should not spawn long running processes from START_PRE. */ - cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, SIGKILL, true, NULL); + cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, SIGKILL, true, NULL, "control"); if (s->type == SERVICE_FORKING) { s->control_command_id = SERVICE_EXEC_START; @@ -2108,15 +2117,17 @@ static void service_enter_start(Service *s) { c = s->main_command = s->exec_command[SERVICE_EXEC_START]; } - if ((r = service_spawn(s, - c, - s->type == SERVICE_FORKING || s->type == SERVICE_DBUS || s->type == SERVICE_NOTIFY, - true, - true, - true, - true, - s->notify_access != NOTIFY_NONE, - &pid)) < 0) + r = service_spawn(s, + c, + s->type == SERVICE_FORKING || s->type == SERVICE_DBUS || s->type == SERVICE_NOTIFY, + true, + true, + true, + true, + s->notify_access != NOTIFY_NONE, + false, + &pid); + if (r < 0) goto fail; if (s->type == SERVICE_SIMPLE) { @@ -2168,19 +2179,21 @@ static void service_enter_start_pre(Service *s) { /* Before we start anything, let's clear up what might * be left from previous runs. */ - cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, SIGKILL, true, NULL); + cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, SIGKILL, true, NULL, "control"); s->control_command_id = SERVICE_EXEC_START_PRE; - if ((r = service_spawn(s, - s->control_command, - true, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - true, - false, - &s->control_pid)) < 0) + r = service_spawn(s, + s->control_command, + true, + false, + !s->permissions_start_only, + !s->root_directory_start_only, + true, + false, + true, + &s->control_pid); + if (r < 0) goto fail; service_set_state(s, SERVICE_START_PRE); @@ -2236,15 +2249,17 @@ static void service_enter_reload(Service *s) { if ((s->control_command = s->exec_command[SERVICE_EXEC_RELOAD])) { s->control_command_id = SERVICE_EXEC_RELOAD; - if ((r = service_spawn(s, - s->control_command, - true, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - false, - false, - &s->control_pid)) < 0) + r = service_spawn(s, + s->control_command, + true, + false, + !s->permissions_start_only, + !s->root_directory_start_only, + false, + false, + true, + &s->control_pid); + if (r < 0) goto fail; service_set_state(s, SERVICE_RELOAD); @@ -2271,16 +2286,18 @@ static void service_run_next_control(Service *s) { s->control_command = s->control_command->command_next; service_unwatch_control_pid(s); - if ((r = service_spawn(s, - s->control_command, - true, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - s->control_command_id == SERVICE_EXEC_START_PRE || - s->control_command_id == SERVICE_EXEC_STOP_POST, - false, - &s->control_pid)) < 0) + r = service_spawn(s, + s->control_command, + true, + false, + !s->permissions_start_only, + !s->root_directory_start_only, + s->control_command_id == SERVICE_EXEC_START_PRE || + s->control_command_id == SERVICE_EXEC_STOP_POST, + false, + true, + &s->control_pid); + if (r < 0) goto fail; return; @@ -2313,15 +2330,17 @@ static void service_run_next_main(Service *s) { s->main_command = s->main_command->command_next; service_unwatch_main_pid(s); - if ((r = service_spawn(s, - s->main_command, - false, - true, - true, - true, - true, - s->notify_access != NOTIFY_NONE, - &pid)) < 0) + r = service_spawn(s, + s->main_command, + false, + true, + true, + true, + true, + s->notify_access != NOTIFY_NONE, + false, + &pid); + if (r < 0) goto fail; service_set_main_pid(s, pid); @@ -3647,8 +3666,8 @@ static int service_kill(Unit *u, KillWho who, KillMode mode, int signo, DBusErro r = q; goto finish; } - - if ((q = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, signo, false, pid_set)) < 0) + q = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, signo, false, pid_set, NULL); + if (q < 0) if (q != -EAGAIN && q != -ESRCH && q != -ENOENT) r = q; } diff --git a/src/core/socket.c b/src/core/socket.c index 37a023615..a43971765 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1150,6 +1150,7 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { UNIT(s)->manager->confirm_spawn, UNIT(s)->cgroup_bondings, UNIT(s)->cgroup_attributes, + NULL, &pid); strv_free(argv); @@ -1240,7 +1241,8 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) { if ((r = set_put(pid_set, LONG_TO_PTR(s->control_pid))) < 0) goto fail; - if ((r = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, sig, true, pid_set)) < 0) { + r = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, sig, true, pid_set, NULL); + if (r < 0) { if (r != -EAGAIN && r != -ESRCH && r != -ENOENT) log_warning("Failed to kill control group: %s", strerror(-r)); } else if (r > 0) @@ -2127,7 +2129,8 @@ static int socket_kill(Unit *u, KillWho who, KillMode mode, int signo, DBusError goto finish; } - if ((q = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, signo, false, pid_set)) < 0) + q = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, signo, false, pid_set, NULL); + if (q < 0) if (q != -EAGAIN && q != -ESRCH && q != -ENOENT) r = q; } diff --git a/src/core/swap.c b/src/core/swap.c index 6331864d8..363852d33 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -621,6 +621,7 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { UNIT(s)->manager->confirm_spawn, UNIT(s)->cgroup_bondings, UNIT(s)->cgroup_attributes, + NULL, &pid)) < 0) goto fail; @@ -690,7 +691,8 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { if ((r = set_put(pid_set, LONG_TO_PTR(s->control_pid))) < 0) goto fail; - if ((r = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, sig, true, pid_set)) < 0) { + r = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, sig, true, pid_set, NULL); + if (r < 0) { if (r != -EAGAIN && r != -ESRCH && r != -ENOENT) log_warning("Failed to kill control group: %s", strerror(-r)); } else if (r > 0) @@ -1321,7 +1323,8 @@ static int swap_kill(Unit *u, KillWho who, KillMode mode, int signo, DBusError * goto finish; } - if ((q = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, signo, false, pid_set)) < 0) + q = cgroup_bonding_kill_list(UNIT(s)->cgroup_bondings, signo, false, pid_set, NULL); + if (q < 0) if (q != -EAGAIN && q != -ESRCH && q != -ENOENT) r = q; } -- cgit v1.2.3