a

2 files changed, 36 insertions, 49 deletions

diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c
index 2229f4b42a..0c4bf1b8ec 100644
--- a/src/nspawn/nspawn-cgroup.c
+++ b/src/nspawn/nspawn-cgroup.c
@@ -626,6 +626,7 @@ int cgroup_decide_mounts(
                 CGMounts *ret_mounts,
                 CGroupUnified outer_cgver, CGroupUnified inner_cgver,
                 bool use_cgns) {
+        inner_cgver = CGROUP_UNIFIED_INHERIT;
         switch (inner_cgver) {
         case CGROUP_UNIFIED_INHERIT:
                 return cgroup_decide_mounts_inherit(ret_mounts);
@@ -657,50 +658,48 @@ static int cgroup_mount_cg(
         const bool rw = (use_userns && use_cgns) || fstype == CGMOUNT_CGROUP2 || streq(mountpoint, "/sys/fs/cgroup/systemd");
         int r;
 
-        /* First the base mount; this is always RW, as to not change the superblock settings */
         r = mount_verbose(LOG_ERR, "cgroup", mountpoint, fstype == CGMOUNT_CGROUP1 ? "cgroup" : "cgroup2",
-                          MS_NOSUID|MS_NOEXEC|MS_NODEV, opts);
+                          MS_NOSUID|MS_NOEXEC|MS_NODEV| ((!rw||!use_cgns) ? MS_RDONLY : 0), opts);
         if (r < 0)
                 return r;
 
-        /* Now, if nescessary, we remount RO */
-        if (rw) {
-                if (!use_cgns) {
-                        /* emulate cgns by mounting everything but our subcgroup RO */
-                        const char *cgpath;
-                        char *cgroup = NULL;
-                        if (fstype == CGMOUNT_CGROUP2) {
-                                rewind(cgfile);
-                                r = cg_pid_get_path_internal(NULL, cgfile, &cgroup);
-                                if (r < 0)
-                                        return log_error_errno(r, "Failed to get child's cgroup v2 path");
-                        } else {
-                                const char *scontroller, *state;
-                                size_t controller_len;
-                                FOREACH_WORD_SEPARATOR(scontroller, controller_len, opts, ",", state) {
-                                        _cleanup_free_ const char *controller = strndup(scontroller, controller_len);
-                                        rewind(cgfile);
-                                        if (cg_pid_get_path_internal(controller, cgfile, &cgroup) == 0)
-                                                break;
-                                }
-                                if (!cgroup)
-                                        return log_error_errno(EBADMSG, "Failed to associate mounted cgroup hierarchy %s with numbered cgroup hierarchy", mountpoint);
-                        }
-                        cgpath = prefix_roota(mountpoint, cgroup);
+        if (rw && !use_cgns) {
+                /* emulate cgns by mounting everything but our subcgroup RO */
+                const char *rwmountpoint = strjoina(mountpoint, ".");
 
-                        r = mount_verbose(LOG_ERR, cgpath, cgpath, NULL, MS_BIND, NULL);
-                        if (r < 0)
-                                return r;
-                        r = mount_verbose(LOG_ERR, NULL, mountpoint, NULL,
-                                          MS_BIND|MS_REMOUNT|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, NULL);
+                char *cgroup = NULL;
+                if (fstype == CGMOUNT_CGROUP2) {
+                        rewind(cgfile);
+                        r = cg_pid_get_path_internal(NULL, cgfile, &cgroup);
                         if (r < 0)
-                                return r;
+                                return log_error_errno(r, "Failed to get child's cgroup v2 path");
+                } else {
+                        const char *scontroller, *state;
+                        size_t controller_len;
+                        FOREACH_WORD_SEPARATOR(scontroller, controller_len, opts, ",", state) {
+                                _cleanup_free_ const char *controller = strndup(scontroller, controller_len);
+                                rewind(cgfile);
+                                if (cg_pid_get_path_internal(controller, cgfile, &cgroup) == 0)
+                                        break;
+                        }
+                        if (!cgroup)
+                                return log_error_errno(EBADMSG, "Failed to associate mounted cgroup hierarchy %s with numbered cgroup hierarchy", mountpoint);
                 }
-        } else {
-                r = mount_verbose(LOG_ERR, NULL, mountpoint, NULL,
-                                  MS_BIND|MS_REMOUNT|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, NULL);
+
+                (void) mkdir(rwmountpoint, 0755);
+                r = mount_verbose(LOG_ERR, "cgroup", rwmountpoint, fstype == CGMOUNT_CGROUP1 ? "cgroup" : "cgroup2",
+                                  MS_NOSUID|MS_NOEXEC|MS_NODEV, opts);
+                if (r < 0)
+                        return r;
+                r = mount_verbose(LOG_ERR, strjoina(rwmountpoint, cgroup), strjoina(mountpoint, cgroup), NULL, MS_BIND, NULL);
                 if (r < 0)
                         return r;
+                r = umount_verbose(rwmountpoint);
+                if (r < 0)
+                        return r;
+                r = rmdir(rwmountpoint);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to rmdir temporary mountpoint %s: %m", rwmountpoint);
         }
 
         return 0;
diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c
index 68d844bfaf..f3fb0922c3 100644
--- a/src/nspawn/nspawn-mount.c
+++ b/src/nspawn/nspawn-mount.c
@@ -284,20 +284,8 @@ int mount_sysfs(const char *dest) {
         x = prefix_roota(top, "/fs/kdbus");
         (void) mkdir_p(x, 0755);
 
-        /* We need to ensure that /sys/fs/cgroup exists before we remount /sys read-only.
-         *
-         * If !use_cgns, then this was already done by the outer child; so we only need to do it here it if use_cgns.
-         * This function doesn't know whether use_cgns, but !cg_ns_supported()⇒!use_cgns, so we can "optimize" the case
-         * where we _know_ !use_cgns, and deal with a no-op mkdir_p() in the false-positive where cgns_supported() but
-         * !use_cgns.
-         *
-         * But is it really much of an optimization?  We're potentially spending an access(2) (cg_ns_supported() could
-         * be cached from a previous call) to potentially save an lstat(2) and mkdir(2); and all of them are on virtual
-         * fileystems, so they should all be pretty cheap. */
-        if (cg_ns_supported()) { /* if (use_cgns) { */
-                x = prefix_roota(top, "/fs/cgroup");
-                (void) mkdir_p(x, 0755);
-        }
+        x = prefix_roota(top, "/fs/cgroup");
+        (void) mkdir_p(x, 0755);
 
         return mount_verbose(LOG_ERR, NULL, top, NULL,
                              MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT, NULL);