From: Zbigniew Jędrzejewski-Szmek Date: Thu, 18 Apr 2024 14:11:06 +0000 (+0200) Subject: Try path without sbin even if compiled with split-bin=true X-Git-Tag: v256-rc1~80^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f36a4c897ff53eb0be3bd3728a3ff91e9c0664d;p=thirdparty%2Fsystemd.git 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 4d335c64eed..5272477c2ac 100644 --- 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 6d88c54d7f0..62a012d39f0 100644 --- 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 3ffe0f87c0c..b627e7abc08 100644 --- 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 85bca9fd3b7..f0be9f1e4bd 100644 --- 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 a57786037bc..533c988a472 100644 --- 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 72b92b7a9aa..e02bd8c8575 100644 --- 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) {