From 3a5d9b71f45e2e3a86b8712fd3b05d2127469c56 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sep 29 2016 21:04:15 +0000 Subject: Better fix for the notify message issue --- diff --git a/0016-If-the-notification-message-length-is-0-ignore-the-m.patch b/0016-If-the-notification-message-length-is-0-ignore-the-m.patch index 470988d..0213889 100644 --- a/0016-If-the-notification-message-length-is-0-ignore-the-m.patch +++ b/0016-If-the-notification-message-length-is-0-ignore-the-m.patch @@ -1,4 +1,4 @@ -From d22ed4fb5c503575bc4d62b599d790ebce8cbab4 Mon Sep 17 00:00:00 2001 +From 2e9b525caa9e3126e54f0d9506d0c36d7d533997 Mon Sep 17 00:00:00 2001 From: Jorge Niedbalski Date: Wed, 28 Sep 2016 18:25:50 -0300 Subject: [PATCH] If the notification message length is 0, ignore the message @@ -7,6 +7,7 @@ Subject: [PATCH] If the notification message length is 0, ignore the message Fixes #4234. Signed-off-by: Jorge Niedbalski +(cherry picked from commit 531ac2b2349da02acc9c382849758e07eb92b020) --- src/core/manager.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/0017-pid1-don-t-return-any-error-in-manager_dispatch_noti.patch b/0017-pid1-don-t-return-any-error-in-manager_dispatch_noti.patch new file mode 100644 index 0000000..f0599cf --- /dev/null +++ b/0017-pid1-don-t-return-any-error-in-manager_dispatch_noti.patch @@ -0,0 +1,51 @@ +From 39e5e97e68a9c1bca3bcfa6c9316a83dad0b072d Mon Sep 17 00:00:00 2001 +From: Franck Bui +Date: Thu, 29 Sep 2016 19:44:34 +0200 +Subject: [PATCH] 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. +(cherry picked from commit 9987750e7a4c62e0eb8473603150596ba7c3a015) +--- + 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 a085ed899a..36488b673a 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; + } + if (n == 0) { + log_debug("Got zero-length notification message. Ignoring."); +@@ -1673,7 +1677,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; + } + } + +-- +2.9.0 + diff --git a/0018-pid1-process-zero-length-notification-messages-again.patch b/0018-pid1-process-zero-length-notification-messages-again.patch new file mode 100644 index 0000000..d602290 --- /dev/null +++ b/0018-pid1-process-zero-length-notification-messages-again.patch @@ -0,0 +1,80 @@ +From 9d77c48a80e1cc2ad016eba1756a5ca293d51f86 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= +Date: Thu, 29 Sep 2016 16:06:02 +0200 +Subject: [PATCH] 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(). + +(cherry picked from commit 8523bf7dd514a3a2c6114b7b8fb8f308b4f09fc4) +--- + 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 36488b673a..85bf858992 100644 +--- a/src/core/manager.c ++++ b/src/core/manager.c +@@ -1584,13 +1584,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) { +@@ -1652,10 +1651,6 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t + * example... */ + return 0; + } +- 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) { +@@ -1692,25 +1687,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; + } + +-- +2.9.0 + diff --git a/systemd.spec b/systemd.spec index ae4d383..9e53b9d 100644 --- a/systemd.spec +++ b/systemd.spec @@ -12,7 +12,7 @@ Name: systemd Url: http://www.freedesktop.org/wiki/Software/systemd Version: 231 -Release: 5%{?gitcommit:.git%{gitcommitshort}}%{?dist} +Release: 6%{?gitcommit:.git%{gitcommitshort}}%{?dist} # For a breakdown of the licensing, see README License: LGPLv2+ and MIT and GPLv2+ Summary: A System and Service Manager @@ -946,6 +946,9 @@ getent passwd systemd-journal-upload >/dev/null 2>&1 || useradd -r -l -g systemd %{_mandir}/man[1578]/systemd-nspawn.* %changelog +* Thu Sep 29 2016 Zbigniew Jędrzejewski-Szmek - 231-6 +- Better fix for (#1380286) + * Thu Sep 29 2016 Zbigniew Jędrzejewski-Szmek - 231-5 - Denial-of-service bug against pid1 (#1380286)