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" <

 [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" <

 [Install]

-# FIXME: self-alias should be ignored

-# Alias=link5.service

+Alias=link5.service

 Alias=link5alias.service

 Alias=link5alias2.service

 EOF