Skip to content

Commit 0f36a4c

Browse files
keszybzkeszybz
committed
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.
1 parent 18d2641 commit 0f36a4c

File tree

6 files changed

+68
-32
lines changed

6 files changed

+68
-32
lines changed

‎src/basic/path-util.c‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ int find_executable_full(
684684
* binary. */
685685
p = getenv("PATH");
686686
if (!p)
687-
p = DEFAULT_PATH;
687+
p = default_PATH();
688688

689689
if (exec_search_path) {
690690
STRV_FOREACH(element, exec_search_path) {
@@ -1441,3 +1441,31 @@ int path_glob_can_match(const char *pattern, const char *prefix, char **ret) {
14411441
*ret = NULL;
14421442
return false;
14431443
}
1444+
1445+
const char* default_PATH(void) {
1446+
#if HAVE_SPLIT_BIN
1447+
static int split = -1;
1448+
int r;
1449+
1450+
/* Check whether /usr/sbin is not a symlink and return the appropriate $PATH.
1451+
* On error fall back to the safe value with both directories as configured… */
1452+
1453+
if (split < 0)
1454+
STRV_FOREACH_PAIR(bin, sbin, STRV_MAKE("/usr/bin", "/usr/sbin",
1455+
"/usr/local/bin", "/usr/local/sbin")) {
1456+
r = inode_same(*bin, *sbin, AT_NO_AUTOMOUNT);
1457+
if (r > 0 || r == -ENOENT)
1458+
continue;
1459+
if (r < 0)
1460+
log_debug_errno(r, "Failed to compare \"%s\" and \"%s\", using compat $PATH: %m",
1461+
*bin, *sbin);
1462+
split = true;
1463+
break;
1464+
}
1465+
if (split < 0)
1466+
split = false;
1467+
if (split)
1468+
return DEFAULT_PATH_WITH_SBIN;
1469+
#endif
1470+
return DEFAULT_PATH_WITHOUT_SBIN;
1471+
}

‎src/basic/path-util.h‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,26 @@
1111
#include "strv.h"
1212
#include "time-util.h"
1313

14-
#define PATH_SPLIT_SBIN_BIN(x) x "sbin:" x "bin"
15-
#define PATH_SPLIT_SBIN_BIN_NULSTR(x) x "sbin\0" x "bin\0"
14+
#define PATH_SPLIT_BIN(x) x "sbin:" x "bin"
15+
#define PATH_SPLIT_BIN_NULSTR(x) x "sbin\0" x "bin\0"
1616

17-
#define PATH_NORMAL_SBIN_BIN(x) x "bin"
18-
#define PATH_NORMAL_SBIN_BIN_NULSTR(x) x "bin\0"
17+
#define PATH_MERGED_BIN(x) x "bin"
18+
#define PATH_MERGED_BIN_NULSTR(x) x "bin\0"
1919

20-
#if HAVE_SPLIT_BIN
21-
# define PATH_SBIN_BIN(x) PATH_SPLIT_SBIN_BIN(x)
22-
# define PATH_SBIN_BIN_NULSTR(x) PATH_SPLIT_SBIN_BIN_NULSTR(x)
23-
#else
24-
# define PATH_SBIN_BIN(x) PATH_NORMAL_SBIN_BIN(x)
25-
# define PATH_SBIN_BIN_NULSTR(x) PATH_NORMAL_SBIN_BIN_NULSTR(x)
26-
#endif
20+
#define DEFAULT_PATH_WITH_SBIN PATH_SPLIT_BIN("/usr/local/") ":" PATH_SPLIT_BIN("/usr/")
21+
#define DEFAULT_PATH_WITHOUT_SBIN PATH_MERGED_BIN("/usr/local/") ":" PATH_MERGED_BIN("/usr/")
22+
23+
#define DEFAULT_PATH_COMPAT PATH_SPLIT_BIN("/usr/local/") ":" PATH_SPLIT_BIN("/usr/") ":" PATH_SPLIT_BIN("/")
2724

28-
#define DEFAULT_PATH PATH_SBIN_BIN("/usr/local/") ":" PATH_SBIN_BIN("/usr/")
29-
#define DEFAULT_PATH_NULSTR PATH_SBIN_BIN_NULSTR("/usr/local/") PATH_SBIN_BIN_NULSTR("/usr/")
30-
#define DEFAULT_PATH_COMPAT PATH_SPLIT_SBIN_BIN("/usr/local/") ":" PATH_SPLIT_SBIN_BIN("/usr/") ":" PATH_SPLIT_SBIN_BIN("/")
25+
const char* default_PATH(void);
3126

32-
#ifndef DEFAULT_USER_PATH
33-
# define DEFAULT_USER_PATH DEFAULT_PATH
27+
static inline const char* default_user_PATH(void) {
28+
#ifdef DEFAULT_USER_PATH
29+
return DEFAULT_USER_PATH;
30+
#else
31+
return default_PATH();
3432
#endif
33+
}
3534

3635
static inline bool is_path(const char *p) {
3736
if (!p) /* A NULL pointer is definitely not a path */

‎src/core/manager.c‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -658,8 +658,6 @@ static char** sanitize_environment(char **l) {
658658
}
659659

660660
int manager_default_environment(Manager *m) {
661-
int r;
662-
663661
assert(m);
664662

665663
m->transient_environment = strv_free(m->transient_environment);
@@ -670,8 +668,11 @@ int manager_default_environment(Manager *m) {
670668
*
671669
* The initial passed environment is untouched to keep /proc/self/environ valid; it is used
672670
* for tagging the init process inside containers. */
673-
m->transient_environment = strv_new("PATH=" DEFAULT_PATH);
674-
if (!m->transient_environment)
671+
char *path = strjoin("PATH=", default_PATH());
672+
if (!path)
673+
return log_oom();
674+
675+
if (strv_consume(&m->transient_environment, path) < 0)
675676
return log_oom();
676677

677678
/* Import locale variables LC_*= from configuration */
@@ -684,8 +685,11 @@ int manager_default_environment(Manager *m) {
684685
if (!m->transient_environment)
685686
return log_oom();
686687

687-
r = strv_env_replace_strdup(&m->transient_environment, "PATH=" DEFAULT_USER_PATH);
688-
if (r < 0)
688+
char *path = strjoin("PATH=", default_user_PATH());
689+
if (!path)
690+
return log_oom();
691+
692+
if (strv_env_replace_consume(&m->transient_environment, path) < 0)
689693
return log_oom();
690694

691695
/* Envvars set for our 'manager' class session are private and should not be propagated

‎src/libsystemd/sd-path/sd-path.c‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,14 @@ static int get_search(uint64_t type, char ***list) {
591591
"/etc",
592592
NULL);
593593

594-
case SD_PATH_SEARCH_BINARIES_DEFAULT:
595-
return strv_from_nulstr(list, DEFAULT_PATH_NULSTR);
594+
case SD_PATH_SEARCH_BINARIES_DEFAULT: {
595+
char **t = strv_split(default_PATH(), ":");
596+
if (!t)
597+
return -ENOMEM;
598+
599+
*list = t;
600+
return 0;
601+
}
596602

597603
case SD_PATH_SYSTEMD_SEARCH_SYSTEM_UNIT:
598604
case SD_PATH_SYSTEMD_SEARCH_USER_UNIT: {

‎src/test/test-exec-util.c‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ TEST(environment_gathering) {
331331
assert_se(chmod(name3, 0755) == 0);
332332

333333
/* When booting in containers or without initrd there might not be any PATH in the environment and if
334-
* there is no PATH /bin/sh built-in PATH may leak and override systemd's DEFAULT_PATH which is not
334+
* there is no PATH /bin/sh built-in PATH may leak and override systemd's default path which is not
335335
* good. Force our own PATH in environment, to prevent expansion of sh built-in $PATH */
336336
old = getenv("PATH");
337337
r = setenv("PATH", "no-sh-built-in-path", 1);
@@ -351,10 +351,9 @@ TEST(environment_gathering) {
351351
ASSERT_STREQ(strv_env_get(env, "C"), "001");
352352
ASSERT_STREQ(strv_env_get(env, "PATH"), "no-sh-built-in-path:/no/such/file");
353353

354-
/* now retest with "default" path passed in, as created by
355-
* manager_default_environment */
354+
/* Now retest with some "default" path passed. */
356355
env = strv_free(env);
357-
env = strv_new("PATH=" DEFAULT_PATH);
356+
env = strv_new("PATH=" DEFAULT_PATH_WITHOUT_SBIN);
358357
assert_se(env);
359358

360359
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) {
366365
ASSERT_STREQ(strv_env_get(env, "A"), "22:23:24");
367366
ASSERT_STREQ(strv_env_get(env, "B"), "12");
368367
ASSERT_STREQ(strv_env_get(env, "C"), "001");
369-
ASSERT_STREQ(strv_env_get(env, "PATH"), DEFAULT_PATH ":/no/such/file");
368+
ASSERT_STREQ(strv_env_get(env, "PATH"), DEFAULT_PATH_WITHOUT_SBIN ":/no/such/file");
370369

371370
/* reset environ PATH */
372371
assert_se(set_unset_env("PATH", old, true) == 0);

‎src/test/test-path-util.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
#include "tmpfile-util.h"
1919

2020
TEST(print_paths) {
21-
log_info("DEFAULT_PATH=%s", DEFAULT_PATH);
22-
log_info("DEFAULT_USER_PATH=%s", DEFAULT_USER_PATH);
21+
log_info("default system PATH: %s", default_PATH());
22+
log_info("default user PATH: %s", default_user_PATH());
2323
}
2424

2525
TEST(path) {

0 commit comments

Comments
 (0)