From 56bc8e8eef5fcbfcf72dd1b3caa56b9186e1011d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= 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" <"$root/etc/systemd/system/test2.socket" <