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.
135 lines
5.9 KiB
135 lines
5.9 KiB
2 years ago
|
From c892aaa100fa04f877c7bf66e3cce846a7aa834c Mon Sep 17 00:00:00 2001
|
||
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
||
|
Date: Thu, 10 Mar 2022 20:26:59 +0100
|
||
|
Subject: [PATCH] shared/install: also check for self-aliases during
|
||
|
installation and ignore them
|
||
|
|
||
|
We had a check that was done in unit_file_resolve_symlink(). Let's move
|
||
|
the check to unit_validate_alias_symlink_or_warn(), which makes it available
|
||
|
to the code in install.c.
|
||
|
|
||
|
With this, unit_file_resolve_symlink() behaves almost the same. The warning
|
||
|
about "suspicious symlink" is done a bit later. I think this should be OK.
|
||
|
|
||
|
(cherry picked from commit f663e6468ff6f667a67fa1a0f9ca5c4962d4c605)
|
||
|
|
||
|
Related: #2082131
|
||
|
---
|
||
|
src/basic/unit-file.c | 26 ++++++++++----------------
|
||
|
src/shared/install.c | 9 +++++++++
|
||
|
test/test-systemctl-enable.sh | 6 ++----
|
||
|
3 files changed, 21 insertions(+), 20 deletions(-)
|
||
|
|
||
|
diff --git a/src/basic/unit-file.c b/src/basic/unit-file.c
|
||
|
index 105dacc1b2..142ba006f8 100644
|
||
|
--- a/src/basic/unit-file.c
|
||
|
+++ b/src/basic/unit-file.c
|
||
|
@@ -82,7 +82,8 @@ int unit_validate_alias_symlink_or_warn(int log_level, const char *filename, con
|
||
|
*
|
||
|
* -EINVAL is returned if the something is wrong with the source filename or the source unit type is
|
||
|
* not allowed to symlink,
|
||
|
- * -EXDEV if the target filename is not a valid unit name or doesn't match the source.
|
||
|
+ * -EXDEV if the target filename is not a valid unit name or doesn't match the source,
|
||
|
+ * -ELOOP for an alias to self.
|
||
|
*/
|
||
|
|
||
|
src = basename(filename);
|
||
|
@@ -111,6 +112,11 @@ int unit_validate_alias_symlink_or_warn(int log_level, const char *filename, con
|
||
|
|
||
|
/* dst checks */
|
||
|
|
||
|
+ if (streq(src, dst))
|
||
|
+ return log_debug_errno(SYNTHETIC_ERRNO(ELOOP),
|
||
|
+ "%s: unit self-alias: %s → %s, ignoring.",
|
||
|
+ filename, src, dst);
|
||
|
+
|
||
|
dst_name_type = unit_name_to_instance(dst, &dst_instance);
|
||
|
if (dst_name_type < 0)
|
||
|
return log_full_errno(log_level, dst_name_type == -EINVAL ? SYNTHETIC_ERRNO(EXDEV) : dst_name_type,
|
||
|
@@ -348,24 +354,12 @@ int unit_file_resolve_symlink(
|
||
|
if (r < 0)
|
||
|
return r;
|
||
|
|
||
|
- bool self_alias = streq(target_name, filename);
|
||
|
-
|
||
|
- if (is_path(tail))
|
||
|
- log_full(self_alias ? LOG_DEBUG : LOG_WARNING,
|
||
|
- "Suspicious symlink %s/%s→%s, treating as alias.",
|
||
|
- dir, filename, simplified);
|
||
|
-
|
||
|
r = unit_validate_alias_symlink_or_warn(LOG_NOTICE, filename, simplified);
|
||
|
if (r < 0)
|
||
|
return r;
|
||
|
-
|
||
|
- 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, target_name);
|
||
|
-
|
||
|
- log_debug("Unit file alias: %s/%s → %s", dir, filename, target_name);
|
||
|
+ if (is_path(tail))
|
||
|
+ log_warning("Suspicious symlink %s/%s→%s, treating as alias.",
|
||
|
+ dir, filename, simplified);
|
||
|
|
||
|
dst = resolve_destination_target ? TAKE_PTR(simplified) : TAKE_PTR(target_name);
|
||
|
}
|
||
|
diff --git a/src/shared/install.c b/src/shared/install.c
|
||
|
index f1a8b7eb9b..459e8a6951 100644
|
||
|
--- a/src/shared/install.c
|
||
|
+++ b/src/shared/install.c
|
||
|
@@ -1702,6 +1702,11 @@ int unit_file_verify_alias(
|
||
|
* ret_dst is set in cases where "instance propagation" happens, i.e. when the instance part is
|
||
|
* inserted into dst. It is not normally set, even on success, so that the caller can easily
|
||
|
* distinguish the case where instance propagation occurred.
|
||
|
+ *
|
||
|
+ * Returns:
|
||
|
+ * -EXDEV when the alias doesn't match the unit,
|
||
|
+ * -EUCLEAN when the name is invalid,
|
||
|
+ * -ELOOP when the alias it to the unit itself.
|
||
|
*/
|
||
|
|
||
|
const char *path_alias = strrchr(dst, '/');
|
||
|
@@ -1767,6 +1772,8 @@ int unit_file_verify_alias(
|
||
|
}
|
||
|
|
||
|
r = unit_validate_alias_symlink_or_warn(LOG_DEBUG, dst_updated ?: dst, info->name);
|
||
|
+ if (r == -ELOOP) /* -ELOOP means self-alias, which we (quietly) ignore */
|
||
|
+ return r;
|
||
|
if (r < 0) {
|
||
|
unit_file_changes_add(changes, n_changes,
|
||
|
r == -EINVAL ? -EXDEV : r,
|
||
|
@@ -1807,6 +1814,8 @@ static int install_info_symlink_alias(
|
||
|
}
|
||
|
|
||
|
q = unit_file_verify_alias(info, dst, &dst_updated, changes, n_changes);
|
||
|
+ if (q == -ELOOP)
|
||
|
+ continue;
|
||
|
if (q < 0) {
|
||
|
r = r < 0 ? r : q;
|
||
|
continue;
|
||
|
diff --git a/test/test-systemctl-enable.sh b/test/test-systemctl-enable.sh
|
||
|
index 32bc6e5ef7..4117436462 100644
|
||
|
--- a/test/test-systemctl-enable.sh
|
||
|
+++ b/test/test-systemctl-enable.sh
|
||
|
@@ -328,8 +328,7 @@ test ! -h "$root/etc/systemd/system/services.target.wants/templ1@two.service"
|
||
|
test ! -e "$root/etc/systemd/system/link4.service"
|
||
|
cat >"$root/etc/systemd/system/link4.service" <<EOF
|
||
|
[Install]
|
||
|
-# FIXME: self-alias should be ignored
|
||
|
-# Alias=link4.service
|
||
|
+Alias=link4.service
|
||
|
Alias=link4@.service
|
||
|
Alias=link4@inst.service
|
||
|
Alias=link4alias.service
|
||
|
@@ -372,8 +371,7 @@ test ! -h "$root/etc/systemd/system/link4alias2.service"
|
||
|
test ! -e "$root/etc/systemd/system/link5.service"
|
||
|
cat >"$root/etc/systemd/system/link5.service" <<EOF
|
||
|
[Install]
|
||
|
-# FIXME: self-alias should be ignored
|
||
|
-# Alias=link5.service
|
||
|
+Alias=link5.service
|
||
|
Alias=link5alias.service
|
||
|
Alias=link5alias2.service
|
||
|
EOF
|