From 76583ef261c65feb6059680f95693ee582406c90 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Oct 2015 17:56:12 +0100 Subject: core: don't pass uninitialzed PIDs to pid_is_unwaited() Since 5fd9b2c5467b0a42ccdabc7eb8e516d512609a8e passing a pid of 0 to pid_is_unwaited() and pid_is_live() is considered as a request on the current process, similar how the other calls in process-util.c handle a PID of 0. This broke service.c, which passes a 0 PID and expects it to be considered an unwaited process. This fix make sure we can boot again. --- src/core/service.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 31a0d3aebe..a7725c52a0 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -940,7 +940,8 @@ static int service_coldplug(Unit *u) { return r; } - if (pid_is_unwaited(s->main_pid) && + if (s->main_pid > 0 && + pid_is_unwaited(s->main_pid) && ((s->deserialized_state == SERVICE_START && IN_SET(s->type, SERVICE_FORKING, SERVICE_DBUS, SERVICE_ONESHOT, SERVICE_NOTIFY)) || IN_SET(s->deserialized_state, SERVICE_START, SERVICE_START_POST, @@ -952,7 +953,8 @@ static int service_coldplug(Unit *u) { return r; } - if (pid_is_unwaited(s->control_pid) && + if (s->control_pid > 0 && + pid_is_unwaited(s->control_pid) && IN_SET(s->deserialized_state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD, -- cgit v1.2.3-54-g00ecf From c386f5886466de6022b3b4b1c8ac8df72871fbc7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Oct 2015 17:59:45 +0100 Subject: core: all unit types that watch control PIDs should use the same logic When coldplugging the unit state, make sure to follow the same basic logic for all unit types: always verify whether the control PID is still a waitable process before proceeding. --- src/core/busname.c | 7 +++---- src/core/mount.c | 26 +++++++++++++------------- src/core/socket.c | 7 +++---- src/core/swap.c | 20 ++++++++++---------- 4 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/core/busname.c b/src/core/busname.c index 3592f72fe5..68508e20d2 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -364,10 +364,9 @@ static int busname_coldplug(Unit *u) { if (n->deserialized_state == n->state) return 0; - if (IN_SET(n->deserialized_state, BUSNAME_MAKING, BUSNAME_SIGTERM, BUSNAME_SIGKILL)) { - - if (n->control_pid <= 0) - return -EBADMSG; + if (n->control_pid > 0 && + pid_is_unwaited(n->control_pid) && + IN_SET(n->deserialized_state, BUSNAME_MAKING, BUSNAME_SIGTERM, BUSNAME_SIGKILL)) { r = unit_watch_pid(UNIT(n), n->control_pid); if (r < 0) diff --git a/src/core/mount.c b/src/core/mount.c index 2761f632bd..950d5d76d5 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -632,19 +632,19 @@ static int mount_coldplug(Unit *u) { if (new_state == m->state) return 0; - if (new_state == MOUNT_MOUNTING || - new_state == MOUNT_MOUNTING_DONE || - new_state == MOUNT_REMOUNTING || - new_state == MOUNT_UNMOUNTING || - new_state == MOUNT_MOUNTING_SIGTERM || - new_state == MOUNT_MOUNTING_SIGKILL || - new_state == MOUNT_UNMOUNTING_SIGTERM || - new_state == MOUNT_UNMOUNTING_SIGKILL || - new_state == MOUNT_REMOUNTING_SIGTERM || - new_state == MOUNT_REMOUNTING_SIGKILL) { - - if (m->control_pid <= 0) - return -EBADMSG; + if (m->control_pid > 0 && + pid_is_unwaited(m->control_pid) && + IN_SET(new_state, + MOUNT_MOUNTING, + MOUNT_MOUNTING_DONE, + MOUNT_REMOUNTING, + MOUNT_UNMOUNTING, + MOUNT_MOUNTING_SIGTERM, + MOUNT_MOUNTING_SIGKILL, + MOUNT_UNMOUNTING_SIGTERM, + MOUNT_UNMOUNTING_SIGKILL, + MOUNT_REMOUNTING_SIGTERM, + MOUNT_REMOUNTING_SIGKILL)) { r = unit_watch_pid(UNIT(m), m->control_pid); if (r < 0) diff --git a/src/core/socket.c b/src/core/socket.c index f62466c6a0..3c7f972fbc 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1455,7 +1455,9 @@ static int socket_coldplug(Unit *u) { if (s->deserialized_state == s->state) return 0; - if (IN_SET(s->deserialized_state, + if (s->control_pid > 0 && + pid_is_unwaited(s->control_pid) && + IN_SET(s->deserialized_state, SOCKET_START_PRE, SOCKET_START_CHOWN, SOCKET_START_POST, @@ -1466,9 +1468,6 @@ static int socket_coldplug(Unit *u) { SOCKET_FINAL_SIGTERM, SOCKET_FINAL_SIGKILL)) { - if (s->control_pid <= 0) - return -EBADMSG; - r = unit_watch_pid(UNIT(s), s->control_pid); if (r < 0) return r; diff --git a/src/core/swap.c b/src/core/swap.c index 6eff6ffb4c..f626ea4d87 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -528,16 +528,16 @@ static int swap_coldplug(Unit *u) { if (new_state == s->state) return 0; - if (new_state == SWAP_ACTIVATING || - new_state == SWAP_ACTIVATING_SIGTERM || - new_state == SWAP_ACTIVATING_SIGKILL || - new_state == SWAP_ACTIVATING_DONE || - new_state == SWAP_DEACTIVATING || - new_state == SWAP_DEACTIVATING_SIGTERM || - new_state == SWAP_DEACTIVATING_SIGKILL) { - - if (s->control_pid <= 0) - return -EBADMSG; + if (s->control_pid > 0 && + pid_is_unwaited(s->control_pid) && + IN_SET(new_state, + SWAP_ACTIVATING, + SWAP_ACTIVATING_SIGTERM, + SWAP_ACTIVATING_SIGKILL, + SWAP_ACTIVATING_DONE, + SWAP_DEACTIVATING, + SWAP_DEACTIVATING_SIGTERM, + SWAP_DEACTIVATING_SIGKILL)) { r = unit_watch_pid(UNIT(s), s->control_pid); if (r < 0) -- cgit v1.2.3-54-g00ecf From 930d2838f206d725d890c78475312910cd16329a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Oct 2015 18:02:24 +0100 Subject: service: exiting early is nicer than deeply-indented code blocks --- src/core/service.c | 97 +++++++++++++++++++++++++++--------------------------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index a7725c52a0..bafb532e1e 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -912,68 +912,67 @@ static int service_coldplug(Unit *u) { assert(s); assert(s->state == SERVICE_DEAD); - if (s->deserialized_state != s->state) { - - if (IN_SET(s->deserialized_state, - SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, - SERVICE_RELOAD, - SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, - SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) { - - usec_t k; + if (s->deserialized_state == s->state) + return 0; - k = IN_SET(s->deserialized_state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD) ? s->timeout_start_usec : s->timeout_stop_usec; + if (IN_SET(s->deserialized_state, + SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, + SERVICE_RELOAD, + SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, + SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) { - /* For the start/stop timeouts 0 means off */ - if (k > 0) { - r = service_arm_timer(s, k); - if (r < 0) - return r; - } - } + usec_t k; - if (s->deserialized_state == SERVICE_AUTO_RESTART) { + k = IN_SET(s->deserialized_state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD) ? s->timeout_start_usec : s->timeout_stop_usec; - /* The restart timeouts 0 means immediately */ - r = service_arm_timer(s, s->restart_usec); - if (r < 0) - return r; - } - - if (s->main_pid > 0 && - pid_is_unwaited(s->main_pid) && - ((s->deserialized_state == SERVICE_START && IN_SET(s->type, SERVICE_FORKING, SERVICE_DBUS, SERVICE_ONESHOT, SERVICE_NOTIFY)) || - IN_SET(s->deserialized_state, - SERVICE_START, SERVICE_START_POST, - SERVICE_RUNNING, SERVICE_RELOAD, - SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, - SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL))) { - r = unit_watch_pid(UNIT(s), s->main_pid); + /* For the start/stop timeouts 0 means off */ + if (k > 0) { + r = service_arm_timer(s, k); if (r < 0) return r; } + } - if (s->control_pid > 0 && - pid_is_unwaited(s->control_pid) && - IN_SET(s->deserialized_state, - SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, - SERVICE_RELOAD, - SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, - SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) { - r = unit_watch_pid(UNIT(s), s->control_pid); - if (r < 0) - return r; - } + if (s->deserialized_state == SERVICE_AUTO_RESTART) { - if (!IN_SET(s->deserialized_state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) - unit_watch_all_pids(UNIT(s)); + /* The restart timeouts 0 means immediately */ + r = service_arm_timer(s, s->restart_usec); + if (r < 0) + return r; + } - if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) - service_start_watchdog(s); + if (s->main_pid > 0 && + pid_is_unwaited(s->main_pid) && + ((s->deserialized_state == SERVICE_START && IN_SET(s->type, SERVICE_FORKING, SERVICE_DBUS, SERVICE_ONESHOT, SERVICE_NOTIFY)) || + IN_SET(s->deserialized_state, + SERVICE_START, SERVICE_START_POST, + SERVICE_RUNNING, SERVICE_RELOAD, + SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, + SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL))) { + r = unit_watch_pid(UNIT(s), s->main_pid); + if (r < 0) + return r; + } - service_set_state(s, s->deserialized_state); + if (s->control_pid > 0 && + pid_is_unwaited(s->control_pid) && + IN_SET(s->deserialized_state, + SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, + SERVICE_RELOAD, + SERVICE_STOP, SERVICE_STOP_SIGABRT, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, + SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) { + r = unit_watch_pid(UNIT(s), s->control_pid); + if (r < 0) + return r; } + if (!IN_SET(s->deserialized_state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) + unit_watch_all_pids(UNIT(s)); + + if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) + service_start_watchdog(s); + + service_set_state(s, s->deserialized_state); return 0; } -- cgit v1.2.3-54-g00ecf