ryantimwilson / rpms / systemd

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