Pablo Greco 48fc63
From 625b30e09d0090f81722aa8f02e7057839dfcf4f Mon Sep 17 00:00:00 2001
Pablo Greco 48fc63
From: Lennart Poettering <lennart@poettering.net>
Pablo Greco 48fc63
Date: Fri, 9 Feb 2018 17:05:17 +0100
Pablo Greco 48fc63
Subject: [PATCH] service: relax PID file symlink chain checks a bit (#8133)
Pablo Greco 48fc63
Pablo Greco 48fc63
Let's read the PID file after all if there's a potentially unsafe
Pablo Greco 48fc63
symlink chain in place. But if we do, then refuse taking the PID if its
Pablo Greco 48fc63
outside of the cgroup.
Pablo Greco 48fc63
Pablo Greco 48fc63
Fixes: #8085
Pablo Greco 48fc63
Pablo Greco 48fc63
(cherry picked from commit 73969ab61c39357e6892747e43307fbf07cafbed)
Pablo Greco 48fc63
(cherry picked from commit ce87ed7b47c61e649a0f9da39d272631b9524740)
Pablo Greco 48fc63
Pablo Greco 48fc63
Resolves: #1729414
Pablo Greco 48fc63
---
Pablo Greco 48fc63
 src/core/service.c | 15 +++++++++++++--
Pablo Greco 48fc63
 1 file changed, 13 insertions(+), 2 deletions(-)
Pablo Greco 48fc63
Pablo Greco 48fc63
diff --git a/src/core/service.c b/src/core/service.c
Pablo Greco 48fc63
index eaa588863f..6b61ccac18 100644
Pablo Greco 48fc63
--- a/src/core/service.c
Pablo Greco 48fc63
+++ b/src/core/service.c
Pablo Greco 48fc63
@@ -736,6 +736,7 @@ static int service_is_suitable_main_pid(Service *s, pid_t pid, int prio) {
Pablo Greco 48fc63
 
Pablo Greco 48fc63
 static int service_load_pid_file(Service *s, bool may_warn) {
Pablo Greco 48fc63
         char procfs[sizeof("/proc/self/fd/") - 1 + DECIMAL_STR_MAX(int)];
Pablo Greco 48fc63
+        bool questionable_pid_file = false;
Pablo Greco 48fc63
         _cleanup_free_ char *k = NULL;
Pablo Greco 48fc63
         _cleanup_close_ int fd = -1;
Pablo Greco 48fc63
         int r, prio;
Pablo Greco 48fc63
@@ -749,8 +750,13 @@ static int service_load_pid_file(Service *s, bool may_warn) {
Pablo Greco 48fc63
         prio = may_warn ? LOG_INFO : LOG_DEBUG;
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         fd = chase_symlinks(s->pid_file, NULL, CHASE_OPEN|CHASE_SAFE, NULL);
Pablo Greco 48fc63
-        if (fd == -EPERM)
Pablo Greco 48fc63
-                return log_unit_full_errno(UNIT(s)->id, prio, fd, "Permission denied while opening PID file or unsafe symlink chain: %s", s->pid_file);
Pablo Greco 48fc63
+        if (fd == -EPERM) {
Pablo Greco 48fc63
+                log_unit_full(UNIT(s)->id, LOG_DEBUG, "Permission denied while opening PID file or potentially unsafe symlink chain, will now retry with relaxed checks: %s", s->pid_file);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                questionable_pid_file = true;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                fd = chase_symlinks(s->pid_file, NULL, CHASE_OPEN, NULL);
Pablo Greco 48fc63
+        }
Pablo Greco 48fc63
         if (fd < 0)
Pablo Greco 48fc63
                 return log_unit_full_errno(UNIT(s)->id, prio, fd, "Can't open PID file %s (yet?) after %s: %m", s->pid_file, service_state_to_string(s->state));
Pablo Greco 48fc63
 
Pablo Greco 48fc63
@@ -773,6 +779,11 @@ static int service_load_pid_file(Service *s, bool may_warn) {
Pablo Greco 48fc63
         if (r == 0) {
Pablo Greco 48fc63
                 struct stat st;
Pablo Greco 48fc63
 
Pablo Greco 48fc63
+                if (questionable_pid_file) {
Pablo Greco 48fc63
+                        log_unit_error(UNIT(s)->id, "Refusing to accept PID outside of service control group, acquired through unsafe symlink chain: %s", s->pid_file);
Pablo Greco 48fc63
+                        return -EPERM;
Pablo Greco 48fc63
+                }
Pablo Greco 48fc63
+
Pablo Greco 48fc63
                 /* Hmm, it's not clear if the new main PID is safe. Let's allow this if the PID file is owned by root */
Pablo Greco 48fc63
 
Pablo Greco 48fc63
                 if (fstat(fd, &st) < 0)