Better fix for the notify message issue

This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2016-09-29 17:33:41 -04:00
parent bc76c3b9de
commit d550c96234
4 changed files with 157 additions and 2 deletions

View File

@ -0,0 +1,30 @@
From 3373cbd0846a89f27e09ae88e31912122f161112 Mon Sep 17 00:00:00 2001
From: Jorge Niedbalski <jorge.niedbalski@canonical.com>
Date: Wed, 28 Sep 2016 18:25:50 -0300
Subject: [PATCH] If the notification message length is 0, ignore the message
(#4237)
Fixes #4234.
Signed-off-by: Jorge Niedbalski <jnr@metaklass.org>
(cherry picked from commit 531ac2b2349da02acc9c382849758e07eb92b020)
(cherry picked from commit 2e9b525caa9e3126e54f0d9506d0c36d7d533997)
---
src/core/manager.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/core/manager.c b/src/core/manager.c
index f36cf5e320..6bd32ed920 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1557,6 +1557,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) {

View File

@ -0,0 +1,50 @@
From b16caba80a926b16a9a3377fbe856125436500df Mon Sep 17 00:00:00 2001
From: Franck Bui <fbui@suse.com>
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)
(cherry picked from commit 39e5e97e68a9c1bca3bcfa6c9316a83dad0b072d)
---
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 6bd32ed920..5092b1c5f6 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1552,10 +1552,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.");
@@ -1582,7 +1586,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;
}
}

View File

@ -0,0 +1,70 @@
From 7597de4208a828e4bdccaf848a00c290fc0ad957 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
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)
(cherry picked from commit 9d77c48a80e1cc2ad016eba1756a5ca293d51f86)
---
src/core/manager.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/core/manager.c b/src/core/manager.c
index 5092b1c5f6..369ed546b4 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1493,13 +1493,12 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) {
return n;
}
-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) {
@@ -1561,10 +1560,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) {
@@ -1607,19 +1602,19 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
* 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;
}

View File

@ -12,7 +12,7 @@
Name: systemd
Url: http://www.freedesktop.org/wiki/Software/systemd
Version: 229
Release: 14%{?gitcommit:.git%{gitcommitshort}}%{?dist}
Release: 15%{?gitcommit:.git%{gitcommitshort}}%{?dist}
# For a breakdown of the licensing, see README
License: LGPLv2+ and MIT and GPLv2+
Summary: A System and Service Manager
@ -80,7 +80,9 @@ Patch0044: 0044-networkd-add-route-expiration-handler-3242.patch
Patch0045: 0045-coredump-ignore-RLIMIT_CORE.patch
Patch0046: 0046-networkd-link-fix-handler-typo-for-route_remove-3433.patch
Patch0047: 0047-macros.systemd.in-add-systemd_ordering-3776.patch
Patch0048: If-the-notification-message-length-is-0-ignore-the-m.patch
Patch0048: 0037-If-the-notification-message-length-is-0-ignore-the-m.patch
Patch0049: 0038-pid1-don-t-return-any-error-in-manager_dispatch_noti.patch
Patch0050: 0039-pid1-process-zero-length-notification-messages-again.patch
Patch0999: 0999-resolved-create-etc-resolv.conf-symlink-at-runtime.patch
@ -965,6 +967,9 @@ getent passwd systemd-journal-upload >/dev/null 2>&1 || useradd -r -l -g systemd
/usr/lib/firewalld/services/*
%changelog
* Thu Sep 29 2016 Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> - 229-15
- Better fix for #1380286
* Thu Sep 29 2016 Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> - 229-14
- Denial-of-service bug against pid1 (#1380286)