daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
Brian Stinson 2593d8
From 789806ac06bb13d1b579fef47dbb85f224b6dbb1 Mon Sep 17 00:00:00 2001
Brian Stinson 2593d8
From: Lennart Poettering <lennart@poettering.net>
Brian Stinson 2593d8
Date: Thu, 14 Mar 2019 17:19:30 +0100
Brian Stinson 2593d8
Subject: [PATCH] core: change ownership/mode of the execution directories also
Brian Stinson 2593d8
 for static users
Brian Stinson 2593d8
Brian Stinson 2593d8
It's probably unexpected if we do a recursive chown() when dynamic users
Brian Stinson 2593d8
are used but not on static users.
Brian Stinson 2593d8
Brian Stinson 2593d8
hence, let's tweak the logic slightly, and recursively chown in both
Brian Stinson 2593d8
cases, except when operating on the configuration directory.
Brian Stinson 2593d8
Brian Stinson 2593d8
Fixes: #11842
Brian Stinson 2593d8
(cherry picked from commit 206e9864de460dd79d9edd7bedb47dee168765e1)
Brian Stinson 2593d8
Brian Stinson 2593d8
Resolves: #1778384
Brian Stinson 2593d8
---
Brian Stinson 2593d8
 src/core/execute.c | 47 +++++++++++++++++++++++++---------------------
Brian Stinson 2593d8
 1 file changed, 26 insertions(+), 21 deletions(-)
Brian Stinson 2593d8
Brian Stinson 2593d8
diff --git a/src/core/execute.c b/src/core/execute.c
Brian Stinson 2593d8
index 46aa733937..c42300a41e 100644
Brian Stinson 2593d8
--- a/src/core/execute.c
Brian Stinson 2593d8
+++ b/src/core/execute.c
Brian Stinson 2593d8
@@ -2090,37 +2090,42 @@ static int setup_exec_directory(
Brian Stinson 2593d8
                         if (r < 0)
Brian Stinson 2593d8
                                 goto fail;
Brian Stinson 2593d8
 
Brian Stinson 2593d8
-                        /* Lock down the access mode */
Brian Stinson 2593d8
-                        if (chmod(pp, context->directories[type].mode) < 0) {
Brian Stinson 2593d8
-                                r = -errno;
Brian Stinson 2593d8
-                                goto fail;
Brian Stinson 2593d8
-                        }
Brian Stinson 2593d8
                 } else {
Brian Stinson 2593d8
                         r = mkdir_label(p, context->directories[type].mode);
Brian Stinson 2593d8
                         if (r < 0) {
Brian Stinson 2593d8
-                                struct stat st;
Brian Stinson 2593d8
-
Brian Stinson 2593d8
                                 if (r != -EEXIST)
Brian Stinson 2593d8
                                         goto fail;
Brian Stinson 2593d8
 
Brian Stinson 2593d8
-                                if (stat(p, &st) < 0) {
Brian Stinson 2593d8
-                                        r = -errno;
Brian Stinson 2593d8
-                                        goto fail;
Brian Stinson 2593d8
-                                }
Brian Stinson 2593d8
-                                if (((st.st_mode ^ context->directories[type].mode) & 07777) != 0)
Brian Stinson 2593d8
-                                        log_warning("%s \'%s\' already exists but the mode is different. "
Brian Stinson 2593d8
-                                                    "(filesystem: %o %sMode: %o)",
Brian Stinson 2593d8
-                                                    exec_directory_type_to_string(type), *rt,
Brian Stinson 2593d8
-                                                    st.st_mode & 07777, exec_directory_type_to_string(type), context->directories[type].mode & 07777);
Brian Stinson 2593d8
-                                if (!context->dynamic_user)
Brian Stinson 2593d8
+                                if (type == EXEC_DIRECTORY_CONFIGURATION) {
Brian Stinson 2593d8
+                                        struct stat st;
Brian Stinson 2593d8
+
Brian Stinson 2593d8
+                                        /* Don't change the owner/access mode of the configuration directory,
Brian Stinson 2593d8
+                                         * as in the common case it is not written to by a service, and shall
Brian Stinson 2593d8
+                                         * not be writable. */
Brian Stinson 2593d8
+
Brian Stinson 2593d8
+                                        if (stat(p, &st) < 0) {
Brian Stinson 2593d8
+                                                r = -errno;
Brian Stinson 2593d8
+                                                goto fail;
Brian Stinson 2593d8
+                                        }
Brian Stinson 2593d8
+
Brian Stinson 2593d8
+                                        /* Still complain if the access mode doesn't match */
Brian Stinson 2593d8
+                                        if (((st.st_mode ^ context->directories[type].mode) & 07777) != 0)
Brian Stinson 2593d8
+                                                log_warning("%s \'%s\' already exists but the mode is different. "
Brian Stinson 2593d8
+                                                            "(File system: %o %sMode: %o)",
Brian Stinson 2593d8
+                                                            exec_directory_type_to_string(type), *rt,
Brian Stinson 2593d8
+                                                            st.st_mode & 07777, exec_directory_type_to_string(type), context->directories[type].mode & 07777);
Brian Stinson 2593d8
+
Brian Stinson 2593d8
                                         continue;
Brian Stinson 2593d8
+                                }
Brian Stinson 2593d8
                         }
Brian Stinson 2593d8
                 }
Brian Stinson 2593d8
 
Brian Stinson 2593d8
-                /* Don't change the owner of the configuration directory, as in the common case it is not written to by
Brian Stinson 2593d8
-                 * a service, and shall not be writable. */
Brian Stinson 2593d8
-                if (type == EXEC_DIRECTORY_CONFIGURATION)
Brian Stinson 2593d8
-                        continue;
Brian Stinson 2593d8
+                /* Lock down the access mode (we use chmod_and_chown() to make this idempotent. We don't
Brian Stinson 2593d8
+                 * specifiy UID/GID here, so that path_chown_recursive() can optimize things depending on the
Brian Stinson 2593d8
+                 * current UID/GID ownership.) */
Brian Stinson 2593d8
+                r = chmod_and_chown(pp ?: p, context->directories[type].mode, UID_INVALID, GID_INVALID);
Brian Stinson 2593d8
+                if (r < 0)
Brian Stinson 2593d8
+                        goto fail;
Brian Stinson 2593d8
 
Brian Stinson 2593d8
                 /* Then, change the ownership of the whole tree, if necessary */
Brian Stinson 2593d8
                 r = path_chown_recursive(pp ?: p, uid, gid);