From cdcf28f54f085520a96f4f9e480b8df324c5decb Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 13 Jan 2016 15:05:20 -0800 Subject: [PATCH] recovery: Fork a process for fuse when sideloading from SD card. For applying update from SD card, we used to use a thread to serve the file with fuse. Since accessing through fuse involves going from kernel to userspace to kernel, it may run into deadlock (e.g. for mmap_sem) when a page fault occurs. Switch to using a process instead. Bug: 23783099 Bug: 26313124 Change-Id: Iac0f55b1bdb078cadb520cfe1133e70fbb26eadd --- fuse_sdcard_provider.cpp | 74 ++++++---------------------------------- fuse_sdcard_provider.h | 3 +- recovery.cpp | 61 ++++++++++++++++++++++++++++++--- 3 files changed, 69 insertions(+), 69 deletions(-) diff --git a/fuse_sdcard_provider.cpp b/fuse_sdcard_provider.cpp index eb6454f1..df963127 100644 --- a/fuse_sdcard_provider.cpp +++ b/fuse_sdcard_provider.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -60,81 +59,30 @@ static void close_file(void* cookie) { close(fd->fd); } -struct token { - pthread_t th; - const char* path; - int result; -}; - -static void* run_sdcard_fuse(void* cookie) { - token* t = reinterpret_cast(cookie); - +bool start_sdcard_fuse(const char* path) { struct stat sb; - if (stat(t->path, &sb) < 0) { - fprintf(stderr, "failed to stat %s: %s\n", t->path, strerror(errno)); - t->result = -1; - return NULL; + if (stat(path, &sb) == -1) { + fprintf(stderr, "failed to stat %s: %s\n", path, strerror(errno)); + return false; } - struct file_data fd; - struct provider_vtab vtab; - - fd.fd = open(t->path, O_RDONLY); - if (fd.fd < 0) { - fprintf(stderr, "failed to open %s: %s\n", t->path, strerror(errno)); - t->result = -1; - return NULL; + file_data fd; + fd.fd = open(path, O_RDONLY); + if (fd.fd == -1) { + fprintf(stderr, "failed to open %s: %s\n", path, strerror(errno)); + return false; } fd.file_size = sb.st_size; fd.block_size = 65536; + provider_vtab vtab; vtab.read_block = read_block_file; vtab.close = close_file; - t->result = run_fuse_sideload(&vtab, &fd, fd.file_size, fd.block_size); - return NULL; -} - -// How long (in seconds) we wait for the fuse-provided package file to -// appear, before timing out. -#define SDCARD_INSTALL_TIMEOUT 10 - -void* start_sdcard_fuse(const char* path) { - token* t = new token; - - t->path = path; - pthread_create(&(t->th), NULL, run_sdcard_fuse, t); - - struct stat st; - int i; - for (i = 0; i < SDCARD_INSTALL_TIMEOUT; ++i) { - if (stat(FUSE_SIDELOAD_HOST_PATHNAME, &st) != 0) { - if (errno == ENOENT && i < SDCARD_INSTALL_TIMEOUT-1) { - sleep(1); - continue; - } else { - return NULL; - } - } - } - // The installation process expects to find the sdcard unmounted. // Unmount it with MNT_DETACH so that our open file continues to // work but new references see it as unmounted. umount2("/sdcard", MNT_DETACH); - return t; -} - -void finish_sdcard_fuse(void* cookie) { - if (cookie == NULL) return; - token* t = reinterpret_cast(cookie); - - // Calling stat() on this magic filename signals the fuse - // filesystem to shut down. - struct stat st; - stat(FUSE_SIDELOAD_HOST_EXIT_PATHNAME, &st); - - pthread_join(t->th, NULL); - delete t; + return run_fuse_sideload(&vtab, &fd, fd.file_size, fd.block_size) == 0; } diff --git a/fuse_sdcard_provider.h b/fuse_sdcard_provider.h index dc2982ca..bdc60f2b 100644 --- a/fuse_sdcard_provider.h +++ b/fuse_sdcard_provider.h @@ -17,7 +17,6 @@ #ifndef __FUSE_SDCARD_PROVIDER_H #define __FUSE_SDCARD_PROVIDER_H -void* start_sdcard_fuse(const char* path); -void finish_sdcard_fuse(void* token); +bool start_sdcard_fuse(const char* path); #endif diff --git a/recovery.cpp b/recovery.cpp index dace52f9..17e9eb66 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -833,6 +834,10 @@ static void choose_recovery_file(Device* device) { } } +// How long (in seconds) we wait for the fuse-provided package file to +// appear, before timing out. +#define SDCARD_INSTALL_TIMEOUT 10 + static int apply_from_sdcard(Device* device, bool* wipe_cache) { modified_flash = true; @@ -850,14 +855,62 @@ static int apply_from_sdcard(Device* device, bool* wipe_cache) { ui->Print("\n-- Install %s ...\n", path); set_sdcard_update_bootloader_message(); - void* token = start_sdcard_fuse(path); - int status = install_package(FUSE_SIDELOAD_HOST_PATHNAME, wipe_cache, + // We used to use fuse in a thread as opposed to a process. Since accessing + // through fuse involves going from kernel to userspace to kernel, it leads + // to deadlock when a page fault occurs. (Bug: 26313124) + pid_t child; + if ((child = fork()) == 0) { + bool status = start_sdcard_fuse(path); + + _exit(status ? EXIT_SUCCESS : EXIT_FAILURE); + } + + // FUSE_SIDELOAD_HOST_PATHNAME will start to exist once the fuse in child + // process is ready. + int result = INSTALL_ERROR; + int status; + bool waited = false; + for (int i = 0; i < SDCARD_INSTALL_TIMEOUT; ++i) { + if (waitpid(child, &status, WNOHANG) == -1) { + result = INSTALL_ERROR; + waited = true; + break; + } + + struct stat sb; + if (stat(FUSE_SIDELOAD_HOST_PATHNAME, &sb) == -1) { + if (errno == ENOENT && i < SDCARD_INSTALL_TIMEOUT-1) { + sleep(1); + continue; + } else { + LOGE("Timed out waiting for the fuse-provided package.\n"); + result = INSTALL_ERROR; + kill(child, SIGKILL); + break; + } + } + + result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, wipe_cache, TEMPORARY_INSTALL_FILE, false); + break; + } + + if (!waited) { + // Calling stat() on this magic filename signals the fuse + // filesystem to shut down. + struct stat sb; + stat(FUSE_SIDELOAD_HOST_EXIT_PATHNAME, &sb); + + waitpid(child, &status, 0); + } + + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { + LOGE("Error exit from the fuse process: %d\n", WEXITSTATUS(status)); + } - finish_sdcard_fuse(token); ensure_path_unmounted(SDCARD_ROOT); - return status; + return result; } // Return REBOOT, SHUTDOWN, or REBOOT_BOOTLOADER. Returning NO_ACTION