path-lookup: reject empty env path components

Only trailing empty components have special meaning:
they request appending the built-in defaults.

Share the validation between normal path lookup and
systemd-analyze verify, so verify does not compose
values that path lookup later rejects.

Propagate generator path parse errors instead of
turning them into empty generator paths.

Follow-up for: cf7d80a5fe
This commit is contained in:
dongshengyuan
2026-08-14 00:27:09 +09:00
committed by Yu Watanabe
parent cad2c455ec
commit f4284e9ceb
7 changed files with 156 additions and 39 deletions
+4 -1
View File
@@ -157,8 +157,11 @@ int verify_set_unit_path(char **filenames) {
* otherwise use the defaults. Any duplicates will be filtered out in path-lookup.c.
* Treat explicit empty path to mean that nothing should be appended. */
old = getenv("SYSTEMD_UNIT_PATH");
if (!path_is_valid_search_path(old))
return -EINVAL;
if (!streq_ptr(old, "") &&
!strextend_with_separator(&joined, ":", strempty(old)))
!strextend_with_separator(&joined, ":", streq_ptr(old, ":") ? "" : strempty(old)))
return -ENOMEM;
assert_se(setenv_unit_path(joined) >= 0);
+48
View File
@@ -1,9 +1,16 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#include <stdlib.h>
#include "analyze.h"
#include "analyze-verify-util.h"
#include "execute.h"
#include "fileio.h"
#include "path-util.h"
#include "rm-rf.h"
#include "string-util.h"
#include "tests.h"
#include "tmpfile-util.h"
const char *arg_instance = "test_instance";
@@ -17,4 +24,45 @@ TEST(verify_nonexistent) {
assert_se(verify_executable(NULL, &(ExecCommand) {.flags = EXEC_COMMAND_IGNORE_FAILURE, .path = (char*) "/bin/echo"}, NULL) == 0);
}
static void test_verify_set_unit_path_one(const char *old, const char *expected) {
_cleanup_(rm_rf_physical_and_freep) char *tmp = NULL;
_cleanup_free_ char *unit = NULL;
char *unit_paths[2];
ASSERT_OK(mkdtemp_malloc("/tmp/test-verify.XXXXXX", &tmp));
unit = ASSERT_NOT_NULL(path_join(tmp, "test.service"));
ASSERT_OK(write_string_file(unit, "[Unit]\nDescription=test\n", WRITE_STRING_FILE_CREATE));
if (old)
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", old, 1));
else
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_UNIT_PATH"));
unit_paths[0] = unit;
unit_paths[1] = NULL;
if (expected) {
ASSERT_OK(verify_set_unit_path(unit_paths));
ASSERT_STREQ(getenv("SYSTEMD_UNIT_PATH"), strjoina(tmp, expected));
} else
ASSERT_ERROR(verify_set_unit_path(unit_paths), EINVAL);
}
TEST(verify_set_unit_path) {
_cleanup_free_ char *old_saved = getenv("SYSTEMD_UNIT_PATH") ? strdup(getenv("SYSTEMD_UNIT_PATH")) : NULL;
ASSERT_TRUE(old_saved || !getenv("SYSTEMD_UNIT_PATH"));
test_verify_set_unit_path_one(NULL, ":");
test_verify_set_unit_path_one("", "");
test_verify_set_unit_path_one(":", ":");
test_verify_set_unit_path_one("/foo:", ":/foo:");
test_verify_set_unit_path_one(":foo", NULL);
test_verify_set_unit_path_one("/foo::/bar", NULL);
if (old_saved)
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", old_saved, 1));
else
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_UNIT_PATH"));
}
DEFINE_TEST_MAIN(LOG_DEBUG);
+6 -6
View File
@@ -4297,9 +4297,9 @@ static int manager_run_environment_generators(Manager *m) {
if (MANAGER_IS_TEST_RUN(m) && !(m->test_run_flags & MANAGER_TEST_RUN_ENV_GENERATORS))
return 0;
paths = env_generator_binary_paths(m->runtime_scope);
if (!paths)
return log_oom();
r = env_generator_binary_paths(m->runtime_scope, &paths);
if (r < 0)
return log_error_errno(r, "Failed to initialize environment generator search paths: %m");
if (!generator_path_any(paths))
return 0;
@@ -4450,9 +4450,9 @@ static int manager_run_generators(Manager *m) {
if (MANAGER_IS_TEST_RUN(m) && !(m->test_run_flags & MANAGER_TEST_RUN_GENERATORS))
return 0;
paths = generator_binary_paths(m->runtime_scope);
if (!paths)
return log_oom();
r = generator_binary_paths(m->runtime_scope, &paths);
if (r < 0)
return log_error_errno(r, "Failed to initialize generator search paths: %m");
if (!generator_path_any(paths))
return 0;
+15 -6
View File
@@ -397,6 +397,12 @@ static int patch_root_prefix_strv(char **l, const char *root_dir) {
return 0;
}
bool path_is_valid_search_path(const char *path) {
return isempty(path) ||
(isempty(startswith(path, ":")) &&
!strstr(path, "::"));
}
static int get_paths_from_environ(const char *var, char ***ret) {
const char *e;
int r;
@@ -410,11 +416,12 @@ static int get_paths_from_environ(const char *var, char ***ret) {
return 0;
}
if (!path_is_valid_search_path(e))
return -EINVAL;
bool append = endswith(e, ":"); /* Whether to append the normal search paths after what's obtained
from envvar */
/* FIXME: empty components in other places should be rejected. */
r = path_split_and_make_absolute(e, ret);
if (r < 0)
return r;
@@ -796,7 +803,7 @@ static const char* const user_env_generator_paths[] = {
NULL,
};
char** generator_binary_paths_internal(RuntimeScope scope, bool env_generator) {
int generator_binary_paths_internal(RuntimeScope scope, bool env_generator, char ***ret) {
static const struct {
const char *env_name;
const char * const *paths[_RUNTIME_SCOPE_MAX];
@@ -818,6 +825,7 @@ char** generator_binary_paths_internal(RuntimeScope scope, bool env_generator) {
int r;
assert(IN_SET(scope, RUNTIME_SCOPE_SYSTEM, RUNTIME_SCOPE_USER));
assert(ret);
const char *env_name = ASSERT_PTR((env_generator ? environment_generator : unit_generator).env_name);
const char * const *generator_paths = ASSERT_PTR((env_generator ? environment_generator : unit_generator).paths[scope]);
@@ -825,13 +833,14 @@ char** generator_binary_paths_internal(RuntimeScope scope, bool env_generator) {
/* First priority is whatever has been passed to us via env vars */
r = get_paths_from_environ(env_name, &paths);
if (r < 0)
return NULL;
return r;
if (!paths || r > 0) {
r = strv_extend_strv(&paths, (char* const*) generator_paths, /* filter_duplicates= */ true);
if (r < 0)
return NULL;
return r;
}
return TAKE_PTR(paths);
*ret = TAKE_PTR(paths);
return 0;
}
+6 -5
View File
@@ -83,13 +83,14 @@ static inline int xdg_user_state_dir(const char *suffix, char **ret) {
bool path_is_user_data_dir(const char *path);
bool path_is_user_config_dir(const char *path);
bool path_is_valid_search_path(const char *path) _pure_;
char** generator_binary_paths_internal(RuntimeScope scope, bool env_generator);
static inline char** generator_binary_paths(RuntimeScope runtime_scope) {
return generator_binary_paths_internal(runtime_scope, false);
int generator_binary_paths_internal(RuntimeScope scope, bool env_generator, char ***ret);
static inline int generator_binary_paths(RuntimeScope runtime_scope, char ***ret) {
return generator_binary_paths_internal(runtime_scope, false, ret);
}
static inline char** env_generator_binary_paths(RuntimeScope runtime_scope) {
return generator_binary_paths_internal(runtime_scope, true);
static inline int env_generator_binary_paths(RuntimeScope runtime_scope, char ***ret) {
return generator_binary_paths_internal(runtime_scope, true, ret);
}
static inline int credential_store_path(RuntimeScope runtime_scope, char ***ret) {
+5 -4
View File
@@ -631,11 +631,12 @@ static int get_search(uint64_t type, char ***ret) {
bool env_generator = IN_SET(type, SD_PATH_SYSTEMD_SEARCH_SYSTEM_ENVIRONMENT_GENERATOR,
SD_PATH_SYSTEMD_SEARCH_USER_ENVIRONMENT_GENERATOR);
char **t = generator_binary_paths_internal(scope, env_generator);
if (!t)
return -ENOMEM;
_cleanup_strv_free_ char **t = NULL;
r = generator_binary_paths_internal(scope, env_generator, &t);
if (r < 0)
return r;
*ret = t;
*ret = TAKE_PTR(t);
return 0;
}
+72 -17
View File
@@ -18,13 +18,13 @@ static void test_paths_one(RuntimeScope scope) {
assert_se(mkdtemp_malloc("/tmp/test-path-lookup.XXXXXXX", &tmp) >= 0);
assert_se(unsetenv("SYSTEMD_UNIT_PATH") == 0);
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_UNIT_PATH"));
assert_se(lookup_paths_init(&lp_without_env, scope, 0, NULL) >= 0);
assert_se(!strv_isempty(lp_without_env.search_path));
lookup_paths_log(&lp_without_env);
systemd_unit_path = strjoina(tmp, "/systemd-unit-path");
assert_se(setenv("SYSTEMD_UNIT_PATH", systemd_unit_path, 1) == 0);
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", systemd_unit_path, 1));
assert_se(lookup_paths_init(&lp_with_env, scope, 0, NULL) == 0);
assert_se(strv_length(lp_with_env.search_path) == 1);
ASSERT_STREQ(lp_with_env.search_path[0], systemd_unit_path);
@@ -38,14 +38,59 @@ TEST(paths) {
test_paths_one(RUNTIME_SCOPE_GLOBAL);
}
static void test_paths_empty_components_one(RuntimeScope scope) {
_cleanup_(rm_rf_physical_and_freep) char *tmp = NULL;
_cleanup_(lookup_paths_done) LookupPaths lp_without_env = {};
_cleanup_(lookup_paths_done) LookupPaths lp_with_empty = {};
_cleanup_(lookup_paths_done) LookupPaths lp_with_separator = {};
_cleanup_(lookup_paths_done) LookupPaths lp_with_leading_empty = {};
_cleanup_(lookup_paths_done) LookupPaths lp_with_middle_empty = {};
_cleanup_(lookup_paths_done) LookupPaths lp_with_trailing_empty = {};
char *systemd_unit_path;
assert_se(mkdtemp_malloc("/tmp/test-path-lookup.XXXXXXX", &tmp) >= 0);
systemd_unit_path = strjoina(tmp, "/systemd-unit-path");
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_UNIT_PATH"));
ASSERT_OK(lookup_paths_init(&lp_without_env, scope, 0, NULL));
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", "", 1));
ASSERT_OK(lookup_paths_init(&lp_with_empty, scope, 0, NULL));
ASSERT_TRUE(strv_isempty(lp_with_empty.search_path));
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", ":", 1));
ASSERT_OK(lookup_paths_init(&lp_with_separator, scope, 0, NULL));
ASSERT_TRUE(strv_equal(lp_with_separator.search_path, lp_without_env.search_path));
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", strjoina(":", systemd_unit_path), 1));
ASSERT_ERROR(lookup_paths_init(&lp_with_leading_empty, scope, 0, NULL), EINVAL);
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", strjoina(systemd_unit_path, "::/run"), 1));
ASSERT_ERROR(lookup_paths_init(&lp_with_middle_empty, scope, 0, NULL), EINVAL);
ASSERT_OK_ERRNO(setenv("SYSTEMD_UNIT_PATH", strjoina(systemd_unit_path, ":"), 1));
ASSERT_OK(lookup_paths_init(&lp_with_trailing_empty, scope, 0, NULL));
ASSERT_STREQ(lp_with_trailing_empty.search_path[0], systemd_unit_path);
ASSERT_GT(strv_length(lp_with_trailing_empty.search_path), 1u);
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_UNIT_PATH"));
}
TEST(paths_empty_components) {
test_paths_empty_components_one(RUNTIME_SCOPE_SYSTEM);
test_paths_empty_components_one(RUNTIME_SCOPE_USER);
test_paths_empty_components_one(RUNTIME_SCOPE_GLOBAL);
}
TEST(user_and_global_paths) {
_cleanup_(lookup_paths_done) LookupPaths lp_global = {}, lp_user = {};
char **u, **g;
unsigned k = 0;
assert_se(unsetenv("SYSTEMD_UNIT_PATH") == 0);
assert_se(unsetenv("XDG_DATA_DIRS") == 0);
assert_se(unsetenv("XDG_CONFIG_DIRS") == 0);
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_UNIT_PATH"));
ASSERT_OK_ERRNO(unsetenv("XDG_DATA_DIRS"));
ASSERT_OK_ERRNO(unsetenv("XDG_CONFIG_DIRS"));
assert_se(lookup_paths_init(&lp_global, RUNTIME_SCOPE_GLOBAL, 0, NULL) == 0);
assert_se(lookup_paths_init(&lp_user, RUNTIME_SCOPE_USER, 0, NULL) == 0);
@@ -75,16 +120,18 @@ static void test_generator_binary_paths_one(RuntimeScope scope) {
_cleanup_strv_free_ char **env_gp_without_env = NULL;
_cleanup_strv_free_ char **gp_with_env = NULL;
_cleanup_strv_free_ char **env_gp_with_env = NULL;
_cleanup_strv_free_ char **gp_with_bad_env = NULL;
_cleanup_strv_free_ char **env_gp_with_bad_env = NULL;
char *systemd_generator_path = NULL;
char *systemd_env_generator_path = NULL;
assert_se(mkdtemp_malloc("/tmp/test-path-lookup.XXXXXXX", &tmp) >= 0);
assert_se(unsetenv("SYSTEMD_GENERATOR_PATH") == 0);
assert_se(unsetenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH") == 0);
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_GENERATOR_PATH"));
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH"));
gp_without_env = generator_binary_paths(scope);
env_gp_without_env = env_generator_binary_paths(scope);
ASSERT_OK(generator_binary_paths(scope, &gp_without_env));
ASSERT_OK(env_generator_binary_paths(scope, &env_gp_without_env));
log_info("Generators dirs (%s):", runtime_scope_to_string(scope));
STRV_FOREACH(dir, gp_without_env)
@@ -94,16 +141,16 @@ static void test_generator_binary_paths_one(RuntimeScope scope) {
STRV_FOREACH(dir, env_gp_without_env)
log_info(" %s", *dir);
assert_se(!strv_isempty(gp_without_env));
assert_se(!strv_isempty(env_gp_without_env));
ASSERT_FALSE(strv_isempty(gp_without_env));
ASSERT_FALSE(strv_isempty(env_gp_without_env));
systemd_generator_path = strjoina(tmp, "/systemd-generator-path");
systemd_env_generator_path = strjoina(tmp, "/systemd-environment-generator-path");
assert_se(setenv("SYSTEMD_GENERATOR_PATH", systemd_generator_path, 1) == 0);
assert_se(setenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH", systemd_env_generator_path, 1) == 0);
ASSERT_OK_ERRNO(setenv("SYSTEMD_GENERATOR_PATH", systemd_generator_path, 1));
ASSERT_OK_ERRNO(setenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH", systemd_env_generator_path, 1));
gp_with_env = generator_binary_paths(scope);
env_gp_with_env = env_generator_binary_paths(scope);
ASSERT_OK(generator_binary_paths(scope, &gp_with_env));
ASSERT_OK(env_generator_binary_paths(scope, &env_gp_with_env));
log_info("Generators dirs (%s):", runtime_scope_to_string(scope));
STRV_FOREACH(dir, gp_with_env)
@@ -113,8 +160,16 @@ static void test_generator_binary_paths_one(RuntimeScope scope) {
STRV_FOREACH(dir, env_gp_with_env)
log_info(" %s", *dir);
assert_se(strv_equal(gp_with_env, STRV_MAKE(systemd_generator_path)));
assert_se(strv_equal(env_gp_with_env, STRV_MAKE(systemd_env_generator_path)));
ASSERT_TRUE(strv_equal(gp_with_env, STRV_MAKE(systemd_generator_path)));
ASSERT_TRUE(strv_equal(env_gp_with_env, STRV_MAKE(systemd_env_generator_path)));
ASSERT_OK_ERRNO(setenv("SYSTEMD_GENERATOR_PATH", strjoina(":", systemd_generator_path), 1));
ASSERT_OK_ERRNO(setenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH", strjoina(":", systemd_env_generator_path), 1));
ASSERT_ERROR(generator_binary_paths(scope, &gp_with_bad_env), EINVAL);
ASSERT_ERROR(env_generator_binary_paths(scope, &env_gp_with_bad_env), EINVAL);
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_GENERATOR_PATH"));
ASSERT_OK_ERRNO(unsetenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH"));
}
TEST(generator_binary_paths) {