systemctl,manager: refuse linking unit files underneath the search paths

We treat symlinks to unit files outside of the search path differently from
symlinks to unit files *in* the search path. The former are "linked" unit
files, while the latter are enablement symlinks and such and will be removed
when disabling the unit.

The history of the check for in_search_path() is interesting: this condition
was added already in the first version of the code in
830964834f330836b9d33752e83de09d4f38da87. Since the beginning, matching
arguments would simply be ignored. I think this is dubious. The man page says:

> Link a unit file that is *not* in the unit file search paths
> into the unit file search path

But for backwards-compat, let's continue to silently do nothing for files
*in* the search path.

The case of symlinks to unit files underneath the search path, but in some
subdirectory, is less clear. We didn't check for this case, so it was
implicitly allowed. But that's just an oversight, we don't want to allow people
to create additional subhierarchies under our hierarchy. Let's check for this
case and refuse.

Closes #24605.

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index bdbaa9b58c62f49934404ea40c57aaf3ef188559..633873da27b8a3b855946f45c05899075b46a03e 100644 (file)
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -2171,6 +2171,11 @@ static int install_error(
                                               "Unit %s is transient or generated.", changes[i].path);
                         goto found;
 
+                case -ETXTBSY:
+                        r = sd_bus_error_setf(error, BUS_ERROR_UNIT_BAD_PATH,
+                                              "File %s is under the systemd unit hierarchy already.", changes[i].path);
+                        goto found;
+
                 case -EUCLEAN:
                         r = sd_bus_error_setf(error, BUS_ERROR_BAD_UNIT_SETTING,
                                               "\"%s\" is not a valid unit name.",

diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h
index c3c25d69c3feaa0e6d191b61a48370d0375d4788..d4a1fb689ebf2faba9129d2e9449dadeb6672684 100644 (file)
--- a/src/libsystemd/sd-bus/bus-common-errors.h
+++ b/src/libsystemd/sd-bus/bus-common-errors.h
@@ -20,6 +20,7 @@
 #define BUS_ERROR_UNIT_MASKED                  "org.freedesktop.systemd1.UnitMasked"
 #define BUS_ERROR_UNIT_GENERATED               "org.freedesktop.systemd1.UnitGenerated"
 #define BUS_ERROR_UNIT_LINKED                  "org.freedesktop.systemd1.UnitLinked"
+#define BUS_ERROR_UNIT_BAD_PATH                "org.freedesktop.systemd1.UnitBadPath"
 #define BUS_ERROR_JOB_TYPE_NOT_APPLICABLE      "org.freedesktop.systemd1.JobTypeNotApplicable"
 #define BUS_ERROR_NO_ISOLATION                 "org.freedesktop.systemd1.NoIsolation"
 #define BUS_ERROR_SHUTTING_DOWN                "org.freedesktop.systemd1.ShuttingDown"

diff --git a/src/shared/install.c b/src/shared/install.c
index cca8b0b0c802f8c22b2ccc1cfc4bf2d5ff0b6c98..6d91a02f115b480e084baab2a23e90a89db829f2 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -104,13 +104,19 @@ static int in_search_path(const LookupPaths *lp, const char *path) {
         _cleanup_free_ char *parent = NULL;
         int r;
 
-        assert(path);
+        /* Check if 'path' is in lp->search_path. */
 
-        r = path_extract_directory(path, &parent);
+        r = path_extract_directory(ASSERT_PTR(path), &parent);
         if (r < 0)
                 return r;
 
-        return path_strv_contains(lp->search_path, parent);
+        return path_strv_contains(ASSERT_PTR(lp)->search_path, parent);
+}
+
+static int underneath_search_path(const LookupPaths *lp, const char *path) {
+        /* Check if 'path' is underneath lp->search_path. */
+
+        return !!path_startswith_strv(ASSERT_PTR(path), ASSERT_PTR(lp)->search_path);
 }
 
 static const char* skip_root(const char *root_dir, const char *path) {
@@ -390,6 +396,10 @@ void install_changes_dump(int r, const char *verb, const InstallChange *changes,
                         err = log_error_errno(changes[i].type, "Failed to %s unit, unit %s is transient or generated.",
                                               verb, changes[i].path);
                         break;
+                case -ETXTBSY:
+                        err = log_error_errno(changes[i].type, "Failed to %s unit, file %s is under the systemd unit hierarchy already.",
+                                              verb, changes[i].path);
+                        break;
                 case -EBADSLT:
                         err = log_error_errno(changes[i].type, "Failed to %s unit, invalid specifier in \"%s\".",
                                               verb, changes[i].path);
@@ -2425,8 +2435,15 @@ int unit_file_link(
                 if (r < 0)
                         return install_changes_add(changes, n_changes, r, *file, NULL);
                 if (r > 0)
+                        /* A silent noop if the file is already in the search path. */
                         continue;
 
+                r = underneath_search_path(&lp, *file);
+                if (r > 0)
+                        r = -ETXTBSY;
+                if (r < 0)
+                        return install_changes_add(changes, n_changes, r, *file, NULL);
+
                 if (!GREEDY_REALLOC0(todo, n_todo + 2))
                         return -ENOMEM;