On Sun, Jun 15, 2025 at 04:25:15PM +0100, Harald van Dijk wrote:
> On 15/06/2025 16:09, Alexey Gladkov wrote:
> > On Sun, Jun 15, 2025 at 03:35:17PM +0100, Harald van Dijk wrote:
> >> On 15/06/2025 14:48, Alexey Gladkov wrote:
> >>> If a mounted image is used as root, overlayfs is typically used to
> >>> provide write access. In this case, it is safe to execute switch_root.
> >>
> >> But when overlayfs is used, this does not guarantee that it is safe, and
> >> does risk the exact sort of destructive behaviour that is meant to be
> >> protected against. It's only safe with specific uses of overlayfs.
> > 
> > Hm. The overlayfs by design does not allow you to change lower layers.
> > Also, the filesystem does not propagate changes to lower layers.
> > 
> > In which case if the whole overlayfs root can it be dangerous to do
> > remove a root content ?
> 
> When there's important data in the upper layer. In your use case, the 
> upper layer is tmpfs and in that case, sure, it makes sense, but that is 
> not guaranteed and is not being checked.

This is debatable because there might be something valuable on tmpfs which
is rootfs too, but I won't insist :)

> >> util-linux's version of switch_root does the same ramfs/tmpfs check that
> >> busybox does, but reacts to it differently: rather than exiting with an
> >> error for other file systems, it simply prints a warning and carries on
> >> without removing any files. Would doing that instead in busybox too be
> >> enough to work for your use case?
> > 
> > Yes, it will absolutely be better than a fatal error. I'm using
> > switch_root in initramfs and of course I'd like to clean up root, but this
> > way at least I can use switch_root from busybox.
> 
> That should be a fairly simple change, that should just be a matter of 
> changing the bb_(simple_)error_msg_and_die to the non-_and_die variant 
> to continue printing the message but carrying on, and then skipping over 
> the delete_contents("/", rootdev).
> 
> > Is it possible to add an option to delete content from root ?
> 
> Also a good suggestion.

Ok, great!

I'm not very familiar with the busybox codebase.
Does this patch look acceptable ?

-- 
Rgrds, legion

diff --git a/util-linux/switch_root.c b/util-linux/switch_root.c
index 14139736e..8898b6a29 100644
--- a/util-linux/switch_root.c
+++ b/util-linux/switch_root.c
@@ -172,48 +172,55 @@ static void drop_capabilities(char *string)
 }
 #endif
 
+enum {
+       OPT_force   = (1 << 0),
+       OPT_console = (1 << 1),
+       OPT_dropcap = (1 << 2),
+       OPT_dry_run = (1 << 3),
+};
+
 int switch_root_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int switch_root_main(int argc UNUSED_PARAM, char **argv)
 {
        char *newroot, *console = NULL;
        struct stat st;
        struct statfs stfs;
-       unsigned dry_run = 0;
        dev_t rootdev;
 
        // Parse args. '+': stop at first non-option
        if (ENABLE_SWITCH_ROOT && (!ENABLE_RUN_INIT || applet_name[0] == 's')) {
 //usage:#define switch_root_trivial_usage
-//usage:       "[-c CONSOLE_DEV] NEW_ROOT NEW_INIT [ARGS]"
+//usage:       "[-c CONSOLE_DEV] [-f] NEW_ROOT NEW_INIT [ARGS]"
 //usage:#define switch_root_full_usage "\n\n"
 //usage:       "Free initramfs and switch to another root fs:\n"
 //usage:       "chroot to NEW_ROOT, delete all in /, move NEW_ROOT to /,\n"
 //usage:       "execute NEW_INIT. PID must be 1. NEW_ROOT must be a 
mountpoint.\n"
+//usage:     "\n       -f      Do not check root filesystem type"
 //usage:     "\n       -c DEV  Reopen stdio to DEV after switch"
                getopt32(argv, "^+"
-                       "c:"
+                       "fc:"
                        "\0" "-2" /* minimum 2 args */,
                        &console
                );
        } else {
 #if ENABLE_RUN_INIT
 //usage:#define run_init_trivial_usage
-//usage:       "[-d CAP,CAP...] [-n] [-c CONSOLE_DEV] NEW_ROOT NEW_INIT [ARGS]"
+//usage:       "[-d CAP,CAP...] [-n] [-c CONSOLE_DEV] [-f] NEW_ROOT NEW_INIT 
[ARGS]"
 //usage:#define run_init_full_usage "\n\n"
 //usage:       "Free initramfs and switch to another root fs:\n"
 //usage:       "chroot to NEW_ROOT, delete all in /, move NEW_ROOT to /,\n"
 //usage:       "execute NEW_INIT. PID must be 1. NEW_ROOT must be a 
mountpoint.\n"
+//usage:     "\n       -f      Do not check root filesystem type"
 //usage:     "\n       -c DEV  Reopen stdio to DEV after switch"
 //usage:     "\n       -d CAPS Drop capabilities"
 //usage:     "\n       -n      Dry run"
                char *cap_list = NULL;
-               dry_run = getopt32(argv, "^+"
-                       "c:d:n"
+               getopt32(argv, "^+"
+                       "fc:d:n"
                        "\0" "-2" /* minimum 2 args */,
                        &console,
                        &cap_list
                );
-               dry_run >>= 2; // -n
                if (cap_list)
                        drop_capabilities(cap_list);
 #endif
@@ -230,7 +237,7 @@ int switch_root_main(int argc UNUSED_PARAM, char **argv)
                // Show usage, it says new root must be a mountpoint
                bb_show_usage();
        }
-       if (!dry_run && getpid() != 1) {
+       if (!(option_mask32 & OPT_dry_run) && getpid() != 1) {
                // Show usage, it says we must be PID 1
                bb_show_usage();
        }
@@ -242,13 +249,14 @@ int switch_root_main(int argc UNUSED_PARAM, char **argv)
                bb_error_msg_and_die("'%s' is not a regular file", "/init");
        }
        statfs("/", &stfs); // this never fails
-       if ((unsigned)stfs.f_type != RAMFS_MAGIC
+       if (!(option_mask32 & OPT_force)
+        && (unsigned)stfs.f_type != RAMFS_MAGIC
         && (unsigned)stfs.f_type != TMPFS_MAGIC
        ) {
                bb_simple_error_msg_and_die("root filesystem is not 
ramfs/tmpfs");
        }
 
-       if (!dry_run) {
+       if (!(option_mask32 & OPT_dry_run)) {
                // Zap everything out of rootdev
                delete_contents("/", rootdev);
 
@@ -272,7 +280,7 @@ int switch_root_main(int argc UNUSED_PARAM, char **argv)
                }
        }
 
-       if (dry_run) {
+       if (option_mask32 & OPT_dry_run) {
                // Does NEW_INIT look like it can be executed?
                //xstat(argv[0], &st);
                //if (!S_ISREG(st.st_mode))
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox

Reply via email to