You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
298 lines
13 KiB
298 lines
13 KiB
From 5ec751ab9a06dadc62b30dc07e9dd7a41f8da403 Mon Sep 17 00:00:00 2001 |
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> |
|
Date: Fri, 4 Mar 2022 18:47:31 +0100 |
|
Subject: [PATCH] shared/install: reuse the standard symlink verification |
|
subroutine |
|
MIME-Version: 1.0 |
|
Content-Type: text/plain; charset=UTF-8 |
|
Content-Transfer-Encoding: 8bit |
|
|
|
We save a few lines, but the important thing is that we don't have two |
|
different implementations with slightly different rules used for enablement |
|
and loading. Fixes #22000. |
|
|
|
Tested with: |
|
- the report in #22000, it now says: |
|
$ SYSTEMD_LOG_LEVEL=debug systemctl --root=/ enable test.service |
|
Suspicious symlink /etc/systemd/system/test.service→/etc/systemd/system/myown.d/test.service, treating as alias. |
|
unit_file_resolve_symlink: self-alias: /etc/systemd/system/test.service → test.service, ignoring. |
|
running_in_chroot(): Permission denied |
|
Suspicious symlink /etc/systemd/system/test.service→/etc/systemd/system/myown.d/test.service, treating as alias. |
|
unit_file_resolve_symlink: self-alias: /etc/systemd/system/test.service → test.service, ignoring. |
|
Failed to enable unit, refusing to operate on linked unit file test.service |
|
|
|
- a symlink to /dev/null: |
|
... |
|
unit_file_resolve_symlink: linked unit file: /etc/systemd/system/test3.service → /dev/null |
|
Failed to enable unit, unit /etc/systemd/system/test3.service is masked. |
|
|
|
- the same from the host: |
|
... |
|
unit_file_resolve_symlink: linked unit file: /var/lib/machines/rawhide/etc/systemd/system/test3.service → /var/lib/machines/rawhide/dev/null |
|
Failed to enable unit, unit /var/lib/machines/rawhide/etc/systemd/system/test3.service is masked. |
|
|
|
- through the manager: |
|
$ sudo systemctl enable test.service |
|
Failed to enable unit: Refusing to operate on alias name or linked unit file: test.service |
|
$ sudo systemctl enable test3.service |
|
Failed to enable unit: Unit file /etc/systemd/system/test3.service is masked. |
|
|
|
As seen in the first example, the warning is repeated. This is because we call |
|
the lookup logic twice: first for sysv-compat, and then again for real. I think |
|
that since this is only for broken setups, and when sysv-compat is enabled, and |
|
in an infrequent manual operation, at debug level, this is OK. |
|
|
|
(cherry picked from commit 047d37dc3d376d912275c14d217f7a0dda9a5f0e) |
|
|
|
Related: #2082131 |
|
--- |
|
src/basic/unit-file.c | 70 +++++++++++++++++++++++++++++++---------- |
|
src/basic/unit-file.h | 10 ++++++ |
|
src/shared/install.c | 72 +++++++------------------------------------ |
|
3 files changed, 75 insertions(+), 77 deletions(-) |
|
|
|
diff --git a/src/basic/unit-file.c b/src/basic/unit-file.c |
|
index 25abce932a..f7a10b22c6 100644 |
|
--- a/src/basic/unit-file.c |
|
+++ b/src/basic/unit-file.c |
|
@@ -260,27 +260,50 @@ static int directory_name_is_valid(const char *name) { |
|
return false; |
|
} |
|
|
|
-static int unit_file_resolve_symlink( |
|
+int unit_file_resolve_symlink( |
|
const char *root_dir, |
|
char **search_path, |
|
const char *dir, |
|
int dirfd, |
|
const char *filename, |
|
+ bool resolve_destination_target, |
|
char **ret_destination) { |
|
|
|
- _cleanup_free_ char *target = NULL, *simplified = NULL, *dst = NULL; |
|
+ _cleanup_free_ char *target = NULL, *simplified = NULL, *dst = NULL, *_dir = NULL, *_filename = NULL; |
|
int r; |
|
|
|
- assert(dir); |
|
- assert(dirfd >= 0); |
|
+ /* This can be called with either dir+dirfd valid and filename just a name, |
|
+ * or !dir && dirfd==AT_FDCWD, and filename being a full path. |
|
+ * |
|
+ * If resolve_destination_target is true, an absolute path will be returned. |
|
+ * If not, an absolute path is returned for linked unit files, and a relative |
|
+ * path otherwise. */ |
|
+ |
|
assert(filename); |
|
assert(ret_destination); |
|
+ assert(dir || path_is_absolute(filename)); |
|
+ assert(dirfd >= 0 || dirfd == AT_FDCWD); |
|
|
|
r = readlinkat_malloc(dirfd, filename, &target); |
|
if (r < 0) |
|
return log_warning_errno(r, "Failed to read symlink %s%s%s: %m", |
|
dir, dir ? "/" : "", filename); |
|
|
|
+ if (!dir) { |
|
+ r = path_extract_directory(filename, &_dir); |
|
+ if (r < 0) |
|
+ return r; |
|
+ dir = _dir; |
|
+ |
|
+ r = path_extract_filename(filename, &_filename); |
|
+ if (r < 0) |
|
+ return r; |
|
+ if (r == O_DIRECTORY) |
|
+ return log_warning_errno(SYNTHETIC_ERRNO(EISDIR), |
|
+ "Unexpected path to a directory \"%s\", refusing.", filename); |
|
+ filename = _filename; |
|
+ } |
|
+ |
|
bool is_abs = path_is_absolute(target); |
|
if (root_dir || !is_abs) { |
|
char *target_abs = path_join(is_abs ? root_dir : dir, target); |
|
@@ -296,24 +319,36 @@ static int unit_file_resolve_symlink( |
|
return log_warning_errno(r, "Failed to resolve symlink %s/%s pointing to %s: %m", |
|
dir, filename, target); |
|
|
|
+ assert(path_is_absolute(simplified)); |
|
+ |
|
/* Check if the symlink goes outside of our search path. |
|
- * If yes, it's a linked unit file or mask, and we don't care about the target name. |
|
- * Let's just store the link source directly. |
|
- * If not, let's verify that it's a good symlink. */ |
|
+ * If yes, it's a linked unit file or mask, and we don't care about the target name |
|
+ * when loading units, and we return the link *source* (resolve_destination_target == false); |
|
+ * When this is called for installation purposes, we want the final destination, |
|
+ * so we return the *target*. |
|
+ * |
|
+ * Otherwise, let's verify that it's a good alias. |
|
+ */ |
|
const char *tail = path_startswith_strv(simplified, search_path); |
|
if (!tail) { |
|
log_debug("Linked unit file: %s/%s → %s", dir, filename, simplified); |
|
|
|
- dst = path_join(dir, filename); |
|
- if (!dst) |
|
- return log_oom(); |
|
+ if (resolve_destination_target) |
|
+ dst = TAKE_PTR(simplified); |
|
+ else { |
|
+ dst = path_join(dir, filename); |
|
+ if (!dst) |
|
+ return log_oom(); |
|
+ } |
|
|
|
} else { |
|
- r = path_extract_filename(simplified, &dst); |
|
+ _cleanup_free_ char *target_name = NULL; |
|
+ |
|
+ r = path_extract_filename(simplified, &target_name); |
|
if (r < 0) |
|
return r; |
|
|
|
- bool self_alias = streq(dst, filename); |
|
+ bool self_alias = streq(target_name, filename); |
|
|
|
if (is_path(tail)) |
|
log_full(self_alias ? LOG_DEBUG : LOG_WARNING, |
|
@@ -324,13 +359,15 @@ static int unit_file_resolve_symlink( |
|
if (r < 0) |
|
return r; |
|
|
|
- if (self_alias) |
|
- /* A self-alias that has no effect */ |
|
+ if (self_alias && !resolve_destination_target) |
|
+ /* A self-alias that has no effect when loading, let's just ignore it. */ |
|
return log_debug_errno(SYNTHETIC_ERRNO(ELOOP), |
|
"Unit file self-alias: %s/%s → %s, ignoring.", |
|
- dir, filename, dst); |
|
+ dir, filename, target_name); |
|
+ |
|
+ log_debug("Unit file alias: %s/%s → %s", dir, filename, target_name); |
|
|
|
- log_debug("Unit file alias: %s/%s → %s", dir, filename, dst); |
|
+ dst = resolve_destination_target ? TAKE_PTR(simplified) : TAKE_PTR(target_name); |
|
} |
|
|
|
*ret_destination = TAKE_PTR(dst); |
|
@@ -475,6 +512,7 @@ int unit_file_build_name_map( |
|
|
|
r = unit_file_resolve_symlink(lp->root_dir, lp->search_path, |
|
*dir, dirfd(d), de->d_name, |
|
+ /* resolve_destination_target= */ false, |
|
&dst); |
|
if (r == -ENOMEM) |
|
return r; |
|
diff --git a/src/basic/unit-file.h b/src/basic/unit-file.h |
|
index cc731a9e06..e29e878cfd 100644 |
|
--- a/src/basic/unit-file.h |
|
+++ b/src/basic/unit-file.h |
|
@@ -44,6 +44,16 @@ int unit_symlink_name_compatible(const char *symlink, const char *target, bool i |
|
int unit_validate_alias_symlink_and_warn(const char *filename, const char *target); |
|
|
|
bool lookup_paths_timestamp_hash_same(const LookupPaths *lp, uint64_t timestamp_hash, uint64_t *ret_new); |
|
+ |
|
+int unit_file_resolve_symlink( |
|
+ const char *root_dir, |
|
+ char **search_path, |
|
+ const char *dir, |
|
+ int dirfd, |
|
+ const char *filename, |
|
+ bool resolve_destination_target, |
|
+ char **ret_destination); |
|
+ |
|
int unit_file_build_name_map( |
|
const LookupPaths *lp, |
|
uint64_t *cache_timestamp_hash, |
|
diff --git a/src/shared/install.c b/src/shared/install.c |
|
index 79e5109ce1..e07ca31797 100644 |
|
--- a/src/shared/install.c |
|
+++ b/src/shared/install.c |
|
@@ -1338,76 +1338,26 @@ static int unit_file_load_or_readlink( |
|
const char *path, |
|
const LookupPaths *lp, |
|
SearchFlags flags) { |
|
- |
|
- _cleanup_free_ char *resolved = NULL; |
|
int r; |
|
|
|
r = unit_file_load(c, info, path, lp->root_dir, flags); |
|
if (r != -ELOOP || (flags & SEARCH_DROPIN)) |
|
return r; |
|
|
|
- r = chase_symlinks(path, lp->root_dir, CHASE_WARN | CHASE_NONEXISTENT, &resolved, NULL); |
|
- if (r >= 0 && |
|
- lp->root_dir && |
|
- path_equal_ptr(path_startswith(resolved, lp->root_dir), "dev/null")) |
|
- /* When looking under root_dir, we can't expect /dev/ to be mounted, |
|
- * so let's see if the path is a (possibly dangling) symlink to /dev/null. */ |
|
- info->type = UNIT_FILE_TYPE_MASKED; |
|
- |
|
- else if (r > 0 && null_or_empty_path(resolved) > 0) |
|
+ /* This is a symlink, let's read and verify it. */ |
|
+ r = unit_file_resolve_symlink(lp->root_dir, lp->search_path, |
|
+ NULL, AT_FDCWD, path, |
|
+ true, &info->symlink_target); |
|
+ if (r < 0) |
|
+ return r; |
|
|
|
+ r = null_or_empty_path_with_root(info->symlink_target, lp->root_dir); |
|
+ if (r < 0 && r != -ENOENT) |
|
+ return log_debug_errno(r, "Failed to stat %s: %m", info->symlink_target); |
|
+ if (r > 0) |
|
info->type = UNIT_FILE_TYPE_MASKED; |
|
- |
|
- else { |
|
- _cleanup_free_ char *target = NULL; |
|
- const char *bn; |
|
- UnitType a, b; |
|
- |
|
- /* This is a symlink, let's read it. We read the link again, because last time |
|
- * we followed the link until resolution, and here we need to do one step. */ |
|
- |
|
- r = readlink_malloc(path, &target); |
|
- if (r < 0) |
|
- return r; |
|
- |
|
- bn = basename(target); |
|
- |
|
- if (unit_name_is_valid(info->name, UNIT_NAME_PLAIN)) { |
|
- |
|
- if (!unit_name_is_valid(bn, UNIT_NAME_PLAIN)) |
|
- return -EINVAL; |
|
- |
|
- } else if (unit_name_is_valid(info->name, UNIT_NAME_INSTANCE)) { |
|
- |
|
- if (!unit_name_is_valid(bn, UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)) |
|
- return -EINVAL; |
|
- |
|
- } else if (unit_name_is_valid(info->name, UNIT_NAME_TEMPLATE)) { |
|
- |
|
- if (!unit_name_is_valid(bn, UNIT_NAME_TEMPLATE)) |
|
- return -EINVAL; |
|
- } else |
|
- return -EINVAL; |
|
- |
|
- /* Enforce that the symlink destination does not |
|
- * change the unit file type. */ |
|
- |
|
- a = unit_name_to_type(info->name); |
|
- b = unit_name_to_type(bn); |
|
- if (a < 0 || b < 0 || a != b) |
|
- return -EINVAL; |
|
- |
|
- if (path_is_absolute(target)) |
|
- /* This is an absolute path, prefix the root so that we always deal with fully qualified paths */ |
|
- info->symlink_target = path_join(lp->root_dir, target); |
|
- else |
|
- /* This is a relative path, take it relative to the dir the symlink is located in. */ |
|
- info->symlink_target = file_in_same_dir(path, target); |
|
- if (!info->symlink_target) |
|
- return -ENOMEM; |
|
- |
|
+ else |
|
info->type = UNIT_FILE_TYPE_SYMLINK; |
|
- } |
|
|
|
return 0; |
|
}
|
|
|