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.
176 lines
7.2 KiB
176 lines
7.2 KiB
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" <<EOF |
|
+[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" <<EOF |
|
[Install]
|
|
|