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