Try path without sbin even if compiled with split-bin=true

I'm working on the transition to merged sbin in Fedora. While the transition is
happening (and probably for a while after), we need to compile systemd with
split-bin=true to support systems upgraded from previous versions. But when the
system has been upgraded and already has /usr/sbin that is a symlink, be nice
and give $PATH without sbin.

We check for both /usr/sbin and /usr/local/sbin. If either exists and is not a
symlink to ./bin, we retain previous behaviour. This means that if both are
converted, we get the same behaviour as split-bin=false, and otherwise we
get the same behaviour as before.

sd-path uses the same logic. This is not a hot path, so I got rid of the nulstr
macros that duplicated the logic.

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 4d335c64eeda287fa27c1ae2fe8b608b42ccb6f7..5272477c2ac8deef4b4ecf0a2d8901a6e16dbae7 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -684,7 +684,7 @@ int find_executable_full(
                  * binary. */
                 p = getenv("PATH");
         if (!p)
-                p = DEFAULT_PATH;
+                p = default_PATH();
 
         if (exec_search_path) {
                 STRV_FOREACH(element, exec_search_path) {
@@ -1441,3 +1441,31 @@ int path_glob_can_match(const char *pattern, const char *prefix, char **ret) {
                 *ret = NULL;
         return false;
 }
+
+const char* default_PATH(void) {
+#if HAVE_SPLIT_BIN
+        static int split = -1;
+        int r;
+
+        /* Check whether /usr/sbin is not a symlink and return the appropriate $PATH.
+         * On error fall back to the safe value with both directories as configured… */
+
+        if (split < 0)
+                STRV_FOREACH_PAIR(bin, sbin, STRV_MAKE("/usr/bin", "/usr/sbin",
+                                                       "/usr/local/bin", "/usr/local/sbin")) {
+                        r = inode_same(*bin, *sbin, AT_NO_AUTOMOUNT);
+                        if (r > 0 || r == -ENOENT)
+                                continue;
+                        if (r < 0)
+                                log_debug_errno(r, "Failed to compare \"%s\" and \"%s\", using compat $PATH: %m",
+                                                *bin, *sbin);
+                        split = true;
+                        break;
+                }
+        if (split < 0)
+                split = false;
+        if (split)
+                return DEFAULT_PATH_WITH_SBIN;
+#endif
+        return DEFAULT_PATH_WITHOUT_SBIN;
+}

diff --git a/src/basic/path-util.h b/src/basic/path-util.h
index 6d88c54d7f0bdad66912d6f9f813ce49e85b7814..62a012d39f02ed968a55cd7f1e522a5c388b4266 100644 (file)
--- a/src/basic/path-util.h
+++ b/src/basic/path-util.h
@@ -11,27 +11,26 @@
 #include "strv.h"
 #include "time-util.h"
 
-#define PATH_SPLIT_SBIN_BIN(x) x "sbin:" x "bin"
-#define PATH_SPLIT_SBIN_BIN_NULSTR(x) x "sbin\0" x "bin\0"
+#define PATH_SPLIT_BIN(x) x "sbin:" x "bin"
+#define PATH_SPLIT_BIN_NULSTR(x) x "sbin\0" x "bin\0"
 
-#define PATH_NORMAL_SBIN_BIN(x) x "bin"
-#define PATH_NORMAL_SBIN_BIN_NULSTR(x) x "bin\0"
+#define PATH_MERGED_BIN(x) x "bin"
+#define PATH_MERGED_BIN_NULSTR(x) x "bin\0"
 
-#if HAVE_SPLIT_BIN
-#  define PATH_SBIN_BIN(x) PATH_SPLIT_SBIN_BIN(x)
-#  define PATH_SBIN_BIN_NULSTR(x) PATH_SPLIT_SBIN_BIN_NULSTR(x)
-#else
-#  define PATH_SBIN_BIN(x) PATH_NORMAL_SBIN_BIN(x)
-#  define PATH_SBIN_BIN_NULSTR(x) PATH_NORMAL_SBIN_BIN_NULSTR(x)
-#endif
+#define DEFAULT_PATH_WITH_SBIN PATH_SPLIT_BIN("/usr/local/") ":" PATH_SPLIT_BIN("/usr/")
+#define DEFAULT_PATH_WITHOUT_SBIN PATH_MERGED_BIN("/usr/local/") ":" PATH_MERGED_BIN("/usr/")
+
+#define DEFAULT_PATH_COMPAT PATH_SPLIT_BIN("/usr/local/") ":" PATH_SPLIT_BIN("/usr/") ":" PATH_SPLIT_BIN("/")
 
-#define DEFAULT_PATH PATH_SBIN_BIN("/usr/local/") ":" PATH_SBIN_BIN("/usr/")
-#define DEFAULT_PATH_NULSTR PATH_SBIN_BIN_NULSTR("/usr/local/") PATH_SBIN_BIN_NULSTR("/usr/")
-#define DEFAULT_PATH_COMPAT PATH_SPLIT_SBIN_BIN("/usr/local/") ":" PATH_SPLIT_SBIN_BIN("/usr/") ":" PATH_SPLIT_SBIN_BIN("/")
+const char* default_PATH(void);
 
-#ifndef DEFAULT_USER_PATH
-#  define DEFAULT_USER_PATH DEFAULT_PATH
+static inline const char* default_user_PATH(void) {
+#ifdef DEFAULT_USER_PATH
+        return DEFAULT_USER_PATH;
+#else
+        return default_PATH();
 #endif
+}
 
 static inline bool is_path(const char *p) {
         if (!p) /* A NULL pointer is definitely not a path */

diff --git a/src/core/manager.c b/src/core/manager.c
index 3ffe0f87c0cf3dd8255fbcd794fb68a720bd0559..b627e7abc088c7f5f36288d0fcfbf5a1cf24ee15 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -658,8 +658,6 @@ static char** sanitize_environment(char **l) {
 }
 
 int manager_default_environment(Manager *m) {
-        int r;
-
         assert(m);
 
         m->transient_environment = strv_free(m->transient_environment);
@@ -670,8 +668,11 @@ int manager_default_environment(Manager *m) {
                  *
                  * The initial passed environment is untouched to keep /proc/self/environ valid; it is used
                  * for tagging the init process inside containers. */
-                m->transient_environment = strv_new("PATH=" DEFAULT_PATH);
-                if (!m->transient_environment)
+                char *path = strjoin("PATH=", default_PATH());
+                if (!path)
+                        return log_oom();
+
+                if (strv_consume(&m->transient_environment, path) < 0)
                         return log_oom();
 
                 /* Import locale variables LC_*= from configuration */
@@ -684,8 +685,11 @@ int manager_default_environment(Manager *m) {
                 if (!m->transient_environment)
                         return log_oom();
 
-                r = strv_env_replace_strdup(&m->transient_environment, "PATH=" DEFAULT_USER_PATH);
-                if (r < 0)
+                char *path = strjoin("PATH=", default_user_PATH());
+                if (!path)
+                        return log_oom();
+
+                if (strv_env_replace_consume(&m->transient_environment, path) < 0)
                         return log_oom();
 
                 /* Envvars set for our 'manager' class session are private and should not be propagated

diff --git a/src/libsystemd/sd-path/sd-path.c b/src/libsystemd/sd-path/sd-path.c
index 85bca9fd3b758ce2bad59cc4c1e45d22d733adce..f0be9f1e4bd722b401db13a1e8e40c0f13a7b8f2 100644 (file)
--- a/src/libsystemd/sd-path/sd-path.c
+++ b/src/libsystemd/sd-path/sd-path.c
@@ -591,8 +591,14 @@ static int get_search(uint64_t type, char ***list) {
                                                "/etc",
                                                NULL);
 
-        case SD_PATH_SEARCH_BINARIES_DEFAULT:
-                return strv_from_nulstr(list, DEFAULT_PATH_NULSTR);
+        case SD_PATH_SEARCH_BINARIES_DEFAULT: {
+                char **t = strv_split(default_PATH(), ":");
+                if (!t)
+                        return -ENOMEM;
+
+                *list = t;
+                return 0;
+        }
 
         case SD_PATH_SYSTEMD_SEARCH_SYSTEM_UNIT:
         case SD_PATH_SYSTEMD_SEARCH_USER_UNIT: {

diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c
index a57786037bc3fd715c6dcbba05aa3de3fa7d7965..533c988a4720f4d6183f8c5430e5df0279790e03 100644 (file)
--- a/src/test/test-exec-util.c
+++ b/src/test/test-exec-util.c
@@ -331,7 +331,7 @@ TEST(environment_gathering) {
         assert_se(chmod(name3, 0755) == 0);
 
         /* When booting in containers or without initrd there might not be any PATH in the environment and if
-         * there is no PATH /bin/sh built-in PATH may leak and override systemd's DEFAULT_PATH which is not
+         * there is no PATH /bin/sh built-in PATH may leak and override systemd's default path which is not
          * good. Force our own PATH in environment, to prevent expansion of sh built-in $PATH */
         old = getenv("PATH");
         r = setenv("PATH", "no-sh-built-in-path", 1);
@@ -351,10 +351,9 @@ TEST(environment_gathering) {
         ASSERT_STREQ(strv_env_get(env, "C"), "001");
         ASSERT_STREQ(strv_env_get(env, "PATH"), "no-sh-built-in-path:/no/such/file");
 
-        /* now retest with "default" path passed in, as created by
-         * manager_default_environment */
+        /* Now retest with some "default" path passed. */
         env = strv_free(env);
-        env = strv_new("PATH=" DEFAULT_PATH);
+        env = strv_new("PATH=" DEFAULT_PATH_WITHOUT_SBIN);
         assert_se(env);
 
         r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, env, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS);
@@ -366,7 +365,7 @@ TEST(environment_gathering) {
         ASSERT_STREQ(strv_env_get(env, "A"), "22:23:24");
         ASSERT_STREQ(strv_env_get(env, "B"), "12");
         ASSERT_STREQ(strv_env_get(env, "C"), "001");
-        ASSERT_STREQ(strv_env_get(env, "PATH"), DEFAULT_PATH ":/no/such/file");
+        ASSERT_STREQ(strv_env_get(env, "PATH"), DEFAULT_PATH_WITHOUT_SBIN ":/no/such/file");
 
         /* reset environ PATH */
         assert_se(set_unset_env("PATH", old, true) == 0);

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 72b92b7a9aaf2fd02c6e27288246fe7e5a11f68d..e02bd8c857553b4340b698060f6f1b605cd4f49a 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -18,8 +18,8 @@
 #include "tmpfile-util.h"
 
 TEST(print_paths) {
-        log_info("DEFAULT_PATH=%s", DEFAULT_PATH);
-        log_info("DEFAULT_USER_PATH=%s", DEFAULT_USER_PATH);
+        log_info("default system PATH: %s", default_PATH());
+        log_info("default user PATH: %s", default_user_PATH());
 }
 
 TEST(path) {