From: Frantisek Sumsal Date: Wed, 17 Jun 2026 12:09:43 +0000 (+0200) Subject: unit-name: introduce "strict" mode for unit name mangling X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=14b1a92eb251f275d79245e4ed4668fb717804f5;p=thirdparty%2Fsystemd.git unit-name: introduce "strict" mode for unit name mangling unit_name_mangle_with_suffix() is quite benevolent by default and allows the unit to "transition" into a different unit type than what's requested via its suffix argument. For example, calling unit_name_mangle_with_suffix() with "/foo/bar" as a unit name and ".service" as a suffix would give you "foo-bar.mount", without any warning or error. This could then lead to a quite confusing errors in certain situations: ~# systemd-run --remain-after-exit --unit /foo/bar true Failed to start transient service unit: Cannot set property RemainAfterExit, or unknown property. Given we can't change the default behaviour of unit_name_mangle_with_suffix() as some parts of systemd already depend on its "benevolence" (like systemctl), let's introduce a new flag - UNIT_NAME_MANGLE_STRICT - that checks if the mangled/resolved unit name's suffix matches the requested one and errors out if not. With the flag used throughout systemd-run's code, the error in the above case is now a bit more clear: ~# build/systemd-run --remain-after-exit --unit /foo/bar true Path "/foo/bar" resolves to unit type "mount", but "service" is expected as unit. Failed to mangle unit name: Invalid argument Resolves: #39996 --- diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 4f504c047a7..0efc66b91b7 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -730,8 +730,19 @@ int unit_name_mangle_with_suffix( return -EINVAL; /* Already a fully valid unit name? If so, no mangling is necessary... */ - if (unit_name_is_valid(name, UNIT_NAME_ANY)) + if (unit_name_is_valid(name, UNIT_NAME_ANY)) { + if (FLAGS_SET(flags, UNIT_NAME_MANGLE_STRICT) && !endswith(name, suffix)) { + const char *e = ASSERT_PTR(strrchr(name, '.')); + + return log_full_errno(warn ? LOG_NOTICE : LOG_DEBUG, + SYNTHETIC_ERRNO(EINVAL), + "Unit name \"%s\" has unit type \"%s\", but \"%s\" is expected%s%s.", + name, e + 1, suffix + 1, + operation ? " " : "", strempty(operation)); + } + goto good; + } /* Already a fully valid globbing expression? If so, no mangling is necessary either... */ if (string_is_glob(name) && in_charset(name, VALID_CHARS_GLOB)) { @@ -744,23 +755,41 @@ int unit_name_mangle_with_suffix( } if (path_is_absolute(name)) { - _cleanup_free_ char *n = NULL; + _cleanup_free_ char *n = NULL, *u = NULL; r = path_simplify_alloc(name, &n); if (r < 0) return r; if (is_device_path(n)) { - r = unit_name_from_path(n, ".device", ret); - if (r >= 0) + r = unit_name_from_path(n, ".device", &u); + if (r >= 0) { + if (FLAGS_SET(flags, UNIT_NAME_MANGLE_STRICT) && !streq(suffix, ".device")) + return log_full_errno(warn ? LOG_NOTICE : LOG_DEBUG, + SYNTHETIC_ERRNO(EINVAL), + "Path \"%s\" resolves to unit type \"device\", but \"%s\" is expected%s%s.", + name, suffix + 1, + operation ? " " : "", strempty(operation)); + + *ret = TAKE_PTR(u); return 1; + } if (r != -EINVAL) return r; } - r = unit_name_from_path(n, ".mount", ret); - if (r >= 0) + r = unit_name_from_path(n, ".mount", &u); + if (r >= 0) { + if (FLAGS_SET(flags, UNIT_NAME_MANGLE_STRICT) && !streq(suffix, ".mount")) + return log_full_errno(warn ? LOG_NOTICE : LOG_DEBUG, + SYNTHETIC_ERRNO(EINVAL), + "Path \"%s\" resolves to unit type \"mount\", but \"%s\" is expected%s%s.", + name, suffix + 1, + operation ? " " : "", strempty(operation)); + + *ret = TAKE_PTR(u); return 1; + } if (r != -EINVAL) return r; } diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h index e454d0a86db..7484ec6529e 100644 --- a/src/basic/unit-name.h +++ b/src/basic/unit-name.h @@ -56,8 +56,9 @@ int unit_name_from_path_instance(const char *prefix, const char *path, const cha int unit_name_to_path(const char *name, char **ret); typedef enum UnitNameMangle { - UNIT_NAME_MANGLE_GLOB = 1 << 0, - UNIT_NAME_MANGLE_WARN = 1 << 1, + UNIT_NAME_MANGLE_GLOB = 1 << 0, + UNIT_NAME_MANGLE_WARN = 1 << 1, + UNIT_NAME_MANGLE_STRICT = 1 << 2, /* Refuse if the resolved unit type doesn't match the requested suffix */ } UnitNameMangle; int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret); diff --git a/src/run/run.c b/src/run/run.c index 96bd3af9fdf..f7dc5b56708 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -2364,7 +2364,7 @@ static int start_transient_service(sd_bus *bus) { r = unit_name_mangle_with_suffix( arg_unit, "as unit", - arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN) | UNIT_NAME_MANGLE_STRICT, ".service", &c.unit); if (r < 0) @@ -2508,7 +2508,7 @@ static int start_transient_scope(sd_bus *bus) { if (arg_unit) { r = unit_name_mangle_with_suffix(arg_unit, "as unit", - arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN) | UNIT_NAME_MANGLE_STRICT, ".scope", &scope); if (r < 0) return log_error_errno(r, "Failed to mangle scope name: %m"); @@ -2809,13 +2809,13 @@ static int start_transient_trigger(sd_bus *bus, const char *suffix) { default: r = unit_name_mangle_with_suffix(arg_unit, "as unit", - arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN) | UNIT_NAME_MANGLE_STRICT, ".service", &service); if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); r = unit_name_mangle_with_suffix(arg_unit, "as trigger", - arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN) | UNIT_NAME_MANGLE_STRICT, suffix, &trigger); if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index 672162244fd..7f71f5fdc74 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -251,6 +251,18 @@ static void test_unit_name_mangle_with_suffix_one(const char *arg, int expected, ASSERT_STREQ(s, expected_name); } +static void test_unit_name_mangle_with_suffix_strict_one( + const char *arg, const char *suffix, int expected, const char *expected_name) { + _cleanup_free_ char *s = NULL; + int r; + + r = unit_name_mangle_with_suffix(arg, NULL, UNIT_NAME_MANGLE_WARN | UNIT_NAME_MANGLE_STRICT, suffix, &s); + log_debug("%s: %s (suffix=%s) -> %d, %s", __func__, arg, suffix, r, strnull(s)); + + assert_se(r == expected); + ASSERT_STREQ(s, expected_name); +} + TEST(unit_name_mangle_with_suffix) { test_unit_name_mangle_with_suffix_one("", -EINVAL, NULL); @@ -282,6 +294,28 @@ TEST(unit_name_mangle_with_suffix) { test_unit_name_mangle_with_suffix_one("/./.././../proc/", 1, "proc.mount"); } +TEST(unit_name_mangle_with_suffix_strict) { + /* Matching suffix should succeed */ + test_unit_name_mangle_with_suffix_strict_one("foo.service", ".service", 0, "foo.service"); + test_unit_name_mangle_with_suffix_strict_one("foo.mount", ".mount", 0, "foo.mount"); + test_unit_name_mangle_with_suffix_strict_one("/home", ".mount", 1, "home.mount"); + test_unit_name_mangle_with_suffix_strict_one("/dev/sda", ".device", 1, "dev-sda.device"); + test_unit_name_mangle_with_suffix_strict_one("foo@bar.service", ".service", 0, "foo@bar.service"); + test_unit_name_mangle_with_suffix_strict_one("a.timer.service", ".service", 0, "a.timer.service"); + + /* Mismatched suffix should fail with -EINVAL */ + test_unit_name_mangle_with_suffix_strict_one("foo.mount", ".service", -EINVAL, NULL); + test_unit_name_mangle_with_suffix_strict_one("foo.service", ".scope", -EINVAL, NULL); + test_unit_name_mangle_with_suffix_strict_one("/home", ".service", -EINVAL, NULL); + test_unit_name_mangle_with_suffix_strict_one("/dev/sda", ".service", -EINVAL, NULL); + test_unit_name_mangle_with_suffix_strict_one("foo.automount", ".mount", -EINVAL, NULL); + test_unit_name_mangle_with_suffix_strict_one("foo.mount", ".automount", -EINVAL, NULL); + test_unit_name_mangle_with_suffix_strict_one("foo@bar.timer", ".service", -EINVAL, NULL); + + /* Non-path names that need mangling should get the requested suffix and thus pass */ + test_unit_name_mangle_with_suffix_strict_one("foo", ".service", 1, "foo.service"); +} + TEST_RET(unit_printf, .sd_booted = true) { _cleanup_free_ char *architecture, *os_image_version, *boot_id = NULL, *os_build_id,