From 890c14e343e028d55d96716baae14aaf1e74a467 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 16 May 2023 15:52:33 +0200 Subject: [PATCH 1/5] mount-util: keep fd to /proc/self/mountinfo continously open in umount_recursive() That way, if we end up unmounting /proc/ in our loop we can still operate correctly, since we don't have to go through /proc/ again to open the mount table again. --- src/shared/mount-util.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index b41672eb92..b7f3616a1d 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -44,19 +44,22 @@ #include "user-util.h" int umount_recursive(const char *prefix, int flags) { + _cleanup_fclose_ FILE *f = NULL; int n = 0, r; - bool again; /* Try to umount everything recursively below a directory. Also, take care of stacked mounts, and * keep unmounting them until they are gone. */ - do { + f = fopen("/proc/self/mountinfo", "re"); /* Pin the file, in case we unmount /proc/ as part of the logic here */ + if (!f) + return log_debug_errno(errno, "Failed to open /proc/self/mountinfo: %m"); + + for (;;) { _cleanup_(mnt_free_tablep) struct libmnt_table *table = NULL; _cleanup_(mnt_free_iterp) struct libmnt_iter *iter = NULL; + bool again = false; - again = false; - - r = libmount_parse("/proc/self/mountinfo", NULL, &table, &iter); + r = libmount_parse("/proc/self/mountinfo", f, &table, &iter); if (r < 0) return log_debug_errno(r, "Failed to parse /proc/self/mountinfo: %m"); @@ -89,7 +92,12 @@ int umount_recursive(const char *prefix, int flags) { break; } - } while (again); + + if (!again) + break; + + rewind(f); + } return n; } From 84bcb394c8147a29f26b87cf84a54a3d83588b5d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 16 May 2023 15:54:10 +0200 Subject: [PATCH 2/5] mount-util: extend umount_recursive() to optionally take list of dirs to exclude from the unmounting --- src/shared/mount-util.c | 14 +++++++++++++- src/shared/mount-util.h | 7 ++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index b7f3616a1d..c97a7c3188 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -43,7 +43,7 @@ #include "tmpfile-util.h" #include "user-util.h" -int umount_recursive(const char *prefix, int flags) { +int umount_recursive_full(const char *prefix, int flags, char **keep) { _cleanup_fclose_ FILE *f = NULL; int n = 0, r; @@ -64,6 +64,7 @@ int umount_recursive(const char *prefix, int flags) { return log_debug_errno(r, "Failed to parse /proc/self/mountinfo: %m"); for (;;) { + bool shall_keep = false; struct libmnt_fs *fs; const char *path; @@ -80,6 +81,17 @@ int umount_recursive(const char *prefix, int flags) { if (!path_startswith(path, prefix)) continue; + STRV_FOREACH(k, keep) + /* Match against anything in the path to the dirs to keep, or below the dirs to keep */ + if (path_startswith(path, *k) || path_startswith(*k, path)) { + shall_keep = true; + break; + } + if (shall_keep) { + log_debug("Not unmounting %s, referenced by keep list.", path); + continue; + } + if (umount2(path, flags | UMOUNT_NOFOLLOW) < 0) { log_debug_errno(errno, "Failed to umount %s, ignoring: %m", path); continue; diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h index 8a84d61622..b33e739b42 100644 --- a/src/shared/mount-util.h +++ b/src/shared/mount-util.h @@ -12,7 +12,12 @@ #include "macro.h" int repeat_unmount(const char *path, int flags); -int umount_recursive(const char *target, int flags); + +int umount_recursive_full(const char *target, int flags, char **keep); + +static inline int umount_recursive(const char *target, int flags) { + return umount_recursive_full(target, flags, NULL); +} int bind_remount_recursive_with_mountinfo(const char *prefix, unsigned long new_flags, unsigned long flags_mask, char **deny_list, FILE *proc_self_mountinfo); static inline int bind_remount_recursive(const char *prefix, unsigned long new_flags, unsigned long flags_mask, char **deny_list) { From ef742415e61822c5a746f1b0d29d1808f64680e5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 15 May 2023 21:23:55 +0200 Subject: [PATCH 3/5] mount-util: make "prefix" parameter optional for umount_recursive() When switching root via MS_MOVE there's no need to filter the mount table by prefix --- src/shared/mount-util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index c97a7c3188..6dbb65ce10 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -78,8 +78,10 @@ int umount_recursive_full(const char *prefix, int flags, char **keep) { if (!path) continue; - if (!path_startswith(path, prefix)) + if (prefix && !path_startswith(path, prefix)) { + log_debug("Not unmounting %s, outside of prefix: %s", path, prefix); continue; + } STRV_FOREACH(k, keep) /* Match against anything in the path to the dirs to keep, or below the dirs to keep */ From 4e9ef660e6c56a4098e8535b3368c6c96f8630b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 16 May 2023 15:41:48 +0200 Subject: [PATCH 4/5] test: add test for umount_recursive() --- src/test/meson.build | 5 ++- src/test/test-mount-util.c | 90 +++++++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/test/meson.build b/src/test/meson.build index 7f8de2a2ce..d7eb2a857d 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -117,7 +117,6 @@ simple_tests += files( 'test-mempool.c', 'test-mkdir.c', 'test-modhex.c', - 'test-mount-util.c', 'test-mountpoint-util.c', 'test-net-naming-scheme.c', 'test-nulstr-util.c', @@ -319,6 +318,10 @@ tests += [ 'sources' : files('test-mempress.c'), 'dependencies' : threads, }, + { + 'sources' : files('test-mount-util.c'), + 'dependencies' : libmount, + }, { 'sources' : files('test-netlink-manual.c'), 'dependencies' : libkmod, diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c index a395f0cc6d..2529b2f0ee 100644 --- a/src/test/test-mount-util.c +++ b/src/test/test-mount-util.c @@ -8,6 +8,7 @@ #include "fd-util.h" #include "fileio.h" #include "fs-util.h" +#include "libmount-util.h" #include "missing_magic.h" #include "missing_mount.h" #include "mkdir.h" @@ -406,7 +407,7 @@ TEST(make_mount_switch_root) { assert_se(touch(s) >= 0); for (int force_ms_move = 0; force_ms_move < 2; force_ms_move++) { - r = safe_fork("(switch-root", + r = safe_fork("(switch-root)", FORK_RESET_SIGNALS | FORK_CLOSE_ALL_FDS | FORK_DEATHSIG | @@ -431,6 +432,93 @@ TEST(make_mount_switch_root) { } } +TEST(umount_recursive) { + static const struct { + const char *prefix; + const char * const keep[3]; + } test_table[] = { + { + .prefix = NULL, + .keep = {}, + }, + { + .prefix = "/run", + .keep = {}, + }, + { + .prefix = NULL, + .keep = { "/dev/shm", NULL }, + }, + { + .prefix = "/dev", + .keep = { "/dev/pts", "/dev/shm", NULL }, + }, + }; + + int r; + + FOREACH_ARRAY(t, test_table, ELEMENTSOF(test_table)) { + + r = safe_fork("(umount-rec)", + FORK_RESET_SIGNALS | + FORK_CLOSE_ALL_FDS | + FORK_DEATHSIG | + FORK_WAIT | + FORK_REOPEN_LOG | + FORK_LOG | + FORK_NEW_MOUNTNS | + FORK_MOUNTNS_SLAVE, + NULL); + + if (r < 0 && ERRNO_IS_PRIVILEGE(r)) { + log_notice("Skipping umount_recursive() test, lacking privileges"); + return; + } + + assert_se(r >= 0); + if (r == 0) { /* child */ + _cleanup_(mnt_free_tablep) struct libmnt_table *table = NULL; + _cleanup_(mnt_free_iterp) struct libmnt_iter *iter = NULL; + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *k = NULL; + + /* Open /p/s/m file before we unmount everything (which might include /proc/) */ + f = fopen("/proc/self/mountinfo", "re"); + if (!f) { + log_error_errno(errno, "Faile to open /proc/self/mountinfo: %m"); + _exit(EXIT_FAILURE); + } + + assert_se(k = strv_join((char**) t->keep, " ")); + log_info("detaching just %s (keep: %s)", strna(t->prefix), strna(empty_to_null(k))); + + assert_se(umount_recursive_full(t->prefix, MNT_DETACH, (char**) t->keep) >= 0); + + r = libmount_parse("/proc/self/mountinfo", f, &table, &iter); + if (r < 0) { + log_error_errno(r, "Failed to parse /proc/self/mountinfo: %m"); + _exit(EXIT_FAILURE); + } + + for (;;) { + struct libmnt_fs *fs; + + r = mnt_table_next_fs(table, iter, &fs); + if (r == 1) + break; + if (r < 0) { + log_error_errno(r, "Failed to get next entry from /proc/self/mountinfo: %m"); + _exit(EXIT_FAILURE); + } + + log_debug("left after complete umount: %s", mnt_fs_get_target(fs)); + } + + _exit(EXIT_SUCCESS); + } + } +} + static int intro(void) { /* Create a dummy network interface for testing remount_sysfs(). */ (void) system("ip link add dummy-test-mnt type dummy"); From 268d1244e87a35ff8dff56c92ef375ebf69d462e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 15 May 2023 21:25:12 +0200 Subject: [PATCH 5/5] switch-root: when switching root via MS_MOVE unmount all remaining mounts first Let's try to unmount anything left, since if we don't they will remain as "shadow" mounts, hidden underneath our new root. This is only necessary when we transition into a new root via MS_MOVE. If we do so via pivot_root() this is not necessary as the kernel will get rid of the mounts anyway for us. --- src/shared/switch-root.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/shared/switch-root.c b/src/shared/switch-root.c index 1ee06c8ee4..2431facba0 100644 --- a/src/shared/switch-root.c +++ b/src/shared/switch-root.c @@ -114,6 +114,14 @@ int switch_root(const char *new_root, if (r < 0) { log_debug_errno(r, "Pivoting root file system failed, moving mounts instead: %m"); + /* If we have to use MS_MOVE let's first try to get rid of *all* mounts we can, with the + * exception of the path we want to switch to, plus everything leading to it and within + * it. This is necessary because unlike pivot_root() just moving the mount to the root via + * 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. */ + (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);