From b1d92c65082181d28f7396727f393855438b9ad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 12 Feb 2024 15:39:52 +0000 Subject: [PATCH 1/2] first_stage_console: Fix waitpid() as SA_NOCLDWAIT From wait(2): POSIX.1-2001 specifies that if [...] the SA_NOCLDWAIT flag is set for SIGCHLD, then children that terminate do not become zombies and a call to [...] waitpid() will block until all children have terminated, and then fail with errno set to ECHILD. As we call sigaction(SIGCHLD, { SIG_DFL, SA_NOCLDWAIT }), running pid_t w = waitpid(pid, &status, 0); LOG(INFO) << "..." << status << " " << w << " " << errno; shows that the calls consistently return (status=0, w=-1, errno=ECHILD). Therefore, clarify the parent code by prefering wait(2) over waitpid(2), as SA_NOCLDWAIT makes the kernel ignore the passed PID, and stop logging the irrelevant status, to avoid confusion when the logs say the exit status was 0 but the child actually returned an error. Test: run first_stage_console Change-Id: I54df888e38b947e206e374ad28ebb044c70c6640 --- init/first_stage_console.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/init/first_stage_console.cpp b/init/first_stage_console.cpp index 007676483..cb8caabd2 100644 --- a/init/first_stage_console.cpp +++ b/init/first_stage_console.cpp @@ -69,9 +69,8 @@ static void RunScript() { LOG(INFO) << "Attempting to run /first_stage.sh..."; pid_t pid = fork(); if (pid != 0) { - int status; - waitpid(pid, &status, 0); - LOG(INFO) << "/first_stage.sh exited with status " << status; + wait(NULL); + LOG(INFO) << "/first_stage.sh exited"; return; } const char* path = "/system/bin/sh"; @@ -94,9 +93,8 @@ void StartConsole(const std::string& cmdline) { sigaction(SIGCHLD, &chld_act, nullptr); pid_t pid = fork(); if (pid != 0) { - int status; - waitpid(pid, &status, 0); - LOG(ERROR) << "console shell exited with status " << status; + wait(NULL); + LOG(ERROR) << "console shell exited"; return; } From b6b2afb6b3679275fc2876f0832996be35b75139 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 12 Feb 2024 17:13:54 +0000 Subject: [PATCH 2/2] first_stage_console: Refactor RunScript() Introduce SpawnImage() as a reusable single-argument wrapper around posix_spawn(), to avoid having to manually manage the child process. Note that Bionic currently doesn't return the errno from the child's exec() call to the caller in the parent process, which may temporarily hide errors such as ENOENT in first_stage_console until Bionic improves. Also, this introduces a subtle change in behavior as the first_stage.sh script is now passed directly to the loader, which will only properly invoke the Shell if the file contains the right shebang. Inline the call to RunScript() to hopefully make it simpler for readers to track the lifetime of the various processes on different code paths. Test: run first_stage_init Change-Id: Ifaab2be032b2080a039209295d0b5a3759764ea7 --- init/first_stage_console.cpp | 41 ++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/init/first_stage_console.cpp b/init/first_stage_console.cpp index cb8caabd2..f6f93299b 100644 --- a/init/first_stage_console.cpp +++ b/init/first_stage_console.cpp @@ -16,6 +16,7 @@ #include "first_stage_console.h" +#include #include #include #include @@ -65,19 +66,20 @@ static bool SetupConsole() { return true; } -static void RunScript() { - LOG(INFO) << "Attempting to run /first_stage.sh..."; - pid_t pid = fork(); - if (pid != 0) { - wait(NULL); - LOG(INFO) << "/first_stage.sh exited"; - return; - } - const char* path = "/system/bin/sh"; - const char* args[] = {path, "/first_stage.sh", nullptr}; - int rv = execv(path, const_cast(args)); - LOG(ERROR) << "unable to execv /first_stage.sh, returned " << rv << " errno " << errno; - _exit(127); +static pid_t SpawnImage(const char* file) { + const char* argv[] = {file, NULL}; + const char* envp[] = {NULL}; + + char* const* argvp = const_cast(argv); + char* const* envpp = const_cast(envp); + + pid_t pid; + errno = posix_spawn(&pid, argv[0], NULL, NULL, argvp, envpp); + if (!errno) return pid; + + PLOG(ERROR) << "Failed to spawn '" << file << "'"; + + return (pid_t)0; } namespace android { @@ -99,12 +101,15 @@ void StartConsole(const std::string& cmdline) { } if (console) console = SetupConsole(); - RunScript(); + + LOG(INFO) << "Attempting to run /first_stage.sh..."; + if (SpawnImage("/first_stage.sh")) { + wait(NULL); + LOG(INFO) << "/first_stage.sh exited"; + } + if (console) { - const char* path = "/system/bin/sh"; - const char* args[] = {path, nullptr}; - int rv = execv(path, const_cast(args)); - LOG(ERROR) << "unable to execv, returned " << rv << " errno " << errno; + if (SpawnImage("/system/bin/sh")) wait(NULL); } _exit(127); }