From 1ca83249a1748c2458664afa76962e2bb9cdb553 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 25 Aug 2017 15:10:48 -0700 Subject: [PATCH] init: fix signal handling and LOG(FATAL) in child processes Child processes inherit the signal handlers and the 'Aborter' for logging from their parent process. In the case of init, fork()'ed processes, will attempt to reboot the system if they receive a fatal signal or if they call LOG(FATAL). This is not the correct behavior; these processes should terminate due to the provided signal like other processes on the system. This is particularly important as there are multiple LOG(FATAL) calls in service.cpp for failures after fork() but before execv() when a service is started. Note, that pthread_atfork() is not a viable solution since clone() is used in some cases instead of fork() and atfork handlers are not called with clone(). Test: LOG(FATAL) from a child process of init and see that it terminates due to a signal correctly Test: LOG(FATAL) from init proper and see that it reboots to the bootloader Change-Id: I875ebd7a5f6b3f5e3e2c028af3306917c4409db3 --- init/init.cpp | 12 +++++++++--- init/log.cpp | 14 +++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/init/init.cpp b/init/init.cpp index e1bd3a2cb..6557710fa 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -354,7 +354,7 @@ static void set_usb_controller() { } } -static void install_reboot_signal_handlers() { +static void InstallRebootSignalHandlers() { // Instead of panic'ing the kernel as is the default behavior when init crashes, // we prefer to reboot to bootloader on development builds, as this will prevent // boot looping bad configurations and allow both developers and test farms to easily @@ -362,7 +362,13 @@ static void install_reboot_signal_handlers() { struct sigaction action; memset(&action, 0, sizeof(action)); sigfillset(&action.sa_mask); - action.sa_handler = [](int) { + action.sa_handler = [](int signal) { + // These signal handlers are also caught for processes forked from init, however we do not + // want them to trigger reboot, so we directly call _exit() for children processes here. + if (getpid() != 1) { + _exit(signal); + } + // Calling DoReboot() or LOG(FATAL) is not a good option as this is a signal handler. // RebootSystem uses syscall() which isn't actually async-signal-safe, but our only option // and probably good enough given this is already an error case and only enabled for @@ -392,7 +398,7 @@ int main(int argc, char** argv) { } if (REBOOT_BOOTLOADER_ON_PANIC) { - install_reboot_signal_handlers(); + InstallRebootSignalHandlers(); } bool is_first_stage = (getenv("INIT_SECOND_STAGE") == nullptr); diff --git a/init/log.cpp b/init/log.cpp index 391bc1f86..6198fc25f 100644 --- a/init/log.cpp +++ b/init/log.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -29,7 +30,14 @@ namespace android { namespace init { -static void RebootAborter(const char* abort_message) { +static void InitAborter(const char* abort_message) { + // When init forks, it continues to use this aborter for LOG(FATAL), but we want children to + // simply abort instead of trying to reboot the system. + if (getpid() != 1) { + android::base::DefaultAborter(abort_message); + return; + } + // DoReboot() does a lot to try to shutdown the system cleanly. If something happens to call // LOG(FATAL) in the shutdown path, we want to catch this and immediately use the syscall to // reboot instead of recursing here. @@ -49,7 +57,7 @@ void InitKernelLogging(char* argv[]) { int fd = open("/sys/fs/selinux/null", O_RDWR); if (fd == -1) { int saved_errno = errno; - android::base::InitLogging(argv, &android::base::KernelLogger, RebootAborter); + android::base::InitLogging(argv, &android::base::KernelLogger, InitAborter); errno = saved_errno; PLOG(FATAL) << "Couldn't open /sys/fs/selinux/null"; } @@ -58,7 +66,7 @@ void InitKernelLogging(char* argv[]) { dup2(fd, 2); if (fd > 2) close(fd); - android::base::InitLogging(argv, &android::base::KernelLogger, RebootAborter); + android::base::InitLogging(argv, &android::base::KernelLogger, InitAborter); } int selinux_klog_callback(int type, const char *fmt, ...) {