From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Fri, 26 Aug 2022 10:02:43 +0000 (+0200)
Subject: shared/install: print warning when unmasking unit with cmdline mask
X-Git-Tag: v252-rc2~44^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F24461%2Fhead;p=thirdparty%2Fsystemd.git

shared/install: print warning when unmasking unit with cmdline mask

'systemctl unmask foo' will try to remove the symlink to /dev/null under /etc/.
But the unit may also be masked by a symlink under /run/generator, in particular
the one created by systemd-debug-generator based on systemd.mask=foo on the
kernel commandline. The unmask call cannot anything about this: even if it removed
the symlink from /run/generator, it'll be recreated on the next daemon-reload.
Thus, we can only warn about it.

Initially, I wanted to check if 'systemctl.mask' is defined on the kernel
command-line, but that's not effective, because such mask symlinks can be
created by other generators based on other conditions. Checking for runtime
mask is "dumber", but is more robust because it doesn't assume who created the
mask and why.

The handling of InstallInfo is the copied from install_info_symlink_wants().
It's pretty ugly, this whole code should be rewritten from scratch.

The message is printed, but the whole operation is still "successful". This
keep backwards compatibility: people might call unmask to remove filesystem
masks even if there's still a cmdline param in place. We allow 'systemctl
mask' to create such a mask, so 'unmask' should be able to remove it.

Fixes #22689.
---

diff --git a/src/shared/install.c b/src/shared/install.c
index 7cf73fe0aa4..e45ad01092f 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -352,6 +352,11 @@ void install_changes_dump(int r, const char *verb, const InstallChange *changes,
                         if (!quiet)
                                 log_info("Unit %s is masked, ignoring.", changes[i].path);
                         break;
+                case INSTALL_CHANGE_IS_MASKED_GENERATOR:
+                        if (!quiet)
+                                log_info("Unit %s is masked via a generator and cannot be unmasked.",
+                                         changes[i].path);
+                        break;
                 case INSTALL_CHANGE_IS_DANGLING:
                         if (!quiet)
                                 log_info("Unit %s is an alias to a unit that is not present, ignoring.",
@@ -2287,12 +2292,32 @@ int unit_file_unmask(
         bool dry_run = flags & UNIT_FILE_DRY_RUN;
 
         STRV_FOREACH(name, names) {
-                _cleanup_free_ char *path = NULL;
-
                 if (!unit_name_is_valid(*name, UNIT_NAME_ANY))
                         return -EINVAL;
 
-                path = path_make_absolute(*name, config_path);
+                /* If root_dir is set, we don't care about kernel commandline or generators.
+                 * But if it is not set, we need to check for interference. */
+                if (!root_dir) {
+                        _cleanup_(install_info_clear) InstallInfo info = {
+                                .name = *name,  /* We borrow *name temporarily… */
+                                .install_mode = _INSTALL_MODE_INVALID,
+                        };
+
+                        r = unit_file_search(NULL, &info, &lp, 0);
+                        if (r < 0) {
+                                if (r != -ENOENT)
+                                        log_debug_errno(r, "Failed to look up unit %s, ignoring: %m", info.name);
+                        } else {
+                                if (info.install_mode == INSTALL_MODE_MASKED &&
+                                    path_is_generator(&lp, info.path))
+                                        install_changes_add(changes, n_changes,
+                                                            INSTALL_CHANGE_IS_MASKED_GENERATOR, info.name, info.path);
+                        }
+
+                        TAKE_PTR(info.name);  /* … and give it back here */
+                }
+
+                _cleanup_free_ char *path = path_make_absolute(*name, config_path);
                 if (!path)
                         return -ENOMEM;
 
@@ -3645,6 +3670,7 @@ static const char* const install_change_table[_INSTALL_CHANGE_MAX] = {
         [INSTALL_CHANGE_SYMLINK]                 = "symlink",
         [INSTALL_CHANGE_UNLINK]                  = "unlink",
         [INSTALL_CHANGE_IS_MASKED]               = "masked",
+        [INSTALL_CHANGE_IS_MASKED_GENERATOR]     = "masked by generator",
         [INSTALL_CHANGE_IS_DANGLING]             = "dangling",
         [INSTALL_CHANGE_DESTINATION_NOT_PRESENT] = "destination not present",
         [INSTALL_CHANGE_AUXILIARY_FAILED]        = "auxiliary unit failed",
diff --git a/src/shared/install.h b/src/shared/install.h
index 5f43b21335c..1c910e87529 100644
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -32,6 +32,7 @@ enum {
         INSTALL_CHANGE_SYMLINK,
         INSTALL_CHANGE_UNLINK,
         INSTALL_CHANGE_IS_MASKED,
+        INSTALL_CHANGE_IS_MASKED_GENERATOR,
         INSTALL_CHANGE_IS_DANGLING,
         INSTALL_CHANGE_DESTINATION_NOT_PRESENT,
         INSTALL_CHANGE_AUXILIARY_FAILED,