core/mount: stop generating mount units for cred mounts

While @poettering wants to keep mount units for credential
mounts, this has brought nothing but pain in real life.

By generating mount units for each cred mount, we had trouble
with default dependencies on them, which causes their stop jobs
to race with unmounting through exec_context_destroy_credentials().
There were several attempts to workaround the problem, but
none seems very graceful: #26959, #28787, #28957, #31360, #32011.
Also, we want to carry over credentials for services that
survive soft-reboot to the new mount tree, and during the practice
the stop of mount units are irritating.

The mentioned problems are ultimately resolved by disabling
default deps: #32799. But after doing that, maybe the next question
should be "why do we generate these mount units at all?"

Let's revisit the whole concept here. First of all, the credential
dirs are supposed to be opaque to users, and hence nobody should
really reference to these mounts directly. Secondly, the lifetime
of credentials is strictly bound to the service units, but nothing
else. Moreover, as more and more users of credentials pop up,
we could end up with hundreds of such mount units, which is
something we handle poorly. And we emit useless UnitRemoved signals,
etc...

As discussed, it seems that eliminating these mount units
is the correct way to go. No real use cases are impacted,
and the lifetime management becomes sane again.

Replaces #32011

diff --git a/src/core/mount.c b/src/core/mount.c
index ebafcafa923b9ee9ad577b77b84babbadd8132b2..238e3eab501d72ceb324500b12fc4996267bd146 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -425,16 +425,16 @@ static bool mount_is_extrinsic(Unit *u) {
         return false;
 }
 
-static bool mount_is_credentials(Mount *m) {
+static bool mount_point_is_credentials(Manager *manager, const char *path) {
         const char *e;
 
-        assert(m);
+        assert(manager);
+        assert(path);
 
-        /* Returns true if this is a credentials mount. We don't want automatic dependencies on credential
-         * mounts, since they are managed by us for even the earliest services, and we never want anything to
-         * be ordered before them hence. */
+        /* Returns true if this is a credentials mount. We don't want to generate mount units for them,
+         * since their lifetime is strictly bound to services. */
 
-        e = path_startswith(m->where, UNIT(m)->manager->prefix[EXEC_DIRECTORY_RUNTIME]);
+        e = path_startswith(path, manager->prefix[EXEC_DIRECTORY_RUNTIME]);
         if (!e)
                 return false;
 
@@ -490,7 +490,7 @@ static int mount_add_default_ordering_dependencies(Mount *m, MountParameters *p,
                 return r;
 
         /* If this is a tmpfs mount then we have to unmount it before we try to deactivate swaps */
-        if (streq_ptr(p->fstype, "tmpfs") && !mount_is_credentials(m)) {
+        if (streq_ptr(p->fstype, "tmpfs")) {
                 r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET,
                                                 /* add_reference= */ true, mask);
                 if (r < 0)
@@ -578,12 +578,16 @@ static int mount_verify(Mount *m) {
                 return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC), "Where= setting doesn't match unit name. Refusing.");
 
         if (mount_point_is_api(m->where) || mount_point_ignore(m->where))
-                return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC), "Cannot create mount unit for API file system %s. Refusing.", m->where);
+                return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC),
+                                            "Cannot create mount unit for API file system '%s'. Refusing.", m->where);
+
+        if (mount_point_is_credentials(UNIT(m)->manager, m->where))
+                return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC),
+                                            "Cannot create mount unit for credential mount '%s'. Refusing.", m->where);
 
         p = get_mount_parameters_fragment(m);
         if (p && !p->what && !UNIT(m)->perpetual)
-                return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC),
-                                            "What= setting is missing. Refusing.");
+                return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC), "What= setting is missing. Refusing.");
 
         if (m->exec_context.pam_name && m->kill_context.kill_mode != KILL_CONTROL_GROUP)
                 return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC), "Unit has PAM enabled. Kill mode must be set to control-group'. Refusing.");
@@ -604,9 +608,6 @@ static int mount_add_non_exec_dependencies(Mount *m) {
         if (!m->where)
                 return 0;
 
-        if (mount_is_credentials(m))
-                UNIT(m)->default_dependencies = false;
-
         /* Adds in all dependencies directly responsible for ordering the mount, as opposed to dependencies
          * resulting from the ExecContext and such. */
 
@@ -1840,9 +1841,9 @@ static int mount_setup_unit(
         assert(options);
         assert(fstype);
 
-        /* Ignore API mount points. They should never be referenced in
-         * dependencies ever. */
-        if (mount_point_is_api(where) || mount_point_ignore(where))
+        /* Ignore API and credential mount points. They should never be referenced in dependencies ever.
+         * Also check the comment for mount_point_is_credentials. */
+        if (mount_point_is_api(where) || mount_point_ignore(where) || mount_point_is_credentials(m, where))
                 return 0;
 
         if (streq(fstype, "autofs"))