Revert "shutdown: do not umount recursively before MS_MOVE"

This reverts commit 6b219b74de53729249956221a971047aab7c96e0.

This commit doesn't look right to me. We have to unmount everything
recursively *before* we MS_MOVE because the MS_MOVE will not get rid of
it for us, and we simply cannot access these mounts after the MS_MOVE is
complete anymore.

This is a fundamental difference between MS_MOVE and pivot_root(). The
latter repivots the entire mount table getting rid of anything outside
of the new root. MS_MOVE otoh just mounts a bunch of mount points to the
top, leaving in place whatever might be underneath it.

Thus, if we go through the MS_MOVE codepath we must unmount everything
explicitly before doing so because otherwise the mounts will be pinned
forever, but be entirely invisble to userspace.

diff --git a/src/shared/switch-root.c b/src/shared/switch-root.c
index 3801fec6459876f4d02a5bb1fa6b586ca90680de..b620156c75be0558ad3062b44f810e8e0edee8aa 100644 (file)
--- a/src/shared/switch-root.c
+++ b/src/shared/switch-root.c
@@ -185,8 +185,7 @@ int switch_root(const char *new_root,
                  * MS_MOVE won't magically unmount anything below it. Once the chroot() succeeds the mounts
                  * below would still be around but invisible to us, because not accessible via
                  * /proc/self/mountinfo. Hence, let's clean everything up first, as long as we still can. */
-                if (!FLAGS_SET(flags, SWITCH_ROOT_SKIP_RECURSIVE_UMOUNT))
-                        (void) umount_recursive_full(NULL, MNT_DETACH, STRV_MAKE(new_root));
+                (void) umount_recursive_full(NULL, MNT_DETACH, STRV_MAKE(new_root));
 
                 if (mount(".", "/", NULL, MS_MOVE, NULL) < 0)
                         return log_error_errno(errno, "Failed to move %s to /: %m", new_root);

diff --git a/src/shared/switch-root.h b/src/shared/switch-root.h
index 357e17955608e874e67fd3a01bcf7b2fe83a3993..ba0d280eba4ec9f28e926a7e04f0038ba5d29e37 100644 (file)
--- a/src/shared/switch-root.h
+++ b/src/shared/switch-root.h
@@ -8,7 +8,6 @@ typedef enum SwitchRootFlags {
                                                      * that it is backed by non-persistent tmpfs/ramfs/… */
         SWITCH_ROOT_DONT_SYNC             = 1 << 1, /* don't call sync() immediately before switching root */
         SWITCH_ROOT_RECURSIVE_RUN         = 1 << 2, /* move /run/ with MS_REC from old to new root */
-        SWITCH_ROOT_SKIP_RECURSIVE_UMOUNT = 1 << 3, /* do not umount recursively on move */
 } SwitchRootFlags;
 
 int switch_root(const char *new_root, const char *old_root_after, SwitchRootFlags flags);

diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c
index f4776099010ca438a3798b5b92095b553aeb77f9..b976b7d8cf60d299cc96d4a8192dddfc62003f34 100644 (file)
--- a/src/shutdown/shutdown.c
+++ b/src/shutdown/shutdown.c
@@ -169,12 +169,11 @@ static int switch_root_initramfs(void) {
          * Disable sync() during switch-root, we after all sync'ed here plenty, and a dumb sync (as opposed
          * to the "smart" sync() we did here that looks at progress parameters) would defeat much of our
          * efforts here. As the new root will be /run/initramfs/, it is not necessary to mount /run/
-         * recursively. Also, do not umount filesystems before MS_MOVE, as that should be done by ourself. */
+         * recursively. */
         return switch_root(
                         /* new_root= */ "/run/initramfs",
                         /* old_root_after= */ "/oldroot",
-                        /* flags= */ SWITCH_ROOT_DONT_SYNC |
-                                     SWITCH_ROOT_SKIP_RECURSIVE_UMOUNT);
+                        /* flags= */ SWITCH_ROOT_DONT_SYNC);
 }
 
 /* Read the following fields from /proc/meminfo: