init: build dynamically
Needs ReviewPublic
Actions

Authored by kib on Mon, Apr 20, 6:08 PM.

Details

Reviewers

jilles
brooks
emaste
zlei

Summary

This makes it easier to downgrade kernel when it stops providing some
syscall required by libc.  In this case, it is enough to downgrade libc
as well, our crt1 delegates all non-trivial work to
libc::__libc_start1().  With static init, the /sbin/init should be
downgraded as well, which might be not easy.

This does not mean that we support forward compatibility.

Also, as a safety and convenience aid:

bin/sh: make it possible to use as interactive init

If the /sbin/init binary is broken somehow, the way out is to set the
loader environment variable init_path to something else.  The most
natural choice would be either /bin/sh or /rescue/sh.  Unfortunately,
this does not work because the init process starts withoud stdin/out
descriptors.

Make it nicer to users by teaching /bin/sh startup code to open standard
descriptors on /dev/console if the shell is run as init.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.Mon, Apr 20, 6:08 PM

Herald added a subscriber: imp. · View Herald TranscriptMon, Apr 20, 6:08 PM

kib requested review of this revision.Mon, Apr 20, 6:08 PM

zlei added a subscriber: zlei.Fri, Apr 24, 8:31 AM

zlei added inline comments.

sbin/init/Makefile
13	The init(8) is so important that it should be as strong as possible. Will this introduce more chances to break it ? % ls -llh /sbin/init -r-xr-xr-x 1 root wheel 1.2M Mar 30 17:43 /sbin/init It occupies a lot more disk spaces compared to other dynamically utilities. Is that the main reason to make it dynamically ?

kib added inline comments.Fri, Apr 24, 7:54 PM

sbin/init/Makefile
13	I am not sure what it mean for init to be strong. An issue with the statically linked binaries comes when somebody has to downgrade kernel, and older kernel does not have some syscall that is needed for libc startup. In this case user has to downgrade libc as well anyway, but with static init he must also downgrade init, which is currently problematic. The two changes from this review easier the situation: sh change makes it possible to use /bin/sh without init, by specifying init_path in loader to /bin/sh init dyn linking allows only to downgrade libc and get init working.

zlei added inline comments.Sat, Apr 25, 6:44 PM

sbin/init/Makefile
13	I am not sure what it mean for init to be strong. Ahh, I meant robust. For example, init(8) depends on libc, libcompiler_rt, libcrypt and libutil. If any of those libraries breaks for some reason, init(8) might not work. There is `/rescue/init` which is statically linked. I think that should be sufficient if things go wrong. An issue with the statically linked binaries comes when somebody has to downgrade kernel, and older kernel does not have some syscall that is needed for libc startup. In this case user has to downgrade libc as well anyway, but with static init he must also downgrade init, which is currently problematic. I see the problem. I did not thought about downgrade. I think putting an explanation in the commit message will make less confusion why this change is made. The two changes from this review easier the situation: sh change makes it possible to use /bin/sh without init, by specifying init_path in loader to /bin/sh init dyn linking allows only to downgrade libc and get init working.

zlei added inline comments.Sat, Apr 25, 6:53 PM

sbin/init/Makefile
13	I am not sure what it mean for init to be strong. Ahh, I meant robust. For example, init(8) depends on libc, libcompiler_rt, libcrypt and libutil. If any of those libraries breaks for some reason, init(8) might not work. I was wrong about libcompiler_rt. It is a compile time dependency for init(8). Just ignore that.

kib edited the summary of this revision. (Show Details)Sat, Apr 25, 11:03 PM

kib marked 3 inline comments as done.

Tested with dynamically linked init(8). The change to init(8) looks good to me.

This revision is now accepted and ready to land.Sun, Apr 26, 5:49 AM

Tested with init_path=/bin/init set in the boot loader, the change to sh(1) looks good to me.

bin/sh/main.c
130	`O_RDONLY` should be sufficient to emulate stdin.
132
134	It is a bit tricky here to open three file description at the first glance. Indeed that is to create 0, 1 and 2 which corresponds to STDIN_FILENO, STDOUT_FILENO and STDERR_FILENO. I'd rather to leave a note here, to mention this tricky part.
134

kib marked 4 inline comments as done.Sun, Apr 26, 6:52 AM

kib added inline comments.

bin/sh/main.c
130	I am not sure, and it is not how init and login set up stdin/out/err. In fact they open fd 0 rw, and then dup it into 1 and 2. So this should stay as is.
134	What specifically is tricky? I added some comment about reasons for the open.

Add comment about opens.

This revision now requires review to proceed.Sun, Apr 26, 6:52 AM

I'm unsure about this novel dance with F_GETFL. I've not seen this in any of the other init programs I've looked at (admittedly, they were Linux or older BSDs), so I'd be curious to understand the reasoning behind that vs doing the simpler code I left in the specific comment.

bin/sh/main.c
130	Any reason you didn't just dup() here? If 0 is bad 1 and 2 will be absent kernel bugs so I don't understand the point of the dance. The init I wrote for LinuxBoot just does this: close(0) close(1) close(2) fd = open("/dev/console", O_RDWR \| O_NOCTTY, 0) dup(fd) dup(fd) Our init, for an interactive shell, basically does the above sequence (it's scattered in our init because we always close 0, 1, 2 and only try the open dance if we're entering single user). But our init also tries to mount devfs if it isn't mounted (surely this is a leftover from when we were first bringing devfs into the system?). Our init also does revoke("/dev/console"), but I think that's to start fresh for the variety of paths through the code to open_console(). So why doesn't this code close them followed by either dup or dup2? I'd think that as pid1, we'd want to be pessimistic about what we were presented with and not trust it rather than conditionally opening /dev/console only if the fd wasn't already opened. The kernel isn't going to open any files, so what's the thinking of trusting what's there?

In D56536#1297370, @imp wrote:

I'm unsure about this novel dance with F_GETFL. I've not seen this in any of the other init programs I've looked at (admittedly, they were Linux or older BSDs), so I'd be curious to understand the reasoning behind that vs doing the simpler code I left in the specific comment.

My dance with detecting if the stdin/out are opened is more flexible I believe. There is no question of trust, since there is no security boundaries crossed. OTOH, the check for std* validity before opening the console allows me to re-exec sh with io redirected somewhere.

I do not stand strongly with this code, if the agreement is that we must always reopen console, I am fine to change that. For now, I will switch to dup2().

Dup console instead of reopening.
Add documentation for init_path into init.8.
Add RECOVERING to init.8 documenting the sh(1) hack.

Herald added subscribers: ziaee, olce. · View Herald TranscriptSun, Apr 26, 10:30 AM

jilles added inline comments.Sun, Apr 26, 12:01 PM

bin/sh/main.c
134–139	Perhaps add `(void)setsid();` and `(void)tcsetsid(STDIN_FILENO, 1);` too so job control works?
sbin/init/init.8
415–420	By the way, does this still work with pkgbase?
422–433	One additional thing is that the administrator will need to end the session using `reboot -q` since a normal exit will cause a kernel panic.
429–430	An article is needed here.