[PATCH] switch_root: remove /init check
We were having some difficulty switching out of our custom initramfs
into the final filesystem, with the error "message '/init' is not a
regular file". We were confused as to why it was looking for `/init`
-- we didn't have `/init`, neither in our initrd nor in the
destination filesystem -- we were using the rdinit= command line
parameter to provide an alternative path for the init in the initrd,
and the target init was determined by userspace.
Thankfully, the error message was fairly clear and easy to find in the
source!
We thus propose removing this check, since not all initrds have their
init at /init -- setting rdinit= on the kernel command line allows
using an alternative path. Thus, this check can prevent switching root
in a scenario where it would be perfectly appropriate.
---
util-linux/switch_root.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/util-linux/switch_root.c b/util-linux/switch_root.c
index 14139736e..ef6669f5c 100644
--- a/util-linux/switch_root.c
+++ b/util-linux/switch_root.c
@@ -238,9 +238,6 @@ int switch_root_main(int argc UNUSED_PARAM, char **argv)
// Additional sanity checks: we're about to rm -rf /, so be REALLY SURE
// we mean it. I could make this a CONFIG option, but I would get email
// from all the people who WILL destroy their filesystems.
- if (stat("/init", &st) != 0 || !S_ISREG(st.st_mode)) {
- bb_error_msg_and_die("'%s' is not a regular file", "/init");
- }
statfs("/", &stfs); // this never fails
if ((unsigned)stfs.f_type != RAMFS_MAGIC
&& (unsigned)stfs.f_type != TMPFS_MAGIC
--
2.42.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] switch_root: remove /init check
Linus Heckemann 於 2024年4月19日 星期五寫道：
> We were having some difficulty switching out of our custom initramfs
> into the final filesystem, with the error "message '/init' is not a
> regular file". We were confused as to why it was looking for `/init`
> -- we didn't have `/init`, neither in our initrd nor in the
> destination filesystem -- we were using the rdinit= command line
> parameter to provide an alternative path for the init in the initrd,
> and the target init was determined by userspace.
>
> Thankfully, the error message was fairly clear and easy to find in the
> source!
>
> We thus propose removing this check, since not all initrds have their
> init at /init -- setting rdinit= on the kernel command line allows
> using an alternative path. Thus, this check can prevent switching root
> in a scenario where it would be perfectly appropriate.
> ---
> util-linux/switch_root.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/util-linux/switch_root.c b/util-linux/switch_root.c
> index 14139736e..ef6669f5c 100644
> --- a/util-linux/switch_root.c
> +++ b/util-linux/switch_root.c
> @@ -238,9 +238,6 @@ int switch_root_main(int argc UNUSED_PARAM, char
**argv)
> // Additional sanity checks: we're about to rm -rf /, so be
REALLY SURE
> // we mean it. I could make this a CONFIG option, but I would get
email
> // from all the people who WILL destroy their filesystems.
> - if (stat("/init", &st) != 0 || !S_ISREG(st.st_mode)) {
> - bb_error_msg_and_die("'%s' is not a regular file",
"/init");
> - }
> statfs("/", &stfs); // this never fails
> if ((unsigned)stfs.f_type != RAMFS_MAGIC
> && (unsigned)stfs.f_type != TMPFS_MAGIC
> --
> 2.42.0
Did you read the code comments on the lines above what you deleted? It's a
sanity check before deleting everything on the "/" filesystem.
Perhaps a better approach is to check the existence of what's specified in
the "rdinit" parameter instead.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] switch_root: remove /init check
Kang-Che Sung writes:
> Linus Heckemann 於 2024年4月19日 星期五寫道：
>> We were having some difficulty switching out of our custom initramfs
>> into the final filesystem, with the error "message '/init' is not a
>> regular file". We were confused as to why it was looking for `/init`
>> -- we didn't have `/init`, neither in our initrd nor in the
>> destination filesystem -- we were using the rdinit= command line
>> parameter to provide an alternative path for the init in the initrd,
>> and the target init was determined by userspace.
>>
>> Thankfully, the error message was fairly clear and easy to find in the
>> source!
>>
>> We thus propose removing this check, since not all initrds have their
>> init at /init -- setting rdinit= on the kernel command line allows
>> using an alternative path. Thus, this check can prevent switching root
>> in a scenario where it would be perfectly appropriate.
>> ---
>> util-linux/switch_root.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/util-linux/switch_root.c b/util-linux/switch_root.c
>> index 14139736e..ef6669f5c 100644
>> --- a/util-linux/switch_root.c
>> +++ b/util-linux/switch_root.c
>> @@ -238,9 +238,6 @@ int switch_root_main(int argc UNUSED_PARAM, char
> **argv)
>> // Additional sanity checks: we're about to rm -rf /, so be
> REALLY SURE
>> // we mean it. I could make this a CONFIG option, but I would get
> email
>> // from all the people who WILL destroy their filesystems.
>> - if (stat("/init", &st) != 0 || !S_ISREG(st.st_mode)) {
>> - bb_error_msg_and_die("'%s' is not a regular file",
> "/init");
>> - }
>> statfs("/", &stfs); // this never fails
>> if ((unsigned)stfs.f_type != RAMFS_MAGIC
>> && (unsigned)stfs.f_type != TMPFS_MAGIC
>> --
>> 2.42.0
>
> Did you read the code comments on the lines above what you deleted? It's a
> sanity check before deleting everything on the "/" filesystem.
I did! I don't really see the existence of /init being so critical for
this check given that we check below that it's a ramfs or tmpfs, which
seems to me to be enough that people won't be destroying filesystems
they cared a great deal about.
> Perhaps a better approach is to check the existence of what's specified in
> the "rdinit" parameter instead.
That would introduce an additional dependency on /proc being mounted and
require additional parsing. I don't think the check is that necessary,
again because we have the /-is-ramfs-or-tmpfs check. But if you do think
we need it I can rewrite the patch to check for rdinit= on cmdline as well.
Cheers
Linus
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] switch_root: remove /init check
Linus Heckemann 於 2024年4月23日 星期二寫道： > > I don't really see the existence of /init being so critical for > this check given that we check below that it's a ramfs or tmpfs, which > seems to me to be enough that people won't be destroying filesystems > they cared a great deal about. > The ramfs/tmpfs check was good as it won't destroy any _permanent_ file the user could have, but it wouldn't hurt either when there is another sanity check. >> Perhaps a better approach is to check the existence of what's specified in >> the "rdinit" parameter instead. > > That would introduce an additional dependency on /proc being mounted and > require additional parsing. I don't think the check is that necessary, > again because we have the /-is-ramfs-or-tmpfs check. But if you do think > we need it I can rewrite the patch to check for rdinit= on cmdline as well. > /proc should be mounted by most init systems anyway. But we can skip the check when /proc doesn't exist, just in case. The logic would be roughly like this: If "/proc/cmdline" exists Read the "rdinit" parameter from "/proc/cmdline"; if it's unspecified, default to "/init" If the file in "rdinit" doesn't exist, stop. Else Skip the "rdinit" existence check and continue the switch_root process ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] switch_root: remove /init check
Kang-Che Sung writes: > Linus Heckemann 於 2024年4月23日 星期二寫道： >> >> I don't really see the existence of /init being so critical for >> this check given that we check below that it's a ramfs or tmpfs, which >> seems to me to be enough that people won't be destroying filesystems >> they cared a great deal about. >> > > The ramfs/tmpfs check was good as it won't destroy any _permanent_ file the > user could have, but it wouldn't hurt either when there is another sanity > check. > > >>> Perhaps a better approach is to check the existence of what's specified > in >>> the "rdinit" parameter instead. >> >> That would introduce an additional dependency on /proc being mounted and >> require additional parsing. I don't think the check is that necessary, >> again because we have the /-is-ramfs-or-tmpfs check. But if you do think >> we need it I can rewrite the patch to check for rdinit= on cmdline as > well. >> > > /proc should be mounted by most init systems anyway. But we can skip the > check when /proc doesn't exist, just in case. > > The logic would be roughly like this: > > If "/proc/cmdline" exists >Read the "rdinit" parameter from "/proc/cmdline"; if it's unspecified, > default to "/init" >If the file in "rdinit" doesn't exist, stop. > Else >Skip the "rdinit" existence check and continue the switch_root process I was having a look at this again, and I'm not sure reimplementing the kernel command line parsing logic[1][2] is really desirable. I still think removing the check completely would be best, but I'm fine with just dropping this and keeping the patch only on my end or working around it with `touch /init` if you're not convinced :) Linus [1]: https://lxr.linux.no/#linux+v6.7.1/kernel/params.c#L162 [2]: https://lxr.linux.no/#linux+v6.7.1/lib/cmdline.c#L227 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] switch_root: remove /init check
On 31/05/2024 00.08, Linus Heckemann wrote: > I was having a look at this again, and I'm not sure reimplementing the > kernel command line parsing logic[1][2] is really desirable. FWIW, I'm with Linus. It's also odd for busybox to have a check that the util-linux version of this doesn't. For the original patch: Acked-by: Rasmus Villemoes ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
