Merge pull request #14090 from poettering/clonenewns-fix

make sure systemd-logind.service can start if unshare() is blocked

src/basic/errno-util.h

@@ -94,3 +94,10 @@ static inline bool ERRNO_IS_NOT_SUPPORTED(int r) {
                       ENOTTY,
                       ENOSYS);
 }
+
+/* Two different errors for access problems */
+static inline bool ERRNO_IS_PRIVILEGE(int r) {
+        return IN_SET(abs(r),
+                      EACCES,
+                      EPERM);
+}

src/core/execute.c

@@ -2459,6 +2459,40 @@ finish:
         return r;
 }
 
+static bool insist_on_sandboxing(
+                const ExecContext *context,
+                const char *root_dir,
+                const char *root_image,
+                const BindMount *bind_mounts,
+                size_t n_bind_mounts) {
+
+        size_t i;
+
+        assert(context);
+        assert(n_bind_mounts == 0 || bind_mounts);
+
+        /* Checks whether we need to insist on fs namespacing. i.e. whether we have settings configured that
+         * would alter the view on the file system beyond making things read-only or invisble, i.e. would
+         * rearrange stuff in a way we cannot ignore gracefully. */
+
+        if (context->n_temporary_filesystems > 0)
+                return true;
+
+        if (root_dir || root_image)
+                return true;
+
+        if (context->dynamic_user)
+                return true;
+
+        /* If there are any bind mounts set that don't map back onto themselves, fs namespacing becomes
+         * essential. */
+        for (i = 0; i < n_bind_mounts; i++)
+                if (!path_equal(bind_mounts[i].source, bind_mounts[i].destination))
+                        return true;
+
+        return false;
+}
+
 static int apply_mount_namespace(
                 const Unit *u,
                 const ExecCommand *command,
@@ -2545,28 +2579,28 @@ static int apply_mount_namespace(
                             DISSECT_IMAGE_DISCARD_ON_LOOP,
                             error_path);
 
-        bind_mount_free_many(bind_mounts, n_bind_mounts);
-
         /* If we couldn't set up the namespace this is probably due to a missing capability. setup_namespace() reports
          * that with a special, recognizable error ENOANO. In this case, silently proceed, but only if exclusively
          * sandboxing options were used, i.e. nothing such as RootDirectory= or BindMount= that would result in a
          * completely different execution environment. */
         if (r == -ENOANO) {
-                if (n_bind_mounts == 0 &&
-                    context->n_temporary_filesystems == 0 &&
-                    !root_dir && !root_image &&
-                    !context->dynamic_user) {
+                if (insist_on_sandboxing(
+                                    context,
+                                    root_dir, root_image,
+                                    bind_mounts,
+                                    n_bind_mounts)) {
+                        log_unit_debug(u, "Failed to set up namespace, and refusing to continue since the selected namespacing options alter mount environment non-trivially.\n"
+                                       "Bind mounts: %zu, temporary filesystems: %zu, root directory: %s, root image: %s, dynamic user: %s",
+                                       n_bind_mounts, context->n_temporary_filesystems, yes_no(root_dir), yes_no(root_image), yes_no(context->dynamic_user));
+
+                        r = -EOPNOTSUPP;
+                } else {
                         log_unit_debug(u, "Failed to set up namespace, assuming containerized execution and ignoring.");
-                        return 0;
+                        r = 0;
                 }
-
-                log_unit_debug(u, "Failed to set up namespace, and refusing to continue since the selected namespacing options alter mount environment non-trivially.\n"
-                               "Bind mounts: %zu, temporary filesystems: %zu, root directory: %s, root image: %s, dynamic user: %s",
-                               n_bind_mounts, context->n_temporary_filesystems, yes_no(root_dir), yes_no(root_image), yes_no(context->dynamic_user));
-
-                return -EOPNOTSUPP;
         }
 
+        bind_mount_free_many(bind_mounts, n_bind_mounts);
         return r;
 }
 
@@ -3414,8 +3448,12 @@ static int exec_child(
         if (context->protect_hostname) {
                 if (ns_type_supported(NAMESPACE_UTS)) {
                         if (unshare(CLONE_NEWUTS) < 0) {
-                                *exit_status = EXIT_NAMESPACE;
-                                return log_unit_error_errno(unit, errno, "Failed to set up UTS namespacing: %m");
+                                if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) {
+                                        *exit_status = EXIT_NAMESPACE;
+                                        return log_unit_error_errno(unit, errno, "Failed to set up UTS namespacing: %m");
+                                }
+
+                                log_unit_warning(unit, "ProtectHostname=yes is configured, but UTS namespace setup is prohibited (container manager?), ignoring namespace setup.");
                         }
                 } else
                         log_unit_warning(unit, "ProtectHostname=yes is configured, but the kernel does not support UTS namespaces, ignoring namespace setup.");

test/test-execute/exec-readonlypaths-with-bindpaths.service

@@ -3,7 +3,6 @@ Description=Test for ReadOnlyPaths=
 
 [Service]
 ReadOnlyPaths=/etc -/i-dont-exist /usr
-# From 6c47cd7d3bf35c8158a0737f34fe2c5dc95e72d6, RuntimeDirectory= implies BindPaths=.
-RuntimeDirectory=foo
+BindPaths=/etc:/tmp/etc2
 ExecStart=/bin/sh -x -c 'test ! -w /etc && test ! -w /usr && test ! -e /i-dont-exist && test -w /var'
 Type=oneshot