ryantimwilson / rpms / systemd

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