From 56bc8e8eef5fcbfcf72dd1b3caa56b9186e1011d Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

Date: Fri, 25 Mar 2022 15:43:27 +0100

Subject: [PATCH] shared/install: when creating symlinks, accept different but

 equivalent symlinks

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

We would only accept "identical" links, but having e.g. a symlink

/usr/lib/systemd/system/foo-alias.service → /usr/lib/systemd/system/foo.service

when we're trying to create /usr/lib/systemd/system/foo-alias.service →

./foo.service is OK. This fixes an issue found in ubuntuautopkg package

installation, where we'd fail when enabling systemd-resolved.service, because

the existing alias was absolute, and (with the recent patches) we were trying

to create a relative one.

A test is added.

(For .wants/.requires symlinks we were already doing OK. A test is also

added, to verify.)

(cherry picked from commit 3fc53351dc8f37355f5a4ee8f922d3e13a5182c2)

Related: #2082131

---

 src/shared/install.c          | 59 ++++++++++++++++++++++++++---------

 test/test-systemctl-enable.sh | 39 +++++++++++++++++++++--

 2 files changed, 81 insertions(+), 17 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c

index d6951b805d..22b16ad453 100644

--- a/src/shared/install.c

+++ b/src/shared/install.c

@@ -423,21 +423,54 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang

 }

 /**

- * Checks if two paths or symlinks from wd are the same, when root is the root of the filesystem.

- * wc should be the full path in the host file system.

+ * Checks if two symlink targets (starting from src) are equivalent as far as the unit enablement logic is

+ * concerned. If the target is in the unit search path, then anything with the same name is equivalent.

+ * If outside the unit search path, paths must be identical.

  */

-static bool chroot_symlinks_same(const char *root, const char *wd, const char *a, const char *b) {

-        assert(path_is_absolute(wd));

+static int chroot_unit_symlinks_equivalent(

+                const LookupPaths *lp,

+                const char *src,

+                const char *target_a,

+                const char *target_b) {

+

+        assert(lp);

+        assert(src);

+        assert(target_a);

+        assert(target_b);

         /* This will give incorrect results if the paths are relative and go outside

          * of the chroot. False negatives are possible. */

-        if (!root)

-                root = "/";

+        const char *root = lp->root_dir ?: "/";

+        _cleanup_free_ char *dirname = NULL;

+        int r;

+

+        if (!path_is_absolute(target_a) || !path_is_absolute(target_b)) {

+                r = path_extract_directory(src, &dirname);

+                if (r < 0)

+                        return r;

+        }

-        a = strjoina(path_is_absolute(a) ? root : wd, "/", a);

-        b = strjoina(path_is_absolute(b) ? root : wd, "/", b);

-        return path_equal_or_files_same(a, b, 0);

+        _cleanup_free_ char *a = path_join(path_is_absolute(target_a) ? root : dirname, target_a);

+        _cleanup_free_ char *b = path_join(path_is_absolute(target_b) ? root : dirname, target_b);

+        if (!a || !b)

+                return log_oom();

+

+        r = path_equal_or_files_same(a, b, 0);

+        if (r != 0)

+                return r;

+

+        _cleanup_free_ char *a_name = NULL, *b_name = NULL;

+        r = path_extract_filename(a, &a_name);

+        if (r < 0)

+                return r;

+        r = path_extract_filename(b, &b_name);

+        if (r < 0)

+                return r;

+

+        return streq(a_name, b_name) &&

+               path_startswith_strv(a, lp->search_path) &&

+               path_startswith_strv(b, lp->search_path);

 }

 static int create_symlink(

@@ -448,7 +481,7 @@ static int create_symlink(

                 UnitFileChange **changes,

                 size_t *n_changes) {

-        _cleanup_free_ char *dest = NULL, *dirname = NULL;

+        _cleanup_free_ char *dest = NULL;

         const char *rp;

         int r;

@@ -489,11 +522,7 @@ static int create_symlink(

                 return r;

         }

-        dirname = dirname_malloc(new_path);

-        if (!dirname)

-                return -ENOMEM;

-

-        if (chroot_symlinks_same(lp->root_dir, dirname, dest, old_path)) {

+        if (chroot_unit_symlinks_equivalent(lp, new_path, dest, old_path)) {

                 log_debug("Symlink %s → %s already exists", new_path, dest);

                 return 1;

         }

diff --git a/test/test-systemctl-enable.sh b/test/test-systemctl-enable.sh

index 3b30f090a5..0f66af309a 100644

--- a/test/test-systemctl-enable.sh

+++ b/test/test-systemctl-enable.sh

@@ -39,8 +39,29 @@ test -h "$root/etc/systemd/system/default.target.wants/test1.service"

 test -h "$root/etc/systemd/system/special.target.requires/test1.service"

 "$systemctl" --root="$root" disable test1.service

-test ! -e "$root/etc/systemd/system/default.target.wants/test1.service"

-test ! -e "$root/etc/systemd/system/special.target.requires/test1.service"

+test ! -h "$root/etc/systemd/system/default.target.wants/test1.service"

+test ! -h "$root/etc/systemd/system/special.target.requires/test1.service"

+

+: '------enable when link already exists-----------------------'

+# We don't read the symlink target, so it's OK for the symlink to point

+# to something else. We should just silently accept this.

+

+mkdir -p "$root/etc/systemd/system/default.target.wants"

+mkdir -p "$root/etc/systemd/system/special.target.requires"

+ln -s /usr/lib/systemd/system/test1.service "$root/etc/systemd/system/default.target.wants/test1.service"

+ln -s /usr/lib/systemd/system/test1.service "$root/etc/systemd/system/special.target.requires/test1.service"

+

+"$systemctl" --root="$root" enable test1.service

+test -h "$root/etc/systemd/system/default.target.wants/test1.service"

+test -h "$root/etc/systemd/system/special.target.requires/test1.service"

+

+"$systemctl" --root="$root" reenable test1.service

+test -h "$root/etc/systemd/system/default.target.wants/test1.service"

+test -h "$root/etc/systemd/system/special.target.requires/test1.service"

+

+"$systemctl" --root="$root" disable test1.service

+test ! -h "$root/etc/systemd/system/default.target.wants/test1.service"

+test ! -h "$root/etc/systemd/system/special.target.requires/test1.service"

 : '------suffix guessing---------------------------------------'

 "$systemctl" --root="$root" enable test1

@@ -90,6 +111,20 @@ test ! -h "$root/etc/systemd/system/default.target.wants/test1.service"

 test ! -h "$root/etc/systemd/system/special.target.requires/test1.service"

 test ! -h "$root/etc/systemd/system/test1-goodalias.service"

+: '-------aliases when link already exists---------------------'

+cat >"$root/etc/systemd/system/test1a.service" <

+[Install]

+Alias=test1a-alias.service

+EOF

+

+ln -s /usr/lib/systemd/system/test1a.service "$root/etc/systemd/system/test1a-alias.service"

+

+"$systemctl" --root="$root" enable test1a.service

+test -h "$root/etc/systemd/system/test1a-alias.service"

+

+"$systemctl" --root="$root" disable test1a.service

+test ! -h "$root/etc/systemd/system/test1a-alias.service"

+

 : '-------also units-------------------------------------------'

 cat >"$root/etc/systemd/system/test2.socket" <

 [Install]