From 34e70410ee70a0e95ab8318636000e8e28554fe1 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Thu, 16 Mar 2017 18:08:56 -0700 Subject: [PATCH] init: cleanup is_first_stage conditionals A recent change to the is_first_stage conditionals created a unneeded else { } block as both the code in the else { } block and any code that runs after it are both in the second stage of init. A first step to clean this up is to remove this else block. Secondly, given the above confusion, it makes sense to simplify the two if (is_first_stage) conditions into one, which only now requires duplicating one line to initialize logging and the actual "init first/second stage started!" logs. Lastly, there are a few commands ran at the beginning of both init stages that do not need to be, * boot_clock::time_point start_time = boot_clock::now(); This is only used in the first stage so keep it there * umask(0); umasks are preserved across execve() so it only needs to be set in the first stage * chmod("/proc/cmdline", 0440); This needs to be moved until after /proc is mounted in the first stage, but otherwise only needs to be done once Test: Boot bullhead, check umask, check cmdline permissions, check boot time property Change-Id: Idb7df1d4330960ce282d9609f5c62281ee2638b9 --- init/init.cpp | 99 ++++++++++++++++++++++++++------------------------- 1 file changed, 51 insertions(+), 48 deletions(-) diff --git a/init/init.cpp b/init/init.cpp index 4e31865f7..d09568567 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -1108,27 +1108,26 @@ int main(int argc, char** argv) { return watchdogd_main(argc, argv); } - boot_clock::time_point start_time = boot_clock::now(); - - // Clear the umask. - umask(0); - add_environment("PATH", _PATH_DEFPATH); bool is_first_stage = (getenv("INIT_SECOND_STAGE") == nullptr); - // Don't expose the raw commandline to unprivileged processes. - chmod("/proc/cmdline", 0440); - - // Get the basic filesystem setup we need put together in the initramdisk - // on / and then we'll let the rc file figure out the rest. if (is_first_stage) { + boot_clock::time_point start_time = boot_clock::now(); + + // Clear the umask. + umask(0); + + // Get the basic filesystem setup we need put together in the initramdisk + // on / and then we'll let the rc file figure out the rest. mount("tmpfs", "/dev", "tmpfs", MS_NOSUID, "mode=0755"); mkdir("/dev/pts", 0755); mkdir("/dev/socket", 0755); mount("devpts", "/dev/pts", "devpts", 0, NULL); #define MAKE_STR(x) __STRING(x) mount("proc", "/proc", "proc", 0, "hidepid=2,gid=" MAKE_STR(AID_READPROC)); + // Don't expose the raw commandline to unprivileged processes. + chmod("/proc/cmdline", 0440); gid_t groups[] = { AID_READPROC }; setgroups(arraysize(groups), groups); mount("sysfs", "/sys", "sysfs", 0, NULL); @@ -1136,15 +1135,13 @@ int main(int argc, char** argv) { mknod("/dev/kmsg", S_IFCHR | 0600, makedev(1, 11)); mknod("/dev/random", S_IFCHR | 0666, makedev(1, 8)); mknod("/dev/urandom", S_IFCHR | 0666, makedev(1, 9)); - } - // Now that tmpfs is mounted on /dev and we have /dev/kmsg, we can actually - // talk to the outside world... - InitKernelLogging(argv); + // Now that tmpfs is mounted on /dev and we have /dev/kmsg, we can actually + // talk to the outside world... + InitKernelLogging(argv); - LOG(INFO) << "init " << (is_first_stage ? "first" : "second") << " stage started!"; + LOG(INFO) << "init first stage started!"; - if (is_first_stage) { if (!early_mount()) { LOG(ERROR) << "Failed to mount required partitions early ..."; panic(); @@ -1168,41 +1165,47 @@ int main(int argc, char** argv) { char* path = argv[0]; char* args[] = { path, nullptr }; - if (execv(path, args) == -1) { - PLOG(ERROR) << "execv(\"" << path << "\") failed"; - security_failure(); - } - } else { - // Indicate that booting is in progress to background fw loaders, etc. - close(open("/dev/.booting", O_WRONLY | O_CREAT | O_CLOEXEC, 0000)); + execv(path, args); - property_init(); - - // If arguments are passed both on the command line and in DT, - // properties set in DT always have priority over the command-line ones. - process_kernel_dt(); - process_kernel_cmdline(); - - // Propagate the kernel variables to internal variables - // used by init as well as the current required properties. - export_kernel_boot_props(); - - // Make the time that init started available for bootstat to log. - property_set("ro.boottime.init", getenv("INIT_STARTED_AT")); - property_set("ro.boottime.init.selinux", getenv("INIT_SELINUX_TOOK")); - - // Set libavb version for Framework-only OTA match in Treble build. - property_set("ro.boot.init.avb_version", std::to_string(AVB_MAJOR_VERSION).c_str()); - - // Clean up our environment. - unsetenv("INIT_SECOND_STAGE"); - unsetenv("INIT_STARTED_AT"); - unsetenv("INIT_SELINUX_TOOK"); - - // Now set up SELinux for second stage. - selinux_initialize(false); + // execv() only returns if an error happened, in which case we + // panic and never fall through this conditional. + PLOG(ERROR) << "execv(\"" << path << "\") failed"; + security_failure(); } + // At this point we're in the second stage of init. + InitKernelLogging(argv); + LOG(INFO) << "init second stage started!"; + + // Indicate that booting is in progress to background fw loaders, etc. + close(open("/dev/.booting", O_WRONLY | O_CREAT | O_CLOEXEC, 0000)); + + property_init(); + + // If arguments are passed both on the command line and in DT, + // properties set in DT always have priority over the command-line ones. + process_kernel_dt(); + process_kernel_cmdline(); + + // Propagate the kernel variables to internal variables + // used by init as well as the current required properties. + export_kernel_boot_props(); + + // Make the time that init started available for bootstat to log. + property_set("ro.boottime.init", getenv("INIT_STARTED_AT")); + property_set("ro.boottime.init.selinux", getenv("INIT_SELINUX_TOOK")); + + // Set libavb version for Framework-only OTA match in Treble build. + property_set("ro.boot.init.avb_version", std::to_string(AVB_MAJOR_VERSION).c_str()); + + // Clean up our environment. + unsetenv("INIT_SECOND_STAGE"); + unsetenv("INIT_STARTED_AT"); + unsetenv("INIT_SELINUX_TOOK"); + + // Now set up SELinux for second stage. + selinux_initialize(false); + // These directories were necessarily created before initial policy load // and therefore need their security context restored to the proper value. // This must happen before /dev is populated by ueventd.