From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= Date: Thu, 18 Aug 2016 21:39:39 -0400 Subject: Revert "pid1: reconnect to the console before being re-executed" This reverts commit affd7ed1a923b0df8479cff1bd9eafb625fdaa66. > So it looks like make_console_stdio() has bad side effect. More specifically it > does a TIOCSCTTY ioctl (via acquire_terminal()) which sees to disturb the > process which was using/owning the console. Fixes #3842. https://bugs.debian.org/834367 https://bugzilla.redhat.com/show_bug.cgi?id=1367766 --- src/core/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index f2adca7..f59a55f 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2016,9 +2016,6 @@ finish: log_error_errno(r, "Failed to switch root, trying to continue: %m"); } - /* Reopen the console */ - (void) make_console_stdio(); - args_size = MAX(6, argc+1); args = newa(const char*, args_size); @@ -2066,6 +2063,9 @@ finish: arg_serialization = safe_fclose(arg_serialization); fds = fdset_free(fds); + /* Reopen the console */ + (void) make_console_stdio(); + for (j = 1, i = 1; j < (unsigned) argc; j++) args[i++] = argv[j]; args[i++] = NULL; From: Jorge Niedbalski Date: Wed, 28 Sep 2016 18:25:50 -0300 Subject: If the notification message length is 0, ignore the message (#4237) Fixes #4234. Signed-off-by: Jorge Niedbalski --- src/core/manager.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/manager.c b/src/core/manager.c index 02e8681..89bb465 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1649,6 +1649,10 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t return -errno; } + if (n == 0) { + log_debug("Got zero-length notification message. Ignoring."); + return 0; + } CMSG_FOREACH(cmsg, &msghdr) { if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) { From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= Date: Thu, 29 Sep 2016 16:06:02 +0200 Subject: pid1: process zero-length notification messages again This undoes 531ac2b234. I acked that patch without looking at the code carefully enough. There are two problems: - we want to process the fds anyway - in principle empty notification messages are valid, and we should process them as usual, including logging using log_unit_debug(). --- src/core/manager.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 89bb465..83ab0b6 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1585,13 +1585,12 @@ static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, ui return 0; } -static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, const char *buf, size_t n, FDSet *fds) { +static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, const char *buf, FDSet *fds) { _cleanup_strv_free_ char **tags = NULL; assert(m); assert(u); assert(buf); - assert(n > 0); tags = strv_split(buf, "\n\r"); if (!tags) { @@ -1649,10 +1648,6 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t return -errno; } - if (n == 0) { - log_debug("Got zero-length notification message. Ignoring."); - return 0; - } CMSG_FOREACH(cmsg, &msghdr) { if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) { @@ -1688,25 +1683,27 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t return 0; } + /* The message should be a string. Here we make sure it's NUL-terminated, + * but only the part until first NUL will be used anyway. */ buf[n] = 0; /* Notify every unit that might be interested, but try * to avoid notifying the same one multiple times. */ u1 = manager_get_unit_by_pid_cgroup(m, ucred->pid); if (u1) { - manager_invoke_notify_message(m, u1, ucred->pid, buf, n, fds); + manager_invoke_notify_message(m, u1, ucred->pid, buf, fds); found = true; } u2 = hashmap_get(m->watch_pids1, PID_TO_PTR(ucred->pid)); if (u2 && u2 != u1) { - manager_invoke_notify_message(m, u2, ucred->pid, buf, n, fds); + manager_invoke_notify_message(m, u2, ucred->pid, buf, fds); found = true; } u3 = hashmap_get(m->watch_pids2, PID_TO_PTR(ucred->pid)); if (u3 && u3 != u2 && u3 != u1) { - manager_invoke_notify_message(m, u3, ucred->pid, buf, n, fds); + manager_invoke_notify_message(m, u3, ucred->pid, buf, fds); found = true; } From: Franck Bui Date: Thu, 29 Sep 2016 19:44:34 +0200 Subject: pid1: don't return any error in manager_dispatch_notify_fd() (#4240) If manager_dispatch_notify_fd() fails and returns an error then the handling of service notifications will be disabled entirely leading to a compromised system. For example pid1 won't be able to receive the WATCHDOG messages anymore and will kill all services supposed to send such messages. --- src/core/manager.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 83ab0b6..5225190 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1643,10 +1643,14 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t n = recvmsg(m->notify_fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC); if (n < 0) { - if (errno == EAGAIN || errno == EINTR) - return 0; + if (!IN_SET(errno, EAGAIN, EINTR)) + log_error("Failed to receive notification message: %m"); - return -errno; + /* It's not an option to return an error here since it + * would disable the notification handler entirely. Services + * wouldn't be able to send the WATCHDOG message for + * example... */ + return 0; } CMSG_FOREACH(cmsg, &msghdr) { @@ -1669,7 +1673,8 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r = fdset_new_array(&fds, fd_array, n_fds); if (r < 0) { close_many(fd_array, n_fds); - return log_oom(); + log_oom(); + return 0; } }