Page MenuHomeFreeBSD

Reroot support, userspace part.
ClosedPublic

Authored by trasz on Sep 18 2015, 5:35 PM.

Details

Summary

Userspace part of reroot support. This makes it possible to change
the root filesystem without full reboot, using "reboot -r". This makes
it possible to eg. boot from a temporary md_image preloaded by loader(8),
setup an iSCSI session, and continue booting from rootfs mounted over
iSCSI.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

trasz updated this revision to Diff 8834.Sep 18 2015, 5:35 PM
trasz retitled this revision from to Reroot support, userspace part..
trasz updated this object.
trasz edited the test plan for this revision. (Show Details)
trasz added a reviewer: kib.Sep 18 2015, 5:36 PM
kib edited edge metadata.Sep 18 2015, 6:02 PM
kib added a subscriber: emaste.

For reference kernel part is D2698/rS287964

kib added inline comments.Sep 24 2015, 6:27 PM
sbin/init/init.c
233 ↗(On Diff #8834)

Wouldn't it be somewhat more consistent to use 'R' for option ?

391 ↗(On Diff #8834)

Shouldn't some additional checks be done there ? E.g. verify that something is mounted (as a compliment to kernel, to avoid dangerous syscall), and that the type of the mounted fs is expected one (tmpfs) ?

This again raises the question of the MNT flag that could be checked to ensure that we unmount our fs.

724 ↗(On Diff #8834)

Style: do not use initialization in local declarations.

727 ↗(On Diff #8834)

Unneeded empty line.

746 ↗(On Diff #8834)

Same.

753 ↗(On Diff #8834)

Style. Put error declaration last.

757 ↗(On Diff #8834)

I hoped that userpace part of reroot kills all user processes except init. Apparently it does not.
This exacerbates the issue of the kernel root remount being fragile and lacking any protection against parallel modifications of the mount list.

781 ↗(On Diff #8834)

Unneeded empty line.

1753 ↗(On Diff #8834)

Why is this block moved ?

araujo added a subscriber: araujo.Oct 1 2015, 2:05 PM
bapt added a subscriber: bapt.Oct 2 2015, 10:42 AM
wblock added a subscriber: wblock.Oct 3 2015, 8:39 PM
wblock added inline comments.
sbin/reboot/reboot.8
117 ↗(On Diff #8834)

This sentence is not very clear. Maybe

After changing vfs.root.mountfrom with
.Xr kenv 8 ,
.Nm
can be used to change the root filesystem while preserving kernel state.
sbin/reboot/reboot.c
114 ↗(On Diff #8834)

Simpler:

errx(1, "-r cannot be used with -d, -n, or -p");
182 ↗(On Diff #8834)

s/Nobody but/Only/

bapt added a comment.Oct 7 2015, 3:00 PM

Usage should be updated as well :)

bapt added a comment.Oct 7 2015, 3:52 PM

Would be nice to have a mechanism similat to init_chroot so that one could have a very minimal root and start it the following way:

init_path=/rescue/init
init_shell=/rescue/shell
init_script=/prepareroot.sh
init_reroot=ufs:/dev/md1

and in /prepareroot.sh one does prepare the root filesystem
exit 0 in prepareroot.sh
then init auto reroot

bapt requested changes to this revision.Oct 8 2015, 4:10 PM
bapt added a reviewer: bapt.
sbin/init/init.c
763 ↗(On Diff #8834)

This should be respecting init_path

805 ↗(On Diff #8834)

That should be respecting init_path

This revision now requires changes to proceed.Oct 8 2015, 4:10 PM
bapt added a comment.Oct 8 2015, 4:15 PM
sbin/init/init.c
763 ↗(On Diff #8834)

Actually here it should read from the path provided by: kern.proc.pathname

trasz added inline comments.Oct 16 2015, 10:29 AM
sbin/init/init.c
233 ↗(On Diff #8834)

Hm, all the other flags for reboot(8) are lowercase. Or did you mean to change "init -R" to "init -r"?

391 ↗(On Diff #8834)

What's dangerous in unmount(2)? As for interfering: on one hand you're right, on the other - if someone wants to interfere with the operation, and they are UID 0, then it's their fault.

1753 ↗(On Diff #8834)

To make it possible to call that whole block from reroot(), so that initial part of reroot process looks to userspace just like shutdown.

trasz added inline comments.Oct 16 2015, 10:40 AM
sbin/init/init.c
233 ↗(On Diff #8834)

Erm, what I meant was - all the other flags for init(8) are lowercase, so how does "-R" improve consistency?

trasz updated this revision to Diff 9452.Oct 16 2015, 4:11 PM
trasz edited edge metadata.

Fix stuff.

trasz marked 8 inline comments as done.Oct 16 2015, 4:13 PM
trasz added inline comments.
sbin/init/init.c
757 ↗(On Diff #8834)

I've added a call to kill(-1, SIGKILL) in reroot_phase_two().

trasz added inline comments.Oct 16 2015, 4:17 PM
sbin/init/init.c
763 ↗(On Diff #9452)

I'd prefer to obtain init path in a single unified way. And the problem is, the executable is exec(2)ed twice, from different paths, so the kern.proc.pathname would return "/sbin/init" only the first time, and "/dev/reroot/init" after that.

Perhaps we should have a sysctl, eg. kern.init_path? An alternative would be to use kenv(2) to obtain "init_path", and use the default init path if not found. But what if the kernel chose some other value, eg. in /rescue?

kib added inline comments.Oct 16 2015, 4:35 PM
sbin/init/init.c
233 ↗(On Diff #9452)

Several lines below, where you handle options for pid == 1 (i.e. reexec) case, you use '-R'. In fact I think that the re-exec flag should be changed to -r.

391 ↗(On Diff #9452)

System should be as tamper-resistant as it could be, but not prevent users from being ingenious in their system usage. I do not see any useful consequences in tricking init into unmounting something which was not mounted transiently by reroot. In other words, the check could protect a mistaken user from destroying the system.

1777 ↗(On Diff #9452)

Ok, let me explain my question explicitely. The runshutdown() function was clean, it executed /etc/rc.shutdown script and that was all. The death() state handler brings system to the single mode, it needs to execute several steps, like shut down the getty sessions, run the rc.shutdown, switch state to single user and so on.

If you want to get the functionality of shutting down getty sessions + rc.shutdown, you should move getty code into new helper, and call the new helper and runshutdown() from that place, instead of hacking runshutdown.

trasz updated this revision to Diff 9468.Oct 17 2015, 2:52 PM
trasz edited edge metadata.

Replace -R with -r, get runshutdown() back to the original form, and move
kill(-1, ...) a little earlier; no reason to defer it to phase two.

trasz marked 7 inline comments as done.Oct 17 2015, 3:00 PM
trasz added inline comments.
sbin/init/init.c
391 ↗(On Diff #9452)

I understand your point, but still - if a "mistaken user" manages to mount something on /dev/reroot/ during root mount, before even the init(8) gets started, then... well, it's their fault. There is no way to do it by accident.

1777 ↗(On Diff #9452)

Ah, you're right. Fixed.

emaste added inline comments.Oct 19 2015, 6:28 PM
sbin/init/Makefile
5 ↗(On Diff #9468)

Should be in alpha order?

sbin/reboot/reboot.c
113 ↗(On Diff #9468)

When is howto & RB_REROOT anything other than 0 or RB_REROOT?

trasz updated this revision to Diff 9513.Oct 19 2015, 9:50 PM
trasz marked an inline comment as done.
trasz edited edge metadata.

Fix reboot flags verification thinko.

trasz marked an inline comment as done.Oct 19 2015, 9:51 PM
trasz added inline comments.
sbin/init/Makefile
5 ↗(On Diff #9468)

I'm not sure, but other programs using it - eg mount(8) - do it in this order.

sbin/reboot/reboot.c
113 ↗(On Diff #9468)

Fixed, thanks.

bapt added inline comments.Oct 20 2015, 9:00 AM
sbin/init/init.c
764 ↗(On Diff #9513)

respecting init_path from loader.conf/kenv is good enough for my usecase, for sure the current mechanism does not work for my use case.

trasz updated this revision to Diff 9534.Oct 20 2015, 2:31 PM
trasz marked an inline comment as done.
trasz edited edge metadata.

Respect init_path.

trasz marked 7 inline comments as done.Oct 20 2015, 2:56 PM
trasz added inline comments.
sbin/init/init.c
764 ↗(On Diff #9513)

Ok, I made it use init_path. Please test it to see if the semantics fits your use case.

trasz updated this revision to Diff 9793.Oct 29 2015, 3:41 PM
trasz marked an inline comment as done.
trasz edited edge metadata.

Make "reboot -r" work correctly when called from /etc/rc. This could
have been prettier, but it's the least intrusive - as in, safest - way.

bapt added a comment.Nov 2 2015, 9:45 PM

works perfectly with regular init but not with /rescue/init

bapt accepted this revision.Nov 2 2015, 9:51 PM
bapt edited edge metadata.

I think this can go in like that and /rescue/init being debugged in a second step

This revision is now accepted and ready to land.Nov 2 2015, 9:51 PM
kib accepted this revision.Nov 6 2015, 8:48 PM
kib edited edge metadata.

init_path kenv should be mentioned.

This revision was automatically updated to reflect the committed changes.
trasz added a comment.Nov 8 2015, 5:34 PM

Committed; I'll think on how to document loader_path. Thanks!