From c6c5d20de5a68797b49eef4486dca308b39e48ae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Aug 2023 12:15:25 +0200 Subject: [PATCH] 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. --- src/shared/switch-root.c | 3 +-- src/shared/switch-root.h | 1 - src/shutdown/shutdown.c | 5 ++--- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/shared/switch-root.c b/src/shared/switch-root.c index 3801fec6459..b620156c75b 100644 --- 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 357e1795560..ba0d280eba4 100644 --- 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 f4776099010..b976b7d8cf6 100644 --- 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: -- 2.47.3