diff options
| -rw-r--r-- | install.cpp | 110 | ||||
| -rw-r--r-- | minui/events.cpp | 2 | ||||
| -rw-r--r-- | minui/graphics_adf.cpp | 2 | ||||
| -rw-r--r-- | minui/graphics_drm.cpp | 2 | ||||
| -rw-r--r-- | minui/graphics_fbdev.cpp | 2 | ||||
| -rw-r--r-- | recovery_main.cpp | 27 |
6 files changed, 65 insertions, 80 deletions
diff --git a/install.cpp b/install.cpp index 9d8943f7d..a7868fa1d 100644 --- a/install.cpp +++ b/install.cpp @@ -292,6 +292,11 @@ int SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, i return INSTALL_ERROR; } + // When executing the update binary contained in the package, the arguments passed are: + // - the version number for this interface + // - an FD to which the program can write in order to update the progress bar. + // - the name of the package zip file. + // - an optional argument "retry" if this update is a retry of a failed update attempt. *cmd = { binary_path, std::to_string(kRecoveryApiVersion), @@ -334,75 +339,56 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b ReadSourceTargetBuild(metadata, log_buffer); - int pipefd[2]; - pipe(pipefd); - std::vector<std::string> args; - if (int update_status = - is_ab ? SetUpAbUpdateCommands(package, zip, pipefd[1], &args) - : SetUpNonAbUpdateCommands(package, zip, retry_count, pipefd[1], &args); - update_status != 0) { - close(pipefd[0]); - close(pipefd[1]); - log_buffer->push_back(android::base::StringPrintf("error: %d", kUpdateBinaryCommandFailure)); - return update_status; + // The updater in child process writes to the pipe to communicate with recovery. + android::base::unique_fd pipe_read, pipe_write; + if (!android::base::Pipe(&pipe_read, &pipe_write)) { + PLOG(ERROR) << "Failed to create pipe for updater-recovery communication"; + return INSTALL_CORRUPT; } - // When executing the update binary contained in the package, the - // arguments passed are: - // - // - the version number for this interface - // - // - an FD to which the program can write in order to update the - // progress bar. The program can write single-line commands: + // The updater-recovery communication protocol. // - // progress <frac> <secs> - // fill up the next <frac> part of of the progress bar - // over <secs> seconds. If <secs> is zero, use - // set_progress commands to manually control the - // progress of this segment of the bar. + // progress <frac> <secs> + // fill up the next <frac> part of of the progress bar over <secs> seconds. If <secs> is + // zero, use `set_progress` commands to manually control the progress of this segment of the + // bar. // - // set_progress <frac> - // <frac> should be between 0.0 and 1.0; sets the - // progress bar within the segment defined by the most - // recent progress command. + // set_progress <frac> + // <frac> should be between 0.0 and 1.0; sets the progress bar within the segment defined by + // the most recent progress command. // - // ui_print <string> - // display <string> on the screen. + // ui_print <string> + // display <string> on the screen. // - // wipe_cache - // a wipe of cache will be performed following a successful - // installation. + // wipe_cache + // a wipe of cache will be performed following a successful installation. // - // clear_display - // turn off the text display. + // clear_display + // turn off the text display. // - // enable_reboot - // packages can explicitly request that they want the user - // to be able to reboot during installation (useful for - // debugging packages that don't exit). + // enable_reboot + // packages can explicitly request that they want the user to be able to reboot during + // installation (useful for debugging packages that don't exit). // - // retry_update - // updater encounters some issue during the update. It requests - // a reboot to retry the same package automatically. + // retry_update + // updater encounters some issue during the update. It requests a reboot to retry the same + // package automatically. // - // log <string> - // updater requests logging the string (e.g. cause of the - // failure). - // - // - the name of the package zip file. - // - // - an optional argument "retry" if this update is a retry of a failed - // update attempt. + // log <string> + // updater requests logging the string (e.g. cause of the failure). // - // Convert the std::string vector to a NULL-terminated char* vector suitable for execv. - auto chr_args = StringVectorToNullTerminatedArray(args); + std::vector<std::string> args; + if (int update_status = + is_ab ? SetUpAbUpdateCommands(package, zip, pipe_write.get(), &args) + : SetUpNonAbUpdateCommands(package, zip, retry_count, pipe_write.get(), &args); + update_status != 0) { + log_buffer->push_back(android::base::StringPrintf("error: %d", kUpdateBinaryCommandFailure)); + return update_status; + } pid_t pid = fork(); - if (pid == -1) { - close(pipefd[0]); - close(pipefd[1]); PLOG(ERROR) << "Failed to fork update binary"; log_buffer->push_back(android::base::StringPrintf("error: %d", kForkUpdateBinaryFailure)); return INSTALL_ERROR; @@ -410,16 +396,18 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b if (pid == 0) { umask(022); - close(pipefd[0]); + pipe_read.reset(); + + // Convert the std::string vector to a NULL-terminated char* vector suitable for execv. + auto chr_args = StringVectorToNullTerminatedArray(args); execv(chr_args[0], chr_args.data()); - // Bug: 34769056 - // We shouldn't use LOG/PLOG in the forked process, since they may cause - // the child process to hang. This deadlock results from an improperly - // copied mutex in the ui functions. + // We shouldn't use LOG/PLOG in the forked process, since they may cause the child process to + // hang. This deadlock results from an improperly copied mutex in the ui functions. + // (Bug: 34769056) fprintf(stdout, "E:Can't run %s (%s)\n", chr_args[0], strerror(errno)); _exit(EXIT_FAILURE); } - close(pipefd[1]); + pipe_write.reset(); std::atomic<bool> logger_finished(false); std::thread temperature_logger(log_max_temperature, max_temperature, std::ref(logger_finished)); @@ -428,7 +416,7 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b bool retry_update = false; char buffer[1024]; - FILE* from_child = fdopen(pipefd[0], "r"); + FILE* from_child = android::base::Fdopen(std::move(pipe_read), "r"); while (fgets(buffer, sizeof(buffer), from_child) != nullptr) { std::string line(buffer); size_t space = line.find_first_of(" \n"); diff --git a/minui/events.cpp b/minui/events.cpp index d94e97723..30f8d50a2 100644 --- a/minui/events.cpp +++ b/minui/events.cpp @@ -66,7 +66,7 @@ int ev_init(ev_callback input_cb, bool allow_touch_inputs) { dirent* de; while ((de = readdir(dir))) { if (strncmp(de->d_name, "event", 5)) continue; - int fd = openat(dirfd(dir), de->d_name, O_RDONLY); + int fd = openat(dirfd(dir), de->d_name, O_RDONLY | O_CLOEXEC); if (fd == -1) continue; // Use unsigned long to match ioctl's parameter type. diff --git a/minui/graphics_adf.cpp b/minui/graphics_adf.cpp index 9eea497d6..10cd60709 100644 --- a/minui/graphics_adf.cpp +++ b/minui/graphics_adf.cpp @@ -101,7 +101,7 @@ int MinuiBackendAdf::DeviceInit(adf_device* dev) { err = adf_device_attach(dev, eng_id, intf_id); if (err < 0 && err != -EALREADY) return err; - intf_fd = adf_interface_open(dev, intf_id, O_RDWR); + intf_fd = adf_interface_open(dev, intf_id, O_RDWR | O_CLOEXEC); if (intf_fd < 0) return intf_fd; err = InterfaceInit(); diff --git a/minui/graphics_drm.cpp b/minui/graphics_drm.cpp index 765e2625a..7b2eed15d 100644 --- a/minui/graphics_drm.cpp +++ b/minui/graphics_drm.cpp @@ -285,7 +285,7 @@ GRSurface* MinuiBackendDrm::Init() { /* Consider DRM devices in order. */ for (int i = 0; i < DRM_MAX_MINOR; i++) { auto dev_name = android::base::StringPrintf(DRM_DEV_NAME, DRM_DIR_NAME, i); - android::base::unique_fd fd(open(dev_name.c_str(), O_RDWR)); + android::base::unique_fd fd(open(dev_name.c_str(), O_RDWR | O_CLOEXEC)); if (fd == -1) continue; /* We need dumb buffers. */ diff --git a/minui/graphics_fbdev.cpp b/minui/graphics_fbdev.cpp index 8d9c9741d..2584017d6 100644 --- a/minui/graphics_fbdev.cpp +++ b/minui/graphics_fbdev.cpp @@ -56,7 +56,7 @@ void MinuiBackendFbdev::SetDisplayedFramebuffer(size_t n) { } GRSurface* MinuiBackendFbdev::Init() { - android::base::unique_fd fd(open("/dev/graphics/fb0", O_RDWR)); + android::base::unique_fd fd(open("/dev/graphics/fb0", O_RDWR | O_CLOEXEC)); if (fd == -1) { perror("cannot open fb0"); return nullptr; diff --git a/recovery_main.cpp b/recovery_main.cpp index 7fb46163e..935d69815 100644 --- a/recovery_main.cpp +++ b/recovery_main.cpp @@ -217,9 +217,10 @@ static void ListenRecoverySocket(RecoveryUI* ui, std::atomic<Device::BuiltinActi } static void redirect_stdio(const char* filename) { - int pipefd[2]; - if (pipe(pipefd) == -1) { - PLOG(ERROR) << "pipe failed"; + android::base::unique_fd pipe_read, pipe_write; + // Create a pipe that allows parent process sending logs over. + if (!android::base::Pipe(&pipe_read, &pipe_write)) { + PLOG(ERROR) << "Failed to create pipe for redirecting stdio"; // Fall back to traditional logging mode without timestamps. If these fail, there's not really // anywhere to complain... @@ -233,7 +234,7 @@ static void redirect_stdio(const char* filename) { pid_t pid = fork(); if (pid == -1) { - PLOG(ERROR) << "fork failed"; + PLOG(ERROR) << "Failed to fork for redirecting stdio"; // Fall back to traditional logging mode without timestamps. If these fail, there's not really // anywhere to complain... @@ -246,8 +247,8 @@ static void redirect_stdio(const char* filename) { } if (pid == 0) { - /// Close the unused write end. - close(pipefd[1]); + // Child process reads the incoming logs and doesn't write to the pipe. + pipe_write.reset(); auto start = std::chrono::steady_clock::now(); @@ -255,15 +256,13 @@ static void redirect_stdio(const char* filename) { FILE* log_fp = fopen(filename, "ae"); if (log_fp == nullptr) { PLOG(ERROR) << "fopen \"" << filename << "\" failed"; - close(pipefd[0]); _exit(EXIT_FAILURE); } - FILE* pipe_fp = fdopen(pipefd[0], "r"); + FILE* pipe_fp = android::base::Fdopen(std::move(pipe_read), "r"); if (pipe_fp == nullptr) { PLOG(ERROR) << "fdopen failed"; check_and_fclose(log_fp, filename); - close(pipefd[0]); _exit(EXIT_FAILURE); } @@ -283,25 +282,23 @@ static void redirect_stdio(const char* filename) { PLOG(ERROR) << "getline failed"; + fclose(pipe_fp); free(line); check_and_fclose(log_fp, filename); - close(pipefd[0]); _exit(EXIT_FAILURE); } else { // Redirect stdout/stderr to the logger process. Close the unused read end. - close(pipefd[0]); + pipe_read.reset(); setbuf(stdout, nullptr); setbuf(stderr, nullptr); - if (dup2(pipefd[1], STDOUT_FILENO) == -1) { + if (dup2(pipe_write.get(), STDOUT_FILENO) == -1) { PLOG(ERROR) << "dup2 stdout failed"; } - if (dup2(pipefd[1], STDERR_FILENO) == -1) { + if (dup2(pipe_write.get(), STDERR_FILENO) == -1) { PLOG(ERROR) << "dup2 stderr failed"; } - - close(pipefd[1]); } } |