diff options
41 files changed, 1057 insertions, 530 deletions
diff --git a/cmds/atrace/atrace.cpp b/cmds/atrace/atrace.cpp index 645944340e..7aee795fc4 100644 --- a/cmds/atrace/atrace.cpp +++ b/cmds/atrace/atrace.cpp @@ -245,7 +245,7 @@ static const TracingCategory k_categories[] = { { OPT, "events/ion/ion_stat/enable" }, { OPT, "events/gpu_mem/gpu_mem_total/enable" }, } }, - { "thermal", "Thermal event", 0, { + { "thermal", "Thermal event", ATRACE_TAG_THERMAL, { { REQ, "events/thermal/thermal_temperature/enable" }, { OPT, "events/thermal/cdev_update/enable" }, } }, diff --git a/cmds/dumpstate/DumpPool.cpp b/cmds/dumpstate/DumpPool.cpp index c2c8a72f52..4d3a67bcd3 100644 --- a/cmds/dumpstate/DumpPool.cpp +++ b/cmds/dumpstate/DumpPool.cpp @@ -33,6 +33,20 @@ namespace dumpstate { const std::string DumpPool::PREFIX_TMPFILE_NAME = "dump-tmp."; + +void WaitForTask(std::future<std::string> future, const std::string& title, int out_fd) { + DurationReporter duration_reporter("Wait for " + title, true); + + std::string result = future.get(); + if (result.empty()) { + return; + } + DumpFileToFd(out_fd, title, result); + if (unlink(result.c_str())) { + MYLOGE("Failed to unlink (%s): %s\n", result.c_str(), strerror(errno)); + } +} + DumpPool::DumpPool(const std::string& tmp_root) : tmp_root_(tmp_root), shutdown_(false), log_duration_(true) { assert(!tmp_root.empty()); @@ -40,31 +54,10 @@ DumpPool::DumpPool(const std::string& tmp_root) : tmp_root_(tmp_root), shutdown_ } DumpPool::~DumpPool() { - shutdown(); -} - -void DumpPool::start(int thread_counts) { - assert(thread_counts > 0); - assert(threads_.empty()); - if (thread_counts > MAX_THREAD_COUNT) { - thread_counts = MAX_THREAD_COUNT; - } - MYLOGI("Start thread pool:%d", thread_counts); - shutdown_ = false; - for (int i = 0; i < thread_counts; i++) { - threads_.emplace_back(std::thread([=]() { - setThreadName(pthread_self(), i + 1); - loop(); - })); - } -} - -void DumpPool::shutdown() { std::unique_lock lock(lock_); if (shutdown_ || threads_.empty()) { return; } - futures_map_.clear(); while (!tasks_.empty()) tasks_.pop(); shutdown_ = true; @@ -76,27 +69,22 @@ void DumpPool::shutdown() { } threads_.clear(); deleteTempFiles(tmp_root_); - MYLOGI("shutdown thread pool"); + MYLOGI("shutdown thread pool\n"); } -void DumpPool::waitForTask(const std::string& task_name, const std::string& title, - int out_fd) { - DurationReporter duration_reporter("Wait for " + task_name, true); - auto iterator = futures_map_.find(task_name); - if (iterator == futures_map_.end()) { - MYLOGW("Task %s does not exist", task_name.c_str()); - return; - } - Future future = iterator->second; - futures_map_.erase(iterator); - - std::string result = future.get(); - if (result.empty()) { - return; +void DumpPool::start(int thread_counts) { + assert(thread_counts > 0); + assert(threads_.empty()); + if (thread_counts > MAX_THREAD_COUNT) { + thread_counts = MAX_THREAD_COUNT; } - DumpFileToFd(out_fd, title, result); - if (unlink(result.c_str())) { - MYLOGE("Failed to unlink (%s): %s\n", result.c_str(), strerror(errno)); + MYLOGI("Start thread pool:%d\n", thread_counts); + shutdown_ = false; + for (int i = 0; i < thread_counts; i++) { + threads_.emplace_back(std::thread([=]() { + setThreadName(pthread_self(), i + 1); + loop(); + })); } } diff --git a/cmds/dumpstate/DumpPool.h b/cmds/dumpstate/DumpPool.h index 0c3c2cc0d7..9fb0fcc83c 100644 --- a/cmds/dumpstate/DumpPool.h +++ b/cmds/dumpstate/DumpPool.h @@ -18,7 +18,6 @@ #define FRAMEWORK_NATIVE_CMD_DUMPPOOL_H_ #include <future> -#include <map> #include <queue> #include <string> @@ -32,8 +31,26 @@ namespace dumpstate { class DumpPoolTest; /* + * Waits until the task is finished. Dumps the task results to the specified + * out_fd. + * + * |future| The task future. + * |title| Dump title string to the out_fd, an empty string for nothing. + * |out_fd| The target file to dump the result from the task. + */ +void WaitForTask(std::future<std::string> future, const std::string& title, int out_fd); + +/* + * Waits until the task is finished. Dumps the task results to the STDOUT_FILENO. + */ + +inline void WaitForTask(std::future<std::string> future) { + WaitForTask(std::move(future), "", STDOUT_FILENO); +} + +/* * A thread pool with the fixed number of threads to execute multiple dump tasks - * simultaneously for the dumpstate. The dump task is a callable function. It + * simultaneously for dumpstate. The dump task is a callable function. It * could include a file descriptor as a parameter to redirect dump results, if * it needs to output results to the bugreport. This can avoid messing up * bugreport's results when multiple dump tasks are running at the same time. @@ -44,13 +61,16 @@ class DumpPoolTest; * } * ... * DumpPool pool(tmp_root); - * pool.enqueueTaskWithFd("TaskName", &DumpFoo, std::placeholders::_1); + * auto task = pool.enqueueTaskWithFd("TaskName", &DumpFoo, std::placeholders::_1); * ... - * pool.waitForTask("TaskName"); + * WaitForTask(task); * * DumpFoo is a callable function included a out_fd parameter. Using the * enqueueTaskWithFd method in DumpPool to enqueue the task to the pool. The * std::placeholders::_1 is a placeholder for DumpPool to pass a fd argument. + * + * std::futures returned by `enqueueTask*()` must all have their `get` methods + * called, or have been destroyed before the DumpPool itself is destroyed. */ class DumpPool { friend class android::os::dumpstate::DumpPoolTest; @@ -63,6 +83,12 @@ class DumpPool { * files. */ explicit DumpPool(const std::string& tmp_root); + + /* + * Will waits until all threads exit the loop. Destroying DumpPool before destroying the + * associated std::futures created by `enqueueTask*` will cause an abort on Android because + * Android is built with `-fno-exceptions`. + */ ~DumpPool(); /* @@ -73,68 +99,47 @@ class DumpPool { void start(int thread_counts = MAX_THREAD_COUNT); /* - * Requests to shutdown the pool and waits until all threads exit the loop. - */ - void shutdown(); - - /* * Adds a task into the queue of the thread pool. * - * |task_name| The name of the task. It's also the title of the + * |duration_title| The name of the task. It's also the title of the * DurationReporter log. * |f| Callable function to execute the task. * |args| A list of arguments. * * TODO(b/164369078): remove this api to have just one enqueueTask for consistency. */ - template<class F, class... Args> void enqueueTask(const std::string& task_name, F&& f, - Args&&... args) { + template<class F, class... Args> + std::future<std::string> enqueueTask(const std::string& duration_title, F&& f, Args&&... args) { std::function<void(void)> func = std::bind(std::forward<F>(f), std::forward<Args>(args)...); - futures_map_[task_name] = post(task_name, func); + auto future = post(duration_title, func); if (threads_.empty()) { start(); } + return future; } /* * Adds a task into the queue of the thread pool. The task takes a file * descriptor as a parameter to redirect dump results to a temporary file. * - * |task_name| The name of the task. It's also the title of the - * DurationReporter log. + * |duration_title| The title of the DurationReporter log. * |f| Callable function to execute the task. * |args| A list of arguments. A placeholder std::placeholders::_1 as a fd * argument needs to be included here. */ - template<class F, class... Args> void enqueueTaskWithFd(const std::string& task_name, F&& f, - Args&&... args) { + template<class F, class... Args> std::future<std::string> enqueueTaskWithFd( + const std::string& duration_title, F&& f, Args&&... args) { std::function<void(int)> func = std::bind(std::forward<F>(f), std::forward<Args>(args)...); - futures_map_[task_name] = post(task_name, func); + auto future = post(duration_title, func); if (threads_.empty()) { start(); } + return future; } /* - * Waits until the task is finished. Dumps the task results to the STDOUT_FILENO. - */ - void waitForTask(const std::string& task_name) { - waitForTask(task_name, "", STDOUT_FILENO); - } - - /* - * Waits until the task is finished. Dumps the task results to the specified - * out_fd. - * - * |task_name| The name of the task. - * |title| Dump title string to the out_fd, an empty string for nothing. - * |out_fd| The target file to dump the result from the task. - */ - void waitForTask(const std::string& task_name, const std::string& title, int out_fd); - - /* * Deletes temporary files created by DumpPool. */ void deleteTempFiles(); @@ -143,22 +148,22 @@ class DumpPool { private: using Task = std::packaged_task<std::string()>; - using Future = std::shared_future<std::string>; template<class T> void invokeTask(T dump_func, const std::string& duration_title, int out_fd); - template<class T> Future post(const std::string& task_name, T dump_func) { + template<class T> + std::future<std::string> post(const std::string& duration_title, T dump_func) { Task packaged_task([=]() { std::unique_ptr<TmpFile> tmp_file_ptr = createTempFile(); if (!tmp_file_ptr) { return std::string(""); } - invokeTask(dump_func, task_name, tmp_file_ptr->fd.get()); + invokeTask(dump_func, duration_title, tmp_file_ptr->fd.get()); fsync(tmp_file_ptr->fd.get()); return std::string(tmp_file_ptr->path); }); std::unique_lock lock(lock_); - auto future = packaged_task.get_future().share(); + auto future = packaged_task.get_future(); tasks_.push(std::move(packaged_task)); condition_variable_.notify_one(); return future; @@ -194,7 +199,6 @@ class DumpPool { std::vector<std::thread> threads_; std::queue<Task> tasks_; - std::map<std::string, Future> futures_map_; DISALLOW_COPY_AND_ASSIGN(DumpPool); }; diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp index c833d0e6bd..aa42541a66 100644 --- a/cmds/dumpstate/DumpstateUtil.cpp +++ b/cmds/dumpstate/DumpstateUtil.cpp @@ -48,11 +48,26 @@ static bool waitpid_with_timeout(pid_t pid, int timeout_ms, int* status) { sigemptyset(&child_mask); sigaddset(&child_mask, SIGCHLD); + // block SIGCHLD before we check if a process has exited if (sigprocmask(SIG_BLOCK, &child_mask, &old_mask) == -1) { printf("*** sigprocmask failed: %s\n", strerror(errno)); return false; } + // if the child has exited already, handle and reset signals before leaving + pid_t child_pid = waitpid(pid, status, WNOHANG); + if (child_pid != pid) { + if (child_pid > 0) { + printf("*** Waiting for pid %d, got pid %d instead\n", pid, child_pid); + sigprocmask(SIG_SETMASK, &old_mask, nullptr); + return false; + } + } else { + sigprocmask(SIG_SETMASK, &old_mask, nullptr); + return true; + } + + // wait for a SIGCHLD timespec ts; ts.tv_sec = MSEC_TO_SEC(timeout_ms); ts.tv_nsec = (timeout_ms % 1000) * 1000000; @@ -76,7 +91,7 @@ static bool waitpid_with_timeout(pid_t pid, int timeout_ms, int* status) { return false; } - pid_t child_pid = waitpid(pid, status, WNOHANG); + child_pid = waitpid(pid, status, WNOHANG); if (child_pid != pid) { if (child_pid != -1) { printf("*** Waiting for pid %d, got pid %d instead\n", pid, child_pid); @@ -232,7 +247,6 @@ int DumpFileToFd(int out_fd, const std::string& title, const std::string& path) dprintf(out_fd, "*** Error dumping %s (%s): %s\n", path.c_str(), title.c_str(), strerror(err)); } - fsync(out_fd); return -1; } return DumpFileFromFdToFd(title, path, fd.get(), out_fd, PropertiesHelper::IsDryRun()); @@ -280,7 +294,6 @@ int RunCommandToFd(int fd, const std::string& title, const std::vector<std::stri if (!title.empty()) { dprintf(fd, "------ %s (%s) ------\n", title.c_str(), command); - fsync(fd); } const std::string& logging_message = options.LoggingMessage(); @@ -299,14 +312,13 @@ int RunCommandToFd(int fd, const std::string& title, const std::vector<std::stri // There is no title, but we should still print a dry-run message dprintf(fd, "%s: skipped on dry run\n", command_string.c_str()); } - fsync(fd); return 0; } const char* path = args[0]; uint64_t start = Nanotime(); - pid_t pid = fork(); + pid_t pid = vfork(); /* handle error case */ if (pid < 0) { @@ -323,7 +335,7 @@ int RunCommandToFd(int fd, const std::string& title, const std::vector<std::stri strerror(errno)); } MYLOGE("*** could not drop root before running %s: %s\n", command, strerror(errno)); - return -1; + _exit(EXIT_FAILURE); } if (options.ShouldCloseAllFileDescriptorsOnExec()) { @@ -376,7 +388,6 @@ int RunCommandToFd(int fd, const std::string& title, const std::vector<std::stri /* handle parent case */ int status; bool ret = waitpid_with_timeout(pid, options.TimeoutInMs(), &status); - fsync(fd); uint64_t elapsed = Nanotime() - start; if (!ret) { diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index a951c4f0af..0e9ce897bc 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -120,6 +120,7 @@ using android::os::dumpstate::DumpFileToFd; using android::os::dumpstate::DumpPool; using android::os::dumpstate::PropertiesHelper; using android::os::dumpstate::TaskQueue; +using android::os::dumpstate::WaitForTask; // Keep in sync with // frameworks/base/services/core/java/com/android/server/am/ActivityManagerService.java @@ -218,9 +219,9 @@ static const std::string ANR_FILE_PREFIX = "anr_"; RUN_SLOW_FUNCTION_AND_LOG(log_title, func_ptr, __VA_ARGS__); \ RETURN_IF_USER_DENIED_CONSENT(); -#define WAIT_TASK_WITH_CONSENT_CHECK(task_name, pool_ptr) \ +#define WAIT_TASK_WITH_CONSENT_CHECK(future) \ RETURN_IF_USER_DENIED_CONSENT(); \ - pool_ptr->waitForTask(task_name); \ + WaitForTask(future); \ RETURN_IF_USER_DENIED_CONSENT(); static const char* WAKE_LOCK_NAME = "dumpstate_wakelock"; @@ -1087,7 +1088,7 @@ static void DumpDynamicPartitionInfo() { RunCommand("DEVICE-MAPPER", {"gsid", "dump-device-mapper"}); } -static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_dir) { +static void AddAnrTraceDir(const std::string& anr_traces_dir) { MYLOGD("AddAnrTraceDir(): dump_traces_file=%s, anr_traces_dir=%s\n", dump_traces_path, anr_traces_dir.c_str()); @@ -1095,13 +1096,9 @@ static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_ // (created with mkostemp or similar) that contains dumps taken earlier // on in the process. if (dump_traces_path != nullptr) { - if (add_to_zip) { - ds.AddZipEntry(ZIP_ROOT_DIR + anr_traces_dir + "/traces-just-now.txt", dump_traces_path); - } else { - MYLOGD("Dumping current ANR traces (%s) to the main bugreport entry\n", - dump_traces_path); - ds.DumpFile("VM TRACES JUST NOW", dump_traces_path); - } + MYLOGD("Dumping current ANR traces (%s) to the main bugreport entry\n", + dump_traces_path); + ds.DumpFile("VM TRACES JUST NOW", dump_traces_path); const int ret = unlink(dump_traces_path); if (ret == -1) { @@ -1112,14 +1109,12 @@ static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_ // Add a specific message for the first ANR Dump. if (ds.anr_data_.size() > 0) { + // The "last" ANR will always be present in the body of the main entry. AddDumps(ds.anr_data_.begin(), ds.anr_data_.begin() + 1, - "VM TRACES AT LAST ANR", add_to_zip); + "VM TRACES AT LAST ANR", false /* add_to_zip */); - // The "last" ANR will always be included as separate entry in the zip file. In addition, - // it will be present in the body of the main entry if |add_to_zip| == false. - // // Historical ANRs are always included as separate entries in the bugreport zip file. - AddDumps(ds.anr_data_.begin() + ((add_to_zip) ? 1 : 0), ds.anr_data_.end(), + AddDumps(ds.anr_data_.begin(), ds.anr_data_.end(), "HISTORICAL ANR", true /* add_to_zip */); } else { printf("*** NO ANRs to dump in %s\n\n", ANR_DIR.c_str()); @@ -1127,11 +1122,9 @@ static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_ } static void AddAnrTraceFiles() { - const bool add_to_zip = ds.version_ == VERSION_SPLIT_ANR; - std::string anr_traces_dir = "/data/anr"; - AddAnrTraceDir(add_to_zip, anr_traces_dir); + AddAnrTraceDir(anr_traces_dir); RunCommand("ANR FILES", {"ls", "-lt", ANR_DIR}); @@ -1557,15 +1550,18 @@ static Dumpstate::RunStatus dumpstate() { DurationReporter duration_reporter("DUMPSTATE"); // Enqueue slow functions into the thread pool, if the parallel run is enabled. + std::future<std::string> dump_hals, dump_incident_report, dump_board, dump_checkins; if (ds.dump_pool_) { // Pool was shutdown in DumpstateDefaultAfterCritical method in order to // drop root user. Restarts it with two threads for the parallel run. ds.dump_pool_->start(/* thread_counts = */2); - ds.dump_pool_->enqueueTaskWithFd(DUMP_HALS_TASK, &DumpHals, _1); - ds.dump_pool_->enqueueTask(DUMP_INCIDENT_REPORT_TASK, &DumpIncidentReport); - ds.dump_pool_->enqueueTaskWithFd(DUMP_BOARD_TASK, &Dumpstate::DumpstateBoard, &ds, _1); - ds.dump_pool_->enqueueTaskWithFd(DUMP_CHECKINS_TASK, &DumpCheckins, _1); + dump_hals = ds.dump_pool_->enqueueTaskWithFd(DUMP_HALS_TASK, &DumpHals, _1); + dump_incident_report = ds.dump_pool_->enqueueTask( + DUMP_INCIDENT_REPORT_TASK, &DumpIncidentReport); + dump_board = ds.dump_pool_->enqueueTaskWithFd( + DUMP_BOARD_TASK, &Dumpstate::DumpstateBoard, &ds, _1); + dump_checkins = ds.dump_pool_->enqueueTaskWithFd(DUMP_CHECKINS_TASK, &DumpCheckins, _1); } // Dump various things. Note that anything that takes "long" (i.e. several seconds) should @@ -1590,7 +1586,6 @@ static Dumpstate::RunStatus dumpstate() { DumpFile("BUDDYINFO", "/proc/buddyinfo"); DumpExternalFragmentationInfo(); - DumpFile("KERNEL WAKE SOURCES", "/d/wakeup_sources"); DumpFile("KERNEL CPUFREQ", "/sys/devices/system/cpu/cpu0/cpufreq/stats/time_in_state"); RunCommand("PROCESSES AND THREADS", @@ -1600,7 +1595,7 @@ static Dumpstate::RunStatus dumpstate() { CommandOptions::AS_ROOT); if (ds.dump_pool_) { - WAIT_TASK_WITH_CONSENT_CHECK(DUMP_HALS_TASK, ds.dump_pool_); + WAIT_TASK_WITH_CONSENT_CHECK(std::move(dump_hals)); } else { RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_HALS_TASK, DumpHals); } @@ -1697,7 +1692,7 @@ static Dumpstate::RunStatus dumpstate() { ds.AddDir(SNAPSHOTCTL_LOG_DIR, false); if (ds.dump_pool_) { - WAIT_TASK_WITH_CONSENT_CHECK(DUMP_BOARD_TASK, ds.dump_pool_); + WAIT_TASK_WITH_CONSENT_CHECK(std::move(dump_board)); } else { RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_BOARD_TASK, ds.DumpstateBoard); } @@ -1726,7 +1721,7 @@ static Dumpstate::RunStatus dumpstate() { ds.AddDir("/data/misc/bluetooth/logs", true); if (ds.dump_pool_) { - WAIT_TASK_WITH_CONSENT_CHECK(DUMP_CHECKINS_TASK, ds.dump_pool_); + WAIT_TASK_WITH_CONSENT_CHECK(std::move(dump_checkins)); } else { RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_CHECKINS_TASK, DumpCheckins); } @@ -1760,7 +1755,7 @@ static Dumpstate::RunStatus dumpstate() { dump_frozen_cgroupfs(); if (ds.dump_pool_) { - WAIT_TASK_WITH_CONSENT_CHECK(DUMP_INCIDENT_REPORT_TASK, ds.dump_pool_); + WAIT_TASK_WITH_CONSENT_CHECK(std::move(dump_incident_report)); } else { RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_INCIDENT_REPORT_TASK, DumpIncidentReport); @@ -1785,6 +1780,7 @@ Dumpstate::RunStatus Dumpstate::DumpstateDefaultAfterCritical() { time_t logcat_ts = time(nullptr); /* collect stack traces from Dalvik and native processes (needs root) */ + std::future<std::string> dump_traces; if (dump_pool_) { RETURN_IF_USER_DENIED_CONSENT(); // One thread is enough since we only need to enqueue DumpTraces here. @@ -1792,7 +1788,8 @@ Dumpstate::RunStatus Dumpstate::DumpstateDefaultAfterCritical() { // DumpTraces takes long time, post it to the another thread in the // pool, if pool is available - dump_pool_->enqueueTask(DUMP_TRACES_TASK, &Dumpstate::DumpTraces, &ds, &dump_traces_path); + dump_traces = dump_pool_->enqueueTask( + DUMP_TRACES_TASK, &Dumpstate::DumpTraces, &ds, &dump_traces_path); } else { RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_TRACES_TASK, ds.DumpTraces, &dump_traces_path); @@ -1841,12 +1838,11 @@ Dumpstate::RunStatus Dumpstate::DumpstateDefaultAfterCritical() { if (dump_pool_) { RETURN_IF_USER_DENIED_CONSENT(); - dump_pool_->waitForTask(DUMP_TRACES_TASK); + WaitForTask(std::move(dump_traces)); - // Current running thread in the pool is the root user also. Shutdown - // the pool and restart later to ensure all threads in the pool could - // drop the root user. - dump_pool_->shutdown(); + // Current running thread in the pool is the root user also. Delete + // the pool and make a new one later to ensure none of threads in the pool are root. + dump_pool_ = std::make_unique<DumpPool>(bugreport_internal_dir_); } if (!DropRootUser()) { return Dumpstate::RunStatus::ERROR; @@ -1877,8 +1873,9 @@ static void DumpstateRadioCommon(bool include_sensitive_info = true) { } else { // DumpHals takes long time, post it to the another thread in the pool, // if pool is available. + std::future<std::string> dump_hals; if (ds.dump_pool_) { - ds.dump_pool_->enqueueTaskWithFd(DUMP_HALS_TASK, &DumpHals, _1); + dump_hals = ds.dump_pool_->enqueueTaskWithFd(DUMP_HALS_TASK, &DumpHals, _1); } // Contains various system properties and process startup info. do_dmesg(); @@ -1888,7 +1885,7 @@ static void DumpstateRadioCommon(bool include_sensitive_info = true) { DoKmsg(); // DumpHals contains unrelated hardware info (camera, NFC, biometrics, ...). if (ds.dump_pool_) { - ds.dump_pool_->waitForTask(DUMP_HALS_TASK); + WaitForTask(std::move(dump_hals)); } else { RUN_SLOW_FUNCTION_AND_LOG(DUMP_HALS_TASK, DumpHals); } @@ -1922,12 +1919,14 @@ static void DumpstateTelephonyOnly(const std::string& calling_package) { // Starts thread pool after the root user is dropped, and two additional threads // are created for DumpHals in the DumpstateRadioCommon and DumpstateBoard. + std::future<std::string> dump_board; if (ds.dump_pool_) { ds.dump_pool_->start(/*thread_counts =*/2); // DumpstateBoard takes long time, post it to the another thread in the pool, // if pool is available. - ds.dump_pool_->enqueueTaskWithFd(DUMP_BOARD_TASK, &Dumpstate::DumpstateBoard, &ds, _1); + dump_board = ds.dump_pool_->enqueueTaskWithFd( + DUMP_BOARD_TASK, &Dumpstate::DumpstateBoard, &ds, _1); } DumpstateRadioCommon(include_sensitive_info); @@ -2010,7 +2009,7 @@ static void DumpstateTelephonyOnly(const std::string& calling_package) { printf("========================================================\n"); if (ds.dump_pool_) { - ds.dump_pool_->waitForTask(DUMP_BOARD_TASK); + WaitForTask(std::move(dump_board)); } else { RUN_SLOW_FUNCTION_AND_LOG(DUMP_BOARD_TASK, ds.DumpstateBoard); } @@ -2906,10 +2905,9 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, version_ = VERSION_CURRENT; } - if (version_ != VERSION_CURRENT && version_ != VERSION_SPLIT_ANR) { - MYLOGE("invalid version requested ('%s'); suppported values are: ('%s', '%s', '%s')\n", - version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str(), - VERSION_SPLIT_ANR.c_str()); + if (version_ != VERSION_CURRENT) { + MYLOGE("invalid version requested ('%s'); supported values are: ('%s', '%s')\n", + version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str()); return RunStatus::INVALID_INPUT; } @@ -3235,8 +3233,7 @@ void Dumpstate::EnableParallelRunIfNeeded() { void Dumpstate::ShutdownDumpPool() { if (dump_pool_) { - dump_pool_->shutdown(); - dump_pool_ = nullptr; + dump_pool_.reset(); } if (zip_entry_tasks_) { zip_entry_tasks_->run(/* do_cancel = */true); diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index d0acb31cc0..ee6b1aee18 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -157,12 +157,6 @@ class Progress { static std::string VERSION_CURRENT = "2.0"; /* - * Temporary version that adds a anr-traces.txt entry. Once tools support it, the current version - * will be bumped to 3.0. - */ -static std::string VERSION_SPLIT_ANR = "3.0-dev-split-anr"; - -/* * "Alias" for the current version. */ static std::string VERSION_DEFAULT = "default"; @@ -484,7 +478,7 @@ class Dumpstate { // This is useful for debugging. std::string log_path_; - // Full path of the bugreport file, be it zip or text, inside bugreport_internal_dir_. + // Full path of the bugreport zip file inside bugreport_internal_dir_. std::string path_; // Full path of the file containing the screenshot (when requested). diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index 42beb2b6cf..70b4e5c0d8 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -1720,14 +1720,13 @@ TEST_F(DumpPoolTest, EnqueueTaskWithFd) { dprintf(out_fd, "C"); }; setLogDuration(/* log_duration = */false); - dump_pool_->enqueueTaskWithFd(/* task_name = */"1", dump_func_1, std::placeholders::_1); - dump_pool_->enqueueTaskWithFd(/* task_name = */"2", dump_func_2, std::placeholders::_1); - dump_pool_->enqueueTaskWithFd(/* task_name = */"3", dump_func_3, std::placeholders::_1); + auto t1 = dump_pool_->enqueueTaskWithFd("", dump_func_1, std::placeholders::_1); + auto t2 = dump_pool_->enqueueTaskWithFd("", dump_func_2, std::placeholders::_1); + auto t3 = dump_pool_->enqueueTaskWithFd("", dump_func_3, std::placeholders::_1); - dump_pool_->waitForTask("1", "", out_fd_.get()); - dump_pool_->waitForTask("2", "", out_fd_.get()); - dump_pool_->waitForTask("3", "", out_fd_.get()); - dump_pool_->shutdown(); + WaitForTask(std::move(t1), "", out_fd_.get()); + WaitForTask(std::move(t2), "", out_fd_.get()); + WaitForTask(std::move(t3), "", out_fd_.get()); std::string result; ReadFileToString(out_path_, &result); @@ -1741,9 +1740,8 @@ TEST_F(DumpPoolTest, EnqueueTask_withDurationLog) { run_1 = true; }; - dump_pool_->enqueueTask(/* task_name = */"1", dump_func_1); - dump_pool_->waitForTask("1", "", out_fd_.get()); - dump_pool_->shutdown(); + auto t1 = dump_pool_->enqueueTask(/* duration_title = */"1", dump_func_1); + WaitForTask(std::move(t1), "", out_fd_.get()); std::string result; ReadFileToString(out_path_, &result); @@ -1752,27 +1750,6 @@ TEST_F(DumpPoolTest, EnqueueTask_withDurationLog) { EXPECT_THAT(getTempFileCounts(kTestDataPath), Eq(0)); } -TEST_F(DumpPoolTest, Shutdown_withoutCrash) { - bool run_1 = false; - auto dump_func_1 = [&]() { - run_1 = true; - }; - auto dump_func = []() { - sleep(1); - }; - - dump_pool_->start(/* thread_counts = */1); - dump_pool_->enqueueTask(/* task_name = */"1", dump_func_1); - dump_pool_->enqueueTask(/* task_name = */"2", dump_func); - dump_pool_->enqueueTask(/* task_name = */"3", dump_func); - dump_pool_->enqueueTask(/* task_name = */"4", dump_func); - dump_pool_->waitForTask("1", "", out_fd_.get()); - dump_pool_->shutdown(); - - EXPECT_TRUE(run_1); - EXPECT_THAT(getTempFileCounts(kTestDataPath), Eq(0)); -} - class TaskQueueTest : public DumpstateBaseTest { public: void SetUp() { diff --git a/cmds/installd/Android.bp b/cmds/installd/Android.bp index 00babc3346..c9f680b266 100644 --- a/cmds/installd/Android.bp +++ b/cmds/installd/Android.bp @@ -51,6 +51,7 @@ cc_defaults { ], static_libs: [ "libasync_safe", + "libext2_uuid", ], export_shared_lib_headers: [ "libbinder", @@ -262,6 +263,7 @@ cc_binary { "libasync_safe", "libdiskusage", "libotapreoptparameters", + "libext2_uuid", ], shared_libs: [ diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index c3256fcfff..4e3aae4312 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -103,11 +103,6 @@ static constexpr const char* PKG_LIB_POSTFIX = "/lib"; static constexpr const char* CACHE_DIR_POSTFIX = "/cache"; static constexpr const char* CODE_CACHE_DIR_POSTFIX = "/code_cache"; -// fsverity assumes the page size is always 4096. If not, the feature can not be -// enabled. -static constexpr int kVerityPageSize = 4096; -static constexpr size_t kSha256Size = 32; -static constexpr const char* kPropApkVerityMode = "ro.apk_verity.mode"; static constexpr const char* kFuseProp = "persist.sys.fuse"; /** @@ -261,12 +256,6 @@ binder::Status checkArgumentPath(const std::optional<std::string>& path) { } \ } -#define ASSERT_PAGE_SIZE_4K() { \ - if (getpagesize() != kVerityPageSize) { \ - return error("FSVerity only supports 4K pages"); \ - } \ -} - #ifdef GRANULAR_LOCKS /** @@ -698,9 +687,6 @@ binder::Status InstalldNativeService::createAppDataLocked( if (!status.isOk()) { return status; } - if (previousUid != uid) { - chown_app_profile_dir(packageName, appId, userId); - } // Remember inode numbers of cache directories so that we can clear // contents while CE storage is locked @@ -726,6 +712,9 @@ binder::Status InstalldNativeService::createAppDataLocked( if (!status.isOk()) { return status; } + if (previousUid != uid) { + chown_app_profile_dir(packageName, appId, userId); + } if (!prepare_app_profile_dir(packageName, appId, userId)) { return error("Failed to prepare profiles for " + packageName); @@ -968,13 +957,13 @@ binder::Status InstalldNativeService::destroyAppData(const std::optional<std::st binder::Status res = ok(); if (flags & FLAG_STORAGE_CE) { auto path = create_data_user_ce_package_path(uuid_, userId, pkgname, ceDataInode); - if (delete_dir_contents_and_dir(path) != 0) { + if (rename_delete_dir_contents_and_dir(path) != 0) { res = error("Failed to delete " + path); } } if (flags & FLAG_STORAGE_DE) { auto path = create_data_user_de_package_path(uuid_, userId, pkgname); - if (delete_dir_contents_and_dir(path) != 0) { + if (rename_delete_dir_contents_and_dir(path) != 0) { res = error("Failed to delete " + path); } if ((flags & FLAG_CLEAR_APP_DATA_KEEP_ART_PROFILES) == 0) { @@ -1008,7 +997,6 @@ binder::Status InstalldNativeService::destroyAppData(const std::optional<std::st if (delete_dir_contents_and_dir(path, true) != 0) { res = error("Failed to delete contents of " + path); } - path = StringPrintf("%s/Android/media/%s", extPath.c_str(), pkgname); if (delete_dir_contents_and_dir(path, true) != 0) { res = error("Failed to delete contents of " + path); @@ -2022,6 +2010,10 @@ binder::Status InstalldNativeService::getAppSize(const std::optional<std::string const std::vector<std::string>& codePaths, std::vector<int64_t>* _aidl_return) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_UUID(uuid); + if (packageNames.size() != ceDataInodes.size()) { + return exception(binder::Status::EX_ILLEGAL_ARGUMENT, + "packageNames/ceDataInodes size mismatch."); + } for (const auto& packageName : packageNames) { CHECK_ARGUMENT_PACKAGE_NAME(packageName); } @@ -2959,142 +2951,6 @@ binder::Status InstalldNativeService::deleteOdex(const std::string& apkPath, return *_aidl_return == -1 ? error() : ok(); } -// This kernel feature is experimental. -// TODO: remove local definition once upstreamed -#ifndef FS_IOC_ENABLE_VERITY - -#define FS_IOC_ENABLE_VERITY _IO('f', 133) -#define FS_IOC_SET_VERITY_MEASUREMENT _IOW('f', 134, struct fsverity_measurement) - -#define FS_VERITY_ALG_SHA256 1 - -struct fsverity_measurement { - __u16 digest_algorithm; - __u16 digest_size; - __u32 reserved1; - __u64 reserved2[3]; - __u8 digest[]; -}; - -#endif - -binder::Status InstalldNativeService::installApkVerity(const std::string& filePath, - android::base::unique_fd verityInputAshmem, int32_t contentSize) { - ENFORCE_UID(AID_SYSTEM); - CHECK_ARGUMENT_PATH(filePath); - LOCK_PACKAGE(); - - if (!android::base::GetBoolProperty(kPropApkVerityMode, false)) { - return ok(); - } -#ifndef NDEBUG - ASSERT_PAGE_SIZE_4K(); -#endif - // TODO: also check fsverity support in the current file system if compiled with DEBUG. - // TODO: change ashmem to some temporary file to support huge apk. - if (!ashmem_valid(verityInputAshmem.get())) { - return error("FD is not an ashmem"); - } - - // 1. Seek to the next page boundary beyond the end of the file. - ::android::base::unique_fd wfd(open(filePath.c_str(), O_WRONLY)); - if (wfd.get() < 0) { - return error("Failed to open " + filePath); - } - struct stat st; - if (fstat(wfd.get(), &st) < 0) { - return error("Failed to stat " + filePath); - } - // fsverity starts from the block boundary. - off_t padding = kVerityPageSize - st.st_size % kVerityPageSize; - if (padding == kVerityPageSize) { - padding = 0; - } - if (lseek(wfd.get(), st.st_size + padding, SEEK_SET) < 0) { - return error("Failed to lseek " + filePath); - } - - // 2. Write everything in the ashmem to the file. Note that allocated - // ashmem size is multiple of page size, which is different from the - // actual content size. - int shmSize = ashmem_get_size_region(verityInputAshmem.get()); - if (shmSize < 0) { - return error("Failed to get ashmem size: " + std::to_string(shmSize)); - } - if (contentSize < 0) { - return error("Invalid content size: " + std::to_string(contentSize)); - } - if (contentSize > shmSize) { - return error("Content size overflow: " + std::to_string(contentSize) + " > " + - std::to_string(shmSize)); - } - auto data = std::unique_ptr<void, std::function<void (void *)>>( - mmap(nullptr, contentSize, PROT_READ, MAP_SHARED, verityInputAshmem.get(), 0), - [contentSize] (void* ptr) { - if (ptr != MAP_FAILED) { - munmap(ptr, contentSize); - } - }); - - if (data.get() == MAP_FAILED) { - return error("Failed to mmap the ashmem"); - } - char* cursor = reinterpret_cast<char*>(data.get()); - int remaining = contentSize; - while (remaining > 0) { - int ret = TEMP_FAILURE_RETRY(write(wfd.get(), cursor, remaining)); - if (ret < 0) { - return error("Failed to write to " + filePath + " (" + std::to_string(remaining) + - + "/" + std::to_string(contentSize) + ")"); - } - cursor += ret; - remaining -= ret; - } - wfd.reset(); - - // 3. Enable fsverity (needs readonly fd. Once it's done, the file becomes immutable. - ::android::base::unique_fd rfd(open(filePath.c_str(), O_RDONLY)); - if (ioctl(rfd.get(), FS_IOC_ENABLE_VERITY, nullptr) < 0) { - return error("Failed to enable fsverity on " + filePath); - } - return ok(); -} - -binder::Status InstalldNativeService::assertFsverityRootHashMatches(const std::string& filePath, - const std::vector<uint8_t>& expectedHash) { - ENFORCE_UID(AID_SYSTEM); - CHECK_ARGUMENT_PATH(filePath); - LOCK_PACKAGE(); - - if (!android::base::GetBoolProperty(kPropApkVerityMode, false)) { - return ok(); - } - // TODO: also check fsverity support in the current file system if compiled with DEBUG. - if (expectedHash.size() != kSha256Size) { - return error("verity hash size should be " + std::to_string(kSha256Size) + " but is " + - std::to_string(expectedHash.size())); - } - - ::android::base::unique_fd fd(open(filePath.c_str(), O_RDONLY)); - if (fd.get() < 0) { - return error("Failed to open " + filePath + ": " + strerror(errno)); - } - - unsigned int buffer_size = sizeof(fsverity_measurement) + kSha256Size; - std::vector<char> buffer(buffer_size, 0); - - fsverity_measurement* config = reinterpret_cast<fsverity_measurement*>(buffer.data()); - config->digest_algorithm = FS_VERITY_ALG_SHA256; - config->digest_size = kSha256Size; - memcpy(config->digest, expectedHash.data(), kSha256Size); - if (ioctl(fd.get(), FS_IOC_SET_VERITY_MEASUREMENT, config) < 0) { - // This includes an expected failure case with no FSVerity setup. It normally happens when - // the apk does not contains the Merkle tree root hash. - return error("Failed to measure fsverity on " + filePath + ": " + strerror(errno)); - } - return ok(); // hashes match -} - binder::Status InstalldNativeService::reconcileSecondaryDexFile( const std::string& dexPath, const std::string& packageName, int32_t uid, const std::vector<std::string>& isas, const std::optional<std::string>& volumeUuid, @@ -3324,5 +3180,22 @@ binder::Status InstalldNativeService::migrateLegacyObbData() { return ok(); } +binder::Status InstalldNativeService::cleanupInvalidPackageDirs( + const std::optional<std::string>& uuid, int32_t userId, int32_t flags) { + const char* uuid_cstr = uuid ? uuid->c_str() : nullptr; + + if (flags & FLAG_STORAGE_CE) { + auto ce_path = create_data_user_ce_path(uuid_cstr, userId); + cleanup_invalid_package_dirs_under_path(ce_path); + } + + if (flags & FLAG_STORAGE_DE) { + auto de_path = create_data_user_de_path(uuid_cstr, userId); + cleanup_invalid_package_dirs_under_path(de_path); + } + + return ok(); +} + } // namespace installd } // namespace android diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h index 09581bb544..96783c3dbb 100644 --- a/cmds/installd/InstalldNativeService.h +++ b/cmds/installd/InstalldNativeService.h @@ -164,10 +164,6 @@ public: const std::string& outputPath); binder::Status deleteOdex(const std::string& apkPath, const std::string& instructionSet, const std::optional<std::string>& outputPath, int64_t* _aidl_return); - binder::Status installApkVerity(const std::string& filePath, - android::base::unique_fd verityInput, int32_t contentSize); - binder::Status assertFsverityRootHashMatches(const std::string& filePath, - const std::vector<uint8_t>& expectedHash); binder::Status reconcileSecondaryDexFile(const std::string& dexPath, const std::string& packageName, int32_t uid, const std::vector<std::string>& isa, const std::optional<std::string>& volumeUuid, int32_t storage_flag, bool* _aidl_return); @@ -188,6 +184,9 @@ public: binder::Status migrateLegacyObbData(); + binder::Status cleanupInvalidPackageDirs(const std::optional<std::string>& uuid, int32_t userId, + int32_t flags); + private: std::recursive_mutex mLock; std::unordered_map<userid_t, std::weak_ptr<std::shared_mutex>> mUserIdLock; diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl index 637a9f2171..f4fd9a94de 100644 --- a/cmds/installd/binder/android/os/IInstalld.aidl +++ b/cmds/installd/binder/android/os/IInstalld.aidl @@ -97,9 +97,6 @@ interface IInstalld { @utf8InCpp String outputPath); long deleteOdex(@utf8InCpp String apkPath, @utf8InCpp String instructionSet, @nullable @utf8InCpp String outputPath); - void installApkVerity(@utf8InCpp String filePath, in FileDescriptor verityInput, - int contentSize); - void assertFsverityRootHashMatches(@utf8InCpp String filePath, in byte[] expectedHash); boolean reconcileSecondaryDexFile(@utf8InCpp String dexPath, @utf8InCpp String pkgName, int uid, in @utf8InCpp String[] isas, @nullable @utf8InCpp String volume_uuid, @@ -129,6 +126,8 @@ interface IInstalld { void migrateLegacyObbData(); + void cleanupInvalidPackageDirs(@nullable @utf8InCpp String uuid, int userId, int flags); + const int FLAG_STORAGE_DE = 0x1; const int FLAG_STORAGE_CE = 0x2; const int FLAG_STORAGE_EXTERNAL = 0x4; diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index c796da6c46..9647865e4f 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -74,10 +74,19 @@ using android::base::ReadFdToString; using android::base::ReadFully; using android::base::StringPrintf; using android::base::WriteFully; +using android::base::borrowed_fd; using android::base::unique_fd; namespace { +// Timeout for short operations, such as merging profiles. +constexpr int kShortTimeoutMs = 60000; // 1 minute. + +// Timeout for long operations, such as compilation. This should be smaller than the Package Manager +// watchdog (PackageManagerService.WATCHDOG_TIMEOUT, 10 minutes), so that the operation will be +// aborted before that watchdog would take down the system server. +constexpr int kLongTimeoutMs = 570000; // 9.5 minutes. + class DexOptStatus { public: // Check if dexopt is cancelled and fork if it is not cancelled. @@ -486,6 +495,25 @@ static void open_profile_files(uid_t uid, const std::string& package_name, } } +// Cleans up an output file specified by a file descriptor. This function should be called whenever +// a subprocess that modifies a system-managed file crashes. +// If the subprocess crashes while it's writing to the file, the file is likely corrupted, so we +// should remove it. +// If the subprocess times out and is killed while it's acquiring a flock on the file, there is +// probably a deadlock, so it's also good to remove the file so that later operations won't +// encounter the same problem. It's safe to do so because the process that is holding the flock will +// still have access to the file until the file descriptor is closed. +// Note that we can't do `clear_reference_profile` here even if the fd points to a reference profile +// because that also requires a flock and is therefore likely to be stuck in the second case. +static bool cleanup_output_fd(int fd) { + std::string path; + bool ret = remove_file_at_fd(fd, &path); + if (ret) { + LOG(INFO) << "Removed file at path " << path; + } + return ret; +} + static constexpr int PROFMAN_BIN_RETURN_CODE_SUCCESS = 0; static constexpr int PROFMAN_BIN_RETURN_CODE_COMPILE = 1; static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_NOT_ENOUGH_DELTA = 2; @@ -497,13 +525,14 @@ static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_EMPTY_PROFILES = 7 class RunProfman : public ExecVHelper { public: - void SetupArgs(const std::vector<unique_fd>& profile_fds, - const unique_fd& reference_profile_fd, - const std::vector<unique_fd>& apk_fds, - const std::vector<std::string>& dex_locations, - bool copy_and_update, - bool for_snapshot, - bool for_boot_image) { + template <typename T, typename U> + void SetupArgs(const std::vector<T>& profile_fds, + const unique_fd& reference_profile_fd, + const std::vector<U>& apk_fds, + const std::vector<std::string>& dex_locations, + bool copy_and_update, + bool for_snapshot, + bool for_boot_image) { // TODO(calin): Assume for now we run in the bg compile job (which is in // most of the invocation). With the current data flow, is not very easy or @@ -519,11 +548,11 @@ class RunProfman : public ExecVHelper { AddArg("--reference-profile-file-fd=" + std::to_string(reference_profile_fd.get())); } - for (const unique_fd& fd : profile_fds) { + for (const T& fd : profile_fds) { AddArg("--profile-file-fd=" + std::to_string(fd.get())); } - for (const unique_fd& fd : apk_fds) { + for (const U& fd : apk_fds) { AddArg("--apk-fd=" + std::to_string(fd.get())); } @@ -582,20 +611,14 @@ class RunProfman : public ExecVHelper { for_boot_image); } - void SetupCopyAndUpdate(unique_fd&& profile_fd, - unique_fd&& reference_profile_fd, - unique_fd&& apk_fd, + void SetupCopyAndUpdate(const unique_fd& profile_fd, + const unique_fd& reference_profile_fd, + const unique_fd& apk_fd, const std::string& dex_location) { - // The fds need to stay open longer than the scope of the function, so put them into a local - // variable vector. - profiles_fd_.push_back(std::move(profile_fd)); - apk_fds_.push_back(std::move(apk_fd)); - reference_profile_fd_ = std::move(reference_profile_fd); - std::vector<std::string> dex_locations = {dex_location}; - SetupArgs(profiles_fd_, - reference_profile_fd_, - apk_fds_, - dex_locations, + SetupArgs(std::vector<borrowed_fd>{profile_fd}, + reference_profile_fd, + std::vector<borrowed_fd>{apk_fd}, + {dex_location}, /*copy_and_update=*/true, /*for_snapshot*/false, /*for_boot_image*/false); @@ -621,11 +644,6 @@ class RunProfman : public ExecVHelper { void Exec() { ExecVHelper::Exec(DexoptReturnCodes::kProfmanExec); } - - private: - unique_fd reference_profile_fd_; - std::vector<unique_fd> profiles_fd_; - std::vector<unique_fd> apk_fds_; }; static int analyze_profiles(uid_t uid, const std::string& package_name, @@ -657,13 +675,14 @@ static int analyze_profiles(uid_t uid, const std::string& package_name, profman_merge.Exec(); } /* parent */ - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); bool need_to_compile = false; bool empty_profiles = false; bool should_clear_current_profiles = false; bool should_clear_reference_profile = false; if (!WIFEXITED(return_code)) { LOG(WARNING) << "profman failed for location " << location << ": " << return_code; + cleanup_output_fd(reference_profile_fd.get()); } else { return_code = WEXITSTATUS(return_code); switch (return_code) { @@ -797,10 +816,10 @@ bool dump_profiles(int32_t uid, const std::string& pkgname, const std::string& p profman_dump.Exec(); } /* parent */ - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); if (!WIFEXITED(return_code)) { - LOG(WARNING) << "profman failed for package " << pkgname << ": " - << return_code; + LOG(WARNING) << "profman failed for package " << pkgname << ": " << return_code; + cleanup_output_fd(output_fd.get()); return false; } return true; @@ -871,7 +890,11 @@ bool copy_system_profile(const std::string& system_profile, _exit(0); } /* parent */ - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); + if (!WIFEXITED(return_code)) { + cleanup_output_fd(out_fd.get()); + return false; + } return return_code == 0; } @@ -1521,7 +1544,7 @@ static bool get_class_loader_context_dex_paths(const char* class_loader_context, } pipe_read.reset(); - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); if (!WIFEXITED(return_code)) { PLOG(ERROR) << "Error waiting for child dexoptanalyzer process"; return false; @@ -1695,7 +1718,7 @@ static SecondaryDexOptProcessResult process_secondary_dex_dexopt(const std::stri } /* parent */ - int result = wait_child(pid); + int result = wait_child_with_timeout(pid, kShortTimeoutMs); cancelled = dexopt_status_->check_if_killed_and_remove_dexopt_pid(pid); if (!WIFEXITED(result)) { if ((WTERMSIG(result) == SIGKILL) && cancelled) { @@ -1954,7 +1977,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins runner.Exec(DexoptReturnCodes::kDex2oatExec); } else { - int res = wait_child(pid); + int res = wait_child_with_timeout(pid, kLongTimeoutMs); bool cancelled = dexopt_status_->check_if_killed_and_remove_dexopt_pid(pid); if (res == 0) { LOG(VERBOSE) << "DexInv: --- END '" << dex_path << "' (success) ---"; @@ -2143,7 +2166,7 @@ bool reconcile_secondary_dex_file(const std::string& dex_path, _exit(result ? kReconcileSecondaryDexCleanedUp : kReconcileSecondaryDexAccessIOError); } - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); if (!WIFEXITED(return_code)) { LOG(WARNING) << "reconcile dex failed for location " << dex_path << ": " << return_code; } else { @@ -2261,7 +2284,7 @@ bool hash_secondary_dex_file(const std::string& dex_path, const std::string& pkg if (!ReadFully(pipe_read, out_secondary_dex_hash->data(), out_secondary_dex_hash->size())) { out_secondary_dex_hash->clear(); } - return wait_child(pid) == 0; + return wait_child_with_timeout(pid, kShortTimeoutMs) == 0; } // Helper for move_ab, so that we can have common failure-case cleanup. @@ -2591,9 +2614,10 @@ static bool create_app_profile_snapshot(int32_t app_id, } /* parent */ - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); if (!WIFEXITED(return_code)) { LOG(WARNING) << "profman failed for " << package_name << ":" << profile_name; + cleanup_output_fd(snapshot_fd.get()); return false; } @@ -2700,10 +2724,11 @@ static bool create_boot_image_profile_snapshot(const std::string& package_name, } /* parent */ - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); if (!WIFEXITED(return_code)) { PLOG(WARNING) << "profman failed for " << package_name << ":" << profile_name; + cleanup_output_fd(snapshot_fd.get()); return false; } @@ -2774,9 +2799,9 @@ bool prepare_app_profile(const std::string& package_name, } RunProfman args; - args.SetupCopyAndUpdate(std::move(dex_metadata_fd), - std::move(ref_profile_fd), - std::move(apk_fd), + args.SetupCopyAndUpdate(dex_metadata_fd, + ref_profile_fd, + apk_fd, code_path); pid_t pid = fork(); if (pid == 0) { @@ -2789,9 +2814,10 @@ bool prepare_app_profile(const std::string& package_name, } /* parent */ - int return_code = wait_child(pid); + int return_code = wait_child_with_timeout(pid, kShortTimeoutMs); if (!WIFEXITED(return_code)) { PLOG(WARNING) << "profman failed for " << package_name << ":" << profile_name; + cleanup_output_fd(ref_profile_fd.get()); return false; } return true; diff --git a/cmds/installd/tests/Android.bp b/cmds/installd/tests/Android.bp index 51f7716d3a..e390babb57 100644 --- a/cmds/installd/tests/Android.bp +++ b/cmds/installd/tests/Android.bp @@ -25,6 +25,7 @@ cc_test { static_libs: [ "libasync_safe", "libdiskusage", + "libext2_uuid", "libinstalld", "liblog", ], @@ -53,6 +54,7 @@ cc_test { static_libs: [ "libasync_safe", "libdiskusage", + "libext2_uuid", "libinstalld", "libziparchive", "liblog", @@ -99,6 +101,7 @@ cc_test { static_libs: [ "libasync_safe", "libdiskusage", + "libext2_uuid", "libinstalld", "libziparchive", "liblog", @@ -144,6 +147,7 @@ cc_test { static_libs: [ "libasync_safe", "libdiskusage", + "libext2_uuid", "libinstalld", "liblog", "liblogwrap", @@ -204,6 +208,7 @@ cc_test { "libutils", ], static_libs: [ + "libext2_uuid", "libinstalld", "liblog", ], diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp index a937436f9c..aeba451a48 100644 --- a/cmds/installd/tests/installd_dexopt_test.cpp +++ b/cmds/installd/tests/installd_dexopt_test.cpp @@ -50,6 +50,8 @@ using android::base::unique_fd; namespace android { namespace installd { +constexpr int kTimeoutMs = 60000; + // TODO(calin): try to dedup this code. #if defined(__arm__) static const std::string kRuntimeIsa = "arm"; @@ -1072,7 +1074,7 @@ class ProfileTest : public DexoptTest { _exit(0); } /* parent */ - ASSERT_TRUE(WIFEXITED(wait_child(pid))); + ASSERT_TRUE(WIFEXITED(wait_child_with_timeout(pid, kTimeoutMs))); } void mergePackageProfiles(const std::string& package_name, @@ -1376,7 +1378,7 @@ class BootProfileTest : public ProfileTest { _exit(0); } /* parent */ - ASSERT_TRUE(WIFEXITED(wait_child(pid))); + ASSERT_TRUE(WIFEXITED(wait_child_with_timeout(pid, kTimeoutMs))); } protected: std::string intial_android_profiles_dir; diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp index b831515b94..9702d42b14 100644 --- a/cmds/installd/tests/installd_service_test.cpp +++ b/cmds/installd/tests/installd_service_test.cpp @@ -75,6 +75,7 @@ std::string get_package_name(uid_t uid) { namespace installd { constexpr const char* kTestUuid = "TEST"; +constexpr const char* kTestPath = "/data/local/tmp/user/0"; #define FLAG_FORCE InstalldNativeService::FLAG_FORCE @@ -97,7 +98,7 @@ bool create_cache_path(char path[PKG_PATH_MAX], const char *src, const char *ins } static std::string get_full_path(const char* path) { - return StringPrintf("/data/local/tmp/user/0/%s", path); + return StringPrintf("%s/%s", kTestPath, path); } static void mkdir(const char* path, uid_t owner, gid_t group, mode_t mode) { @@ -107,12 +108,16 @@ static void mkdir(const char* path, uid_t owner, gid_t group, mode_t mode) { EXPECT_EQ(::chmod(fullPath.c_str(), mode), 0); } -static void touch(const char* path, uid_t owner, gid_t group, mode_t mode) { +static int create(const char* path, uid_t owner, gid_t group, mode_t mode) { int fd = ::open(get_full_path(path).c_str(), O_RDWR | O_CREAT, mode); EXPECT_NE(fd, -1); EXPECT_EQ(::fchown(fd, owner, group), 0); EXPECT_EQ(::fchmod(fd, mode), 0); - EXPECT_EQ(::close(fd), 0); + return fd; +} + +static void touch(const char* path, uid_t owner, gid_t group, mode_t mode) { + EXPECT_EQ(::close(create(path, owner, group, mode)), 0); } static int stat_gid(const char* path) { @@ -127,6 +132,35 @@ static int stat_mode(const char* path) { return buf.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO | S_ISGID); } +static bool exists(const char* path) { + return ::access(get_full_path(path).c_str(), F_OK) == 0; +} + +template <class Pred> +static bool find_file(const char* path, Pred&& pred) { + bool result = false; + auto d = opendir(path); + if (d == nullptr) { + return result; + } + struct dirent* de; + while ((de = readdir(d))) { + const char* name = de->d_name; + if (pred(name, de->d_type == DT_DIR)) { + result = true; + break; + } + } + closedir(d); + return result; +} + +static bool exists_renamed_deleted_dir() { + return find_file(kTestPath, [](const std::string& name, bool is_dir) { + return is_dir && is_renamed_deleted_dir(name); + }); +} + class ServiceTest : public testing::Test { protected: InstalldNativeService* service; @@ -193,6 +227,134 @@ TEST_F(ServiceTest, FixupAppData_Moved) { EXPECT_EQ(10000, stat_gid("com.example/bar/file")); } +TEST_F(ServiceTest, DestroyUserData) { + LOG(INFO) << "DestroyUserData"; + + mkdir("com.example", 10000, 10000, 0700); + mkdir("com.example/foo", 10000, 10000, 0700); + touch("com.example/foo/file", 10000, 20000, 0700); + mkdir("com.example/bar", 10000, 20000, 0700); + touch("com.example/bar/file", 10000, 20000, 0700); + + EXPECT_TRUE(exists("com.example/foo")); + EXPECT_TRUE(exists("com.example/foo/file")); + EXPECT_TRUE(exists("com.example/bar")); + EXPECT_TRUE(exists("com.example/bar/file")); + + service->destroyUserData(testUuid, 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE); + + EXPECT_FALSE(exists("com.example/foo")); + EXPECT_FALSE(exists("com.example/foo/file")); + EXPECT_FALSE(exists("com.example/bar")); + EXPECT_FALSE(exists("com.example/bar/file")); + + EXPECT_FALSE(exists_renamed_deleted_dir()); +} + +TEST_F(ServiceTest, DestroyAppData) { + LOG(INFO) << "DestroyAppData"; + + mkdir("com.example", 10000, 10000, 0700); + mkdir("com.example/foo", 10000, 10000, 0700); + touch("com.example/foo/file", 10000, 20000, 0700); + mkdir("com.example/bar", 10000, 20000, 0700); + touch("com.example/bar/file", 10000, 20000, 0700); + + EXPECT_TRUE(exists("com.example/foo")); + EXPECT_TRUE(exists("com.example/foo/file")); + EXPECT_TRUE(exists("com.example/bar")); + EXPECT_TRUE(exists("com.example/bar/file")); + + service->destroyAppData(testUuid, "com.example", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, 0); + + EXPECT_FALSE(exists("com.example/foo")); + EXPECT_FALSE(exists("com.example/foo/file")); + EXPECT_FALSE(exists("com.example/bar")); + EXPECT_FALSE(exists("com.example/bar/file")); + + EXPECT_FALSE(exists_renamed_deleted_dir()); +} + +TEST_F(ServiceTest, CleanupInvalidPackageDirs) { + LOG(INFO) << "CleanupInvalidPackageDirs"; + + mkdir("5b14b6458a44==deleted==", 10000, 10000, 0700); + mkdir("5b14b6458a44==deleted==/foo", 10000, 10000, 0700); + touch("5b14b6458a44==deleted==/foo/file", 10000, 20000, 0700); + mkdir("5b14b6458a44==deleted==/bar", 10000, 20000, 0700); + touch("5b14b6458a44==deleted==/bar/file", 10000, 20000, 0700); + + auto fd = create("5b14b6458a44==deleted==/bar/opened_file", 10000, 20000, 0700); + + mkdir("b14b6458a44NOTdeleted", 10000, 10000, 0700); + mkdir("b14b6458a44NOTdeleted/foo", 10000, 10000, 0700); + touch("b14b6458a44NOTdeleted/foo/file", 10000, 20000, 0700); + mkdir("b14b6458a44NOTdeleted/bar", 10000, 20000, 0700); + touch("b14b6458a44NOTdeleted/bar/file", 10000, 20000, 0700); + + mkdir("com.example", 10000, 10000, 0700); + mkdir("com.example/foo", 10000, 10000, 0700); + touch("com.example/foo/file", 10000, 20000, 0700); + mkdir("com.example/bar", 10000, 20000, 0700); + touch("com.example/bar/file", 10000, 20000, 0700); + + mkdir("==deleted==", 10000, 10000, 0700); + mkdir("==deleted==/foo", 10000, 10000, 0700); + touch("==deleted==/foo/file", 10000, 20000, 0700); + mkdir("==deleted==/bar", 10000, 20000, 0700); + touch("==deleted==/bar/file", 10000, 20000, 0700); + + EXPECT_TRUE(exists("5b14b6458a44==deleted==/foo")); + EXPECT_TRUE(exists("5b14b6458a44==deleted==/foo/file")); + EXPECT_TRUE(exists("5b14b6458a44==deleted==/bar")); + EXPECT_TRUE(exists("5b14b6458a44==deleted==/bar/file")); + EXPECT_TRUE(exists("5b14b6458a44==deleted==/bar/opened_file")); + + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo")); + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo/file")); + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar")); + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar/file")); + + EXPECT_TRUE(exists("com.example/foo")); + EXPECT_TRUE(exists("com.example/foo/file")); + EXPECT_TRUE(exists("com.example/bar")); + EXPECT_TRUE(exists("com.example/bar/file")); + + EXPECT_TRUE(exists("==deleted==/foo")); + EXPECT_TRUE(exists("==deleted==/foo/file")); + EXPECT_TRUE(exists("==deleted==/bar")); + EXPECT_TRUE(exists("==deleted==/bar/file")); + + EXPECT_TRUE(exists_renamed_deleted_dir()); + + service->cleanupInvalidPackageDirs(testUuid, 0, FLAG_STORAGE_CE | FLAG_STORAGE_DE); + + EXPECT_EQ(::close(fd), 0); + + EXPECT_FALSE(exists("5b14b6458a44==deleted==/foo")); + EXPECT_FALSE(exists("5b14b6458a44==deleted==/foo/file")); + EXPECT_FALSE(exists("5b14b6458a44==deleted==/bar")); + EXPECT_FALSE(exists("5b14b6458a44==deleted==/bar/file")); + EXPECT_FALSE(exists("5b14b6458a44==deleted==/bar/opened_file")); + + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo")); + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo/file")); + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar")); + EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar/file")); + + EXPECT_TRUE(exists("com.example/foo")); + EXPECT_TRUE(exists("com.example/foo/file")); + EXPECT_TRUE(exists("com.example/bar")); + EXPECT_TRUE(exists("com.example/bar/file")); + + EXPECT_FALSE(exists("==deleted==/foo")); + EXPECT_FALSE(exists("==deleted==/foo/file")); + EXPECT_FALSE(exists("==deleted==/bar")); + EXPECT_FALSE(exists("==deleted==/bar/file")); + + EXPECT_FALSE(exists_renamed_deleted_dir()); +} + TEST_F(ServiceTest, HashSecondaryDex) { LOG(INFO) << "HashSecondaryDex"; @@ -323,6 +485,19 @@ TEST_F(ServiceTest, GetAppSize) { system(removeCommand.c_str()); } } +TEST_F(ServiceTest, GetAppSizeWrongSizes) { + int32_t externalStorageAppId = -1; + std::vector<int64_t> externalStorageSize; + + std::vector<std::string> codePaths; + std::vector<std::string> packageNames = {"package1", "package2"}; + std::vector<int64_t> ceDataInodes = {0}; + + EXPECT_BINDER_FAIL(service->getAppSize(std::nullopt, packageNames, 0, + InstalldNativeService::FLAG_USE_QUOTA, + externalStorageAppId, ceDataInodes, codePaths, + &externalStorageSize)); +} static bool mkdirs(const std::string& path, mode_t mode) { struct stat sb; if (stat(path.c_str(), &sb) != -1 && S_ISDIR(sb.st_mode)) { diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp index ed87b672ce..514b88135a 100644 --- a/cmds/installd/tests/installd_utils_test.cpp +++ b/cmds/installd/tests/installd_utils_test.cpp @@ -14,8 +14,10 @@ * limitations under the License. */ +#include <errno.h> #include <stdlib.h> #include <string.h> +#include <unistd.h> #include <android-base/logging.h> #include <android-base/scopeguard.h> @@ -555,6 +557,24 @@ TEST_F(UtilsTest, MatchExtension_Invalid) { EXPECT_EQ(0, MatchExtension("docx")); } +TEST_F(UtilsTest, TestIsRenamedDeletedDir) { + EXPECT_FALSE(is_renamed_deleted_dir("")); + EXPECT_FALSE(is_renamed_deleted_dir("1")); + EXPECT_FALSE(is_renamed_deleted_dir("=")); + EXPECT_FALSE(is_renamed_deleted_dir("==")); + EXPECT_FALSE(is_renamed_deleted_dir("d==")); + EXPECT_FALSE(is_renamed_deleted_dir("ed==")); + EXPECT_FALSE(is_renamed_deleted_dir("ted==")); + EXPECT_FALSE(is_renamed_deleted_dir("eted==")); + EXPECT_FALSE(is_renamed_deleted_dir("leted==")); + EXPECT_FALSE(is_renamed_deleted_dir("eleted==")); + EXPECT_FALSE(is_renamed_deleted_dir("deleted==")); + EXPECT_FALSE(is_renamed_deleted_dir("=deleted==")); + EXPECT_TRUE(is_renamed_deleted_dir("==deleted==")); + EXPECT_TRUE(is_renamed_deleted_dir("123==deleted==")); + EXPECT_TRUE(is_renamed_deleted_dir("5b14b6458a44==deleted==")); +} + TEST_F(UtilsTest, TestRollbackPaths) { EXPECT_EQ("/data/misc_ce/0/rollback/239/com.foo", create_data_misc_ce_rollback_package_path(nullptr, 0, 239, "com.foo")); @@ -638,5 +658,45 @@ TEST_F(UtilsTest, TestCreateDirIfNeeded) { ASSERT_NE(0, create_dir_if_needed("/data/local/tmp/user/0/bar/baz", 0700)); } +TEST_F(UtilsTest, WaitChild) { + pid_t pid = fork(); + if (pid == 0) { + /* child */ + // Do nothing. + _exit(0); + } + /* parent */ + int return_code = wait_child_with_timeout(pid, /*timeout_ms=*/100); + EXPECT_TRUE(WIFEXITED(return_code)); + EXPECT_EQ(WEXITSTATUS(return_code), 0); +} + +TEST_F(UtilsTest, WaitChildTimeout) { + pid_t pid = fork(); + if (pid == 0) { + /* child */ + sleep(1); + _exit(0); + } + /* parent */ + int return_code = wait_child_with_timeout(pid, /*timeout_ms=*/1); + EXPECT_FALSE(WIFEXITED(return_code)); + EXPECT_EQ(WTERMSIG(return_code), SIGKILL); +} + +TEST_F(UtilsTest, RemoveFileAtFd) { + std::string filename = "/data/local/tmp/tempfile-XXXXXX"; + int fd = mkstemp(filename.data()); + ASSERT_GE(fd, 0); + ASSERT_EQ(access(filename.c_str(), F_OK), 0); + + std::string actual_filename; + remove_file_at_fd(fd, &actual_filename); + EXPECT_NE(access(filename.c_str(), F_OK), 0); + EXPECT_EQ(filename, actual_filename); + + close(fd); +} + } // namespace installd } // namespace android diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp index 0f8a732345..f04ee337d9 100644 --- a/cmds/installd/utils.cpp +++ b/cmds/installd/utils.cpp @@ -19,17 +19,21 @@ #include <errno.h> #include <fcntl.h> #include <fts.h> +#include <poll.h> #include <stdlib.h> #include <sys/capability.h> +#include <sys/pidfd.h> #include <sys/stat.h> +#include <sys/statvfs.h> #include <sys/wait.h> #include <sys/xattr.h> -#include <sys/statvfs.h> +#include <unistd.h> +#include <uuid/uuid.h> #include <android-base/file.h> #include <android-base/logging.h> -#include <android-base/strings.h> #include <android-base/stringprintf.h> +#include <android-base/strings.h> #include <android-base/unique_fd.h> #include <cutils/fs.h> #include <cutils/properties.h> @@ -47,6 +51,7 @@ #define DEBUG_XATTRS 0 +using android::base::Dirname; using android::base::EndsWith; using android::base::Fdopendir; using android::base::StringPrintf; @@ -55,6 +60,10 @@ using android::base::unique_fd; namespace android { namespace installd { +using namespace std::literals; + +static constexpr auto deletedSuffix = "==deleted=="sv; + /** * Check that given string is valid filename, and that it attempts no * parent or child directory traversal. @@ -595,6 +604,97 @@ int delete_dir_contents(const char *pathname, return res; } +static std::string make_unique_name(std::string_view suffix) { + static constexpr auto uuidStringSize = 36; + + uuid_t guid; + uuid_generate(guid); + + std::string name; + const auto suffixSize = suffix.size(); + name.reserve(uuidStringSize + suffixSize); + + name.resize(uuidStringSize); + uuid_unparse(guid, name.data()); + name.append(suffix); + + return name; +} + +static int rename_delete_dir_contents(const std::string& pathname, + int (*exclusion_predicate)(const char*, const int), + bool ignore_if_missing) { + auto temp_dir_name = make_unique_name(deletedSuffix); + auto temp_dir_path = + base::StringPrintf("%s/%s", Dirname(pathname).c_str(), temp_dir_name.c_str()); + + if (::rename(pathname.c_str(), temp_dir_path.c_str())) { + if (ignore_if_missing && (errno == ENOENT)) { + return 0; + } + ALOGE("Couldn't rename %s -> %s: %s \n", pathname.c_str(), temp_dir_path.c_str(), + strerror(errno)); + return -errno; + } + + return delete_dir_contents(temp_dir_path.c_str(), 1, exclusion_predicate, ignore_if_missing); +} + +bool is_renamed_deleted_dir(const std::string& path) { + if (path.size() < deletedSuffix.size()) { + return false; + } + std::string_view pathSuffix{path.c_str() + path.size() - deletedSuffix.size()}; + return pathSuffix == deletedSuffix; +} + +int rename_delete_dir_contents_and_dir(const std::string& pathname, bool ignore_if_missing) { + return rename_delete_dir_contents(pathname, nullptr, ignore_if_missing); +} + +static auto open_dir(const char* dir) { + struct DirCloser { + void operator()(DIR* d) const noexcept { ::closedir(d); } + }; + return std::unique_ptr<DIR, DirCloser>(::opendir(dir)); +} + +void cleanup_invalid_package_dirs_under_path(const std::string& pathname) { + auto dir = open_dir(pathname.c_str()); + if (!dir) { + return; + } + int dfd = dirfd(dir.get()); + if (dfd < 0) { + ALOGE("Couldn't dirfd %s: %s\n", pathname.c_str(), strerror(errno)); + return; + } + + struct dirent* de; + while ((de = readdir(dir.get()))) { + if (de->d_type != DT_DIR) { + continue; + } + + std::string name{de->d_name}; + // always skip "." and ".." + if (name == "." || name == "..") { + continue; + } + + if (is_renamed_deleted_dir(name) || !is_valid_filename(name) || + !is_valid_package_name(name)) { + ALOGI("Deleting renamed or invalid data directory: %s\n", name.c_str()); + // Deleting the content. + delete_dir_contents_fd(dfd, name.c_str()); + // Deleting the directory + if (unlinkat(dfd, name.c_str(), AT_REMOVEDIR) < 0) { + ALOGE("Couldn't unlinkat %s: %s\n", name.c_str(), strerror(errno)); + } + } + } +} + int delete_dir_contents_fd(int dfd, const char *name) { int fd, res; @@ -962,30 +1062,45 @@ int ensure_config_user_dirs(userid_t userid) { return fs_prepare_dir(path.c_str(), 0750, uid, gid); } -int wait_child(pid_t pid) -{ +static int wait_child(pid_t pid) { int status; - pid_t got_pid; + pid_t got_pid = TEMP_FAILURE_RETRY(waitpid(pid, &status, /*options=*/0)); - while (1) { - got_pid = waitpid(pid, &status, 0); - if (got_pid == -1 && errno == EINTR) { - printf("waitpid interrupted, retrying\n"); - } else { - break; - } - } if (got_pid != pid) { - ALOGW("waitpid failed: wanted %d, got %d: %s\n", - (int) pid, (int) got_pid, strerror(errno)); - return 1; + PLOG(ERROR) << "waitpid failed: wanted " << pid << ", got " << got_pid; + return W_EXITCODE(/*exit_code=*/255, /*signal_number=*/0); } - if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { - return 0; - } else { - return status; /* always nonzero */ + return status; +} + +int wait_child_with_timeout(pid_t pid, int timeout_ms) { + int pidfd = pidfd_open(pid, /*flags=*/0); + if (pidfd < 0) { + PLOG(ERROR) << "pidfd_open failed for pid " << pid; + kill(pid, SIGKILL); + return wait_child(pid); + } + + struct pollfd pfd; + pfd.fd = pidfd; + pfd.events = POLLIN; + int poll_ret = TEMP_FAILURE_RETRY(poll(&pfd, /*nfds=*/1, timeout_ms)); + + close(pidfd); + + if (poll_ret < 0) { + PLOG(ERROR) << "poll failed for pid " << pid; + kill(pid, SIGKILL); + return wait_child(pid); + } + if (poll_ret == 0) { + LOG(WARNING) << "Child process " << pid << " timed out after " << timeout_ms + << "ms. Killing it"; + kill(pid, SIGKILL); + return wait_child(pid); } + return wait_child(pid); } /** @@ -1198,5 +1313,27 @@ void drop_capabilities(uid_t uid) { } } +bool remove_file_at_fd(int fd, /*out*/ std::string* path) { + char path_buffer[PATH_MAX + 1]; + std::string proc_path = android::base::StringPrintf("/proc/self/fd/%d", fd); + ssize_t len = readlink(proc_path.c_str(), path_buffer, PATH_MAX); + if (len < 0) { + PLOG(WARNING) << "Could not remove file at fd " << fd << ": Failed to get file path"; + return false; + } + path_buffer[len] = '\0'; + if (path != nullptr) { + *path = path_buffer; + } + if (unlink(path_buffer) != 0) { + if (errno == ENOENT) { + return true; + } + PLOG(WARNING) << "Could not remove file at path " << path_buffer; + return false; + } + return true; +} + } // namespace installd } // namespace android diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h index 549fc6cf04..39738945e4 100644 --- a/cmds/installd/utils.h +++ b/cmds/installd/utils.h @@ -120,6 +120,11 @@ int create_dir_if_needed(const std::string& pathname, mode_t mode); int delete_dir_contents(const std::string& pathname, bool ignore_if_missing = false); int delete_dir_contents_and_dir(const std::string& pathname, bool ignore_if_missing = false); +bool is_renamed_deleted_dir(const std::string& path); +int rename_delete_dir_contents_and_dir(const std::string& pathname, bool ignore_if_missing = true); + +void cleanup_invalid_package_dirs_under_path(const std::string& pathname); + int delete_dir_contents(const char *pathname, int also_delete_dir, int (*exclusion_predicate)(const char *name, const int is_dir), @@ -148,7 +153,8 @@ int validate_apk_path_subdirs(const char *path); int ensure_config_user_dirs(userid_t userid); -int wait_child(pid_t pid); +// Waits for a child process, or kills it if it times out. Returns the exit code. +int wait_child_with_timeout(pid_t pid, int timeout_ms); int prepare_app_cache_dir(const std::string& parent, const char* name, mode_t target_mode, uid_t uid, gid_t gid); @@ -165,6 +171,10 @@ bool collect_profiles(std::vector<std::string>* profiles_paths); void drop_capabilities(uid_t uid); +// Removes a file specified by a file descriptor. Returns true on success. Reports the file path to +// `path` if present. +bool remove_file_at_fd(int fd, /*out*/ std::string* path = nullptr); + } // namespace installd } // namespace android diff --git a/cmds/installd/view_compiler.cpp b/cmds/installd/view_compiler.cpp index 60d6492e70..8c000a11c9 100644 --- a/cmds/installd/view_compiler.cpp +++ b/cmds/installd/view_compiler.cpp @@ -33,7 +33,13 @@ namespace android { namespace installd { -using base::unique_fd; +namespace { + +using ::android::base::unique_fd; + +constexpr int kTimeoutMs = 300000; + +} // namespace bool view_compiler(const char* apk_path, const char* package_name, const char* out_dex_file, int uid) { @@ -88,7 +94,17 @@ bool view_compiler(const char* apk_path, const char* package_name, const char* o _exit(1); } - return wait_child(pid) == 0; + int return_code = wait_child_with_timeout(pid, kTimeoutMs); + if (!WIFEXITED(return_code)) { + LOG(WARNING) << "viewcompiler failed for " << package_name << ":" << apk_path; + if (WTERMSIG(return_code) == SIGKILL) { + // If the subprocess is killed while it's writing to the file, the file is likely + // corrupted, so we should remove it. + remove_file_at_fd(outfd.get()); + } + return false; + } + return WEXITSTATUS(return_code) == 0; } } // namespace installd diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index f84f639d2c..6a138e340f 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -200,7 +200,6 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder) { } flat_binder_object obj; - obj.flags = FLAT_BINDER_FLAG_ACCEPTS_FDS; int schedBits = 0; if (!IPCThreadState::self()->backgroundSchedulingDisabled()) { @@ -221,6 +220,7 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder) { const int32_t handle = proxy ? proxy->getPrivateAccessor().binderHandle() : 0; obj.hdr.type = BINDER_TYPE_HANDLE; obj.binder = 0; /* Don't pass uninitialized stack data to a remote process */ + obj.flags = 0; obj.handle = handle; obj.cookie = 0; } else { @@ -231,6 +231,7 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder) { // override value, since it is set explicitly schedBits = schedPolicyMask(policy, priority); } + obj.flags = FLAT_BINDER_FLAG_ACCEPTS_FDS; if (local->isRequestingSid()) { obj.flags |= FLAT_BINDER_FLAG_TXN_SECURITY_CTX; } @@ -243,6 +244,7 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder) { } } else { obj.hdr.type = BINDER_TYPE_BINDER; + obj.flags = 0; obj.binder = 0; obj.cookie = 0; } diff --git a/libs/binder/PersistableBundle.cpp b/libs/binder/PersistableBundle.cpp index 406fee0227..15047152a1 100644 --- a/libs/binder/PersistableBundle.cpp +++ b/libs/binder/PersistableBundle.cpp @@ -27,13 +27,6 @@ #include "ParcelValTypes.h" -using android::BAD_TYPE; -using android::BAD_VALUE; -using android::NO_ERROR; -using android::Parcel; -using android::status_t; -using android::UNEXPECTED_NULL; - using android::binder::VAL_BOOLEAN; using android::binder::VAL_INTEGER; using android::binder::VAL_LONG; diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp index 269b0861e1..baa817c6b9 100644 --- a/libs/binder/ProcessState.cpp +++ b/libs/binder/ProcessState.cpp @@ -19,6 +19,7 @@ #include <binder/ProcessState.h> #include <android-base/result.h> +#include <android-base/strings.h> #include <binder/BpBinder.h> #include <binder/IPCThreadState.h> #include <binder/IServiceManager.h> @@ -367,8 +368,13 @@ void ProcessState::expungeHandle(int32_t handle, IBinder* binder) String8 ProcessState::makeBinderThreadName() { int32_t s = android_atomic_add(1, &mThreadPoolSeq); pid_t pid = getpid(); + + std::string_view driverName = mDriverName.c_str(); + android::base::ConsumePrefix(&driverName, "/dev/"); + String8 name; - name.appendFormat("Binder:%d_%X", pid, s); + name.appendFormat("%.*s:%d_%X", static_cast<int>(driverName.length()), driverName.data(), pid, + s); return name; } diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index 1d7de98c0d..dbfb1c20a5 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -114,8 +114,8 @@ pub use native::{ }; pub use parcel::{ParcelFileDescriptor, Parcelable, ParcelableHolder}; pub use proxy::{ - get_interface, get_service, wait_for_interface, wait_for_service, DeathRecipient, SpIBinder, - WpIBinder, + get_declared_instances, get_interface, get_service, is_declared, wait_for_interface, + wait_for_service, DeathRecipient, SpIBinder, WpIBinder, }; pub use state::{ProcessState, ThreadState}; diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs index 12bfde755e..e3e47305d4 100644 --- a/libs/binder/rust/src/proxy.rs +++ b/libs/binder/rust/src/proxy.rs @@ -28,9 +28,10 @@ use crate::sys; use std::cmp::Ordering; use std::convert::TryInto; -use std::ffi::{c_void, CString}; +use std::ffi::{c_void, CStr, CString}; use std::fmt; use std::mem; +use std::os::raw::c_char; use std::os::unix::io::AsRawFd; use std::ptr; use std::sync::Arc; @@ -778,6 +779,61 @@ pub fn wait_for_interface<T: FromIBinder + ?Sized>(name: &str) -> Result<Strong< } } +/// Check if a service is declared (e.g. in a VINTF manifest) +pub fn is_declared(interface: &str) -> Result<bool> { + let interface = CString::new(interface).or(Err(StatusCode::UNEXPECTED_NULL))?; + + unsafe { + // Safety: `interface` is a valid null-terminated C-style string and is + // only borrowed for the lifetime of the call. The `interface` local + // outlives this call as it lives for the function scope. + Ok(sys::AServiceManager_isDeclared(interface.as_ptr())) + } +} + +/// Retrieve all declared instances for a particular interface +/// +/// For instance, if 'android.foo.IFoo/foo' is declared, and 'android.foo.IFoo' +/// is passed here, then ["foo"] would be returned. +pub fn get_declared_instances(interface: &str) -> Result<Vec<String>> { + unsafe extern "C" fn callback(instance: *const c_char, opaque: *mut c_void) { + // Safety: opaque was a mutable pointer created below from a Vec of + // CString, and outlives this callback. The null handling here is just + // to avoid the possibility of unwinding across C code if this crate is + // ever compiled with panic=unwind. + if let Some(instances) = opaque.cast::<Vec<CString>>().as_mut() { + // Safety: instance is a valid null-terminated C string with a + // lifetime at least as long as this function, and we immediately + // copy it into an owned CString. + instances.push(CStr::from_ptr(instance).to_owned()); + } else { + eprintln!("Opaque pointer was null in get_declared_instances callback!"); + } + } + + let interface = CString::new(interface).or(Err(StatusCode::UNEXPECTED_NULL))?; + let mut instances: Vec<CString> = vec![]; + unsafe { + // Safety: `interface` and `instances` are borrowed for the length of + // this call and both outlive the call. `interface` is guaranteed to be + // a valid null-terminated C-style string. + sys::AServiceManager_forEachDeclaredInstance( + interface.as_ptr(), + &mut instances as *mut _ as *mut c_void, + Some(callback), + ); + } + + instances + .into_iter() + .map(CString::into_string) + .collect::<std::result::Result<Vec<String>, _>>() + .map_err(|e| { + eprintln!("An interface instance name was not a valid UTF-8 string: {}", e); + StatusCode::BAD_VALUE + }) +} + /// # Safety /// /// `SpIBinder` guarantees that `binder` always contains a valid pointer to an diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index 50daf1c0cc..c9d6af0d75 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -484,6 +484,21 @@ mod tests { } #[test] + fn get_declared_instances() { + // At the time of writing this test, there is no good VINTF interface + // guaranteed to be on all devices. Cuttlefish has light, so this will + // generally test things. + let has_lights = binder::is_declared("android.hardware.light.ILights/default") + .expect("Could not check for declared interface"); + + let instances = binder::get_declared_instances("android.hardware.light.ILights") + .expect("Could not get declared instances"); + + let expected_defaults = if has_lights { 1 } else { 0 }; + assert_eq!(expected_defaults, instances.iter().filter(|i| i.as_str() == "default").count()); + } + + #[test] fn trivial_client() { let service_name = "trivial_client_test"; let _process = ScopedServiceProcess::new(service_name); @@ -909,9 +924,9 @@ mod tests { BinderFeatures::default(), ); - assert!(!(service1 < service1)); - assert!(!(service1 > service1)); - assert_eq!(service1 < service2, !(service2 < service1)); + assert!((service1 >= service1)); + assert!((service1 <= service1)); + assert_eq!(service1 < service2, (service2 >= service1)); } #[test] diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 700940a529..b1e17b7892 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -57,6 +57,7 @@ using namespace std::chrono_literals; using android::base::testing::HasValue; using android::base::testing::Ok; using testing::ExplainMatchResult; +using testing::Matcher; using testing::Not; using testing::WithParamInterface; @@ -1258,12 +1259,23 @@ public: class BinderLibRpcTest : public BinderLibRpcTestBase {}; +// e.g. EXPECT_THAT(expr, Debuggable(StatusEq(...)) +// If device is debuggable AND not on user builds, expects matcher. +// Otherwise expects INVALID_OPERATION. +// Debuggable + non user builds is necessary but not sufficient for setRpcClientDebug to work. +static Matcher<status_t> Debuggable(const Matcher<status_t> &matcher) { + bool isDebuggable = android::base::GetBoolProperty("ro.debuggable", false) && + android::base::GetProperty("ro.build.type", "") != "user"; + return isDebuggable ? matcher : StatusEq(INVALID_OPERATION); +} + TEST_F(BinderLibRpcTest, SetRpcClientDebug) { auto binder = addServer(); ASSERT_TRUE(binder != nullptr); auto [socket, port] = CreateSocket(); ASSERT_TRUE(socket.ok()); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), sp<BBinder>::make()), StatusEq(OK)); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), sp<BBinder>::make()), + Debuggable(StatusEq(OK))); } // Tests for multiple RpcServer's on the same binder object. @@ -1274,12 +1286,14 @@ TEST_F(BinderLibRpcTest, SetRpcClientDebugTwice) { auto [socket1, port1] = CreateSocket(); ASSERT_TRUE(socket1.ok()); auto keepAliveBinder1 = sp<BBinder>::make(); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), keepAliveBinder1), StatusEq(OK)); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), keepAliveBinder1), + Debuggable(StatusEq(OK))); auto [socket2, port2] = CreateSocket(); ASSERT_TRUE(socket2.ok()); auto keepAliveBinder2 = sp<BBinder>::make(); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), keepAliveBinder2), StatusEq(OK)); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), keepAliveBinder2), + Debuggable(StatusEq(OK))); } // Negative tests for RPC APIs on IBinder. Call should fail in the same way on both remote and @@ -1298,7 +1312,7 @@ TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoFd) { auto binder = GetService(); ASSERT_TRUE(binder != nullptr); EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd(), sp<BBinder>::make()), - StatusEq(BAD_VALUE)); + Debuggable(StatusEq(BAD_VALUE))); } TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoKeepAliveBinder) { @@ -1306,7 +1320,8 @@ TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoKeepAliveBinder) { ASSERT_TRUE(binder != nullptr); auto [socket, port] = CreateSocket(); ASSERT_TRUE(socket.ok()); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), nullptr), StatusEq(UNEXPECTED_NULL)); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), nullptr), + Debuggable(StatusEq(UNEXPECTED_NULL))); } INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(), BinderLibRpcTestP::ParamToString); diff --git a/libs/binder/tests/parcel_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/Android.bp index 1446802d58..38bde3a011 100644 --- a/libs/binder/tests/parcel_fuzzer/Android.bp +++ b/libs/binder/tests/parcel_fuzzer/Android.bp @@ -26,13 +26,9 @@ cc_fuzz { static_libs: [ "libbase", "libbinder_random_parcel", - "libcgrouprc", - "libcgrouprc_format", "libcutils", "libhidlbase", "liblog", - "libprocessgroup", - "libjsoncpp", "libutils", ], diff --git a/libs/cputimeinstate/Android.bp b/libs/cputimeinstate/Android.bp index 4f63194f03..79cc15f571 100644 --- a/libs/cputimeinstate/Android.bp +++ b/libs/cputimeinstate/Android.bp @@ -13,12 +13,13 @@ cc_library { shared_libs: [ "libbase", "libbpf_bcc", - "libbpf_android", "libbpf_minimal", "liblog", - "libnetdutils" ], - header_libs: ["bpf_prog_headers"], + header_libs: [ + "bpf_prog_headers", + "bpf_headers", + ], cflags: [ "-Werror", "-Wall", @@ -33,12 +34,13 @@ cc_test { shared_libs: [ "libbase", "libbpf_bcc", - "libbpf_android", "libbpf_minimal", "libtimeinstate", - "libnetdutils", ], - header_libs: ["bpf_prog_headers"], + header_libs: [ + "bpf_prog_headers", + "bpf_headers", + ], cflags: [ "-Werror", "-Wall", diff --git a/libs/gralloc/OWNERS b/libs/gralloc/OWNERS index 93879d88ca..72ff978221 100644 --- a/libs/gralloc/OWNERS +++ b/libs/gralloc/OWNERS @@ -1 +1,4 @@ +# Graphics team +alecmouri@google.com chrisforbes@google.com +jreck@google.com
\ No newline at end of file diff --git a/libs/gralloc/types/Gralloc4.cpp b/libs/gralloc/types/Gralloc4.cpp index e2f072a7ab..81a529df7e 100644 --- a/libs/gralloc/types/Gralloc4.cpp +++ b/libs/gralloc/types/Gralloc4.cpp @@ -196,6 +196,35 @@ status_t encodeMetadataType(const MetadataType& input, OutputHidlVec* output); status_t validateMetadataType(InputHidlVec* input, const MetadataType& expectedMetadataType); /** + * Private helper functions + */ +template <class T> +status_t encodeInteger(const T& input, OutputHidlVec* output) { + static_assert(std::is_same<T, uint32_t>::value || std::is_same<T, int32_t>::value || + std::is_same<T, uint64_t>::value || std::is_same<T, int64_t>::value || + std::is_same<T, float>::value || std::is_same<T, double>::value); + if (!output) { + return BAD_VALUE; + } + + const uint8_t* tmp = reinterpret_cast<const uint8_t*>(&input); + return output->encode(tmp, sizeof(input)); +} + +template <class T> +status_t decodeInteger(InputHidlVec* input, T* output) { + static_assert(std::is_same<T, uint32_t>::value || std::is_same<T, int32_t>::value || + std::is_same<T, uint64_t>::value || std::is_same<T, int64_t>::value || + std::is_same<T, float>::value || std::is_same<T, double>::value); + if (!output) { + return BAD_VALUE; + } + + uint8_t* tmp = reinterpret_cast<uint8_t*>(output); + return input->decode(tmp, sizeof(*output)); +} + +/** * encode/encodeMetadata are the main encoding functions. They take in T and uses the encodeHelper * function to turn T into the hidl_vec byte stream. * @@ -251,10 +280,45 @@ status_t encodeMetadata(const MetadataType& metadataType, const T& input, hidl_v template <class T> status_t encodeOptionalMetadata(const MetadataType& metadataType, const std::optional<T>& input, hidl_vec<uint8_t>* output, EncodeHelper<T> encodeHelper) { - if (!input) { - return NO_ERROR; + OutputHidlVec outputHidlVec{output}; + + status_t err = encodeMetadataType(metadataType, &outputHidlVec); + if (err) { + return err; + } + + err = encodeInteger<uint32_t>(input.has_value() ? 1 : 0, &outputHidlVec); + if (err) { + return err; + } + + if (input) { + err = encodeHelper(*input, &outputHidlVec); + if (err) { + return err; + } + } + + err = outputHidlVec.resize(); + if (err) { + return err; + } + + err = encodeMetadataType(metadataType, &outputHidlVec); + if (err) { + return err; } - return encodeMetadata(metadataType, *input, output, encodeHelper); + + err = encodeInteger<uint32_t>(input.has_value() ? 1 : 0, &outputHidlVec); + if (err) { + return err; + } + + if (input) { + return encodeHelper(*input, &outputHidlVec); + } + + return NO_ERROR; } /** @@ -315,45 +379,36 @@ status_t decodeOptionalMetadata(const MetadataType& metadataType, const hidl_vec if (!output) { return BAD_VALUE; } - if (input.size() <= 0) { - output->reset(); - return NO_ERROR; - } - T tmp; - status_t err = decodeMetadata(metadataType, input, &tmp, decodeHelper); - if (!err) { - *output = tmp; + + InputHidlVec inputHidlVec{&input}; + + status_t err = validateMetadataType(&inputHidlVec, metadataType); + if (err) { + return err; } - return err; -} -/** - * Private helper functions - */ -template <class T> -status_t encodeInteger(const T& input, OutputHidlVec* output) { - static_assert(std::is_same<T, uint32_t>::value || std::is_same<T, int32_t>::value || - std::is_same<T, uint64_t>::value || std::is_same<T, int64_t>::value || - std::is_same<T, float>::value || std::is_same<T, double>::value); - if (!output) { - return BAD_VALUE; + uint32_t present = 0; + err = decodeInteger<uint32_t>(&inputHidlVec, &present); + if (err) { + return err; } - const uint8_t* tmp = reinterpret_cast<const uint8_t*>(&input); - return output->encode(tmp, sizeof(input)); -} + if (present) { + T tmp; + err = decodeHelper(&inputHidlVec, &tmp); + if (err) { + return err; + } -template <class T> -status_t decodeInteger(InputHidlVec* input, T* output) { - static_assert(std::is_same<T, uint32_t>::value || std::is_same<T, int32_t>::value || - std::is_same<T, uint64_t>::value || std::is_same<T, int64_t>::value || - std::is_same<T, float>::value || std::is_same<T, double>::value); - if (!output) { + *output = tmp; + } + + err = inputHidlVec.hasRemainingData(); + if (err) { return BAD_VALUE; } - uint8_t* tmp = reinterpret_cast<uint8_t*>(output); - return input->decode(tmp, sizeof(*output)); + return NO_ERROR; } status_t encodeString(const std::string& input, OutputHidlVec* output) { diff --git a/libs/nativewindow/AHardwareBuffer.cpp b/libs/nativewindow/AHardwareBuffer.cpp index e2f32e374a..ef7602f24e 100644 --- a/libs/nativewindow/AHardwareBuffer.cpp +++ b/libs/nativewindow/AHardwareBuffer.cpp @@ -588,6 +588,8 @@ bool AHardwareBuffer_isValidPixelFormat(uint32_t format) { "HAL and AHardwareBuffer pixel format don't match"); static_assert(HAL_PIXEL_FORMAT_YCBCR_422_I == AHARDWAREBUFFER_FORMAT_YCbCr_422_I, "HAL and AHardwareBuffer pixel format don't match"); + static_assert(HAL_PIXEL_FORMAT_YCBCR_P010 == AHARDWAREBUFFER_FORMAT_YCbCr_P010, + "HAL and AHardwareBuffer pixel format don't match"); switch (format) { case AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM: @@ -617,6 +619,7 @@ bool AHardwareBuffer_isValidPixelFormat(uint32_t format) { case AHARDWAREBUFFER_FORMAT_YCbCr_422_SP: case AHARDWAREBUFFER_FORMAT_YCrCb_420_SP: case AHARDWAREBUFFER_FORMAT_YCbCr_422_I: + case AHARDWAREBUFFER_FORMAT_YCbCr_P010: return true; default: @@ -633,6 +636,7 @@ bool AHardwareBuffer_formatIsYuv(uint32_t format) { case AHARDWAREBUFFER_FORMAT_YCbCr_422_SP: case AHARDWAREBUFFER_FORMAT_YCrCb_420_SP: case AHARDWAREBUFFER_FORMAT_YCbCr_422_I: + case AHARDWAREBUFFER_FORMAT_YCbCr_P010: return true; default: return false; diff --git a/libs/nativewindow/include/android/hardware_buffer.h b/libs/nativewindow/include/android/hardware_buffer.h index d93a84cd25..78c56d9ed5 100644 --- a/libs/nativewindow/include/android/hardware_buffer.h +++ b/libs/nativewindow/include/android/hardware_buffer.h @@ -158,6 +158,14 @@ enum AHardwareBuffer_Format { * cube-maps or multi-layered textures. */ AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_420 = 0x23, + + /** + * YUV P010 format. + * Must have an even width and height. Can be accessed in OpenGL + * shaders through an external sampler. Does not support mip-maps + * cube-maps or multi-layered textures. + */ + AHARDWAREBUFFER_FORMAT_YCbCr_P010 = 0x36, }; /** diff --git a/opengl/OWNERS b/opengl/OWNERS index a9bd4bb2e2..a47fb9a573 100644 --- a/opengl/OWNERS +++ b/opengl/OWNERS @@ -1,6 +1,12 @@ +abdolrashidi@google.com +cclao@google.com chrisforbes@google.com cnorthrop@google.com ianelliott@google.com jessehall@google.com +lfy@google.com lpy@google.com timvp@google.com +romanl@google.com +vantablack@google.com +yuxinhu@google.com diff --git a/services/gpuservice/bpfprogs/Android.bp b/services/gpuservice/bpfprogs/Android.bp index 9842ed7c3c..076affda5a 100644 --- a/services/gpuservice/bpfprogs/Android.bp +++ b/services/gpuservice/bpfprogs/Android.bp @@ -24,6 +24,7 @@ package { bpf { name: "gpu_mem.o", srcs: ["gpu_mem.c"], + btf: true, cflags: [ "-Wall", "-Werror", diff --git a/services/gpuservice/gpumem/Android.bp b/services/gpuservice/gpumem/Android.bp index 24087ac9e6..d0ea856fb5 100644 --- a/services/gpuservice/gpumem/Android.bp +++ b/services/gpuservice/gpumem/Android.bp @@ -26,19 +26,17 @@ cc_library_shared { srcs: [ "GpuMem.cpp", ], + header_libs: ["bpf_headers"], shared_libs: [ "libbase", "libbpf_bcc", - "libbpf_android", "libcutils", "liblog", "libutils", ], export_include_dirs: ["include"], - export_shared_lib_headers: [ - "libbase", - "libbpf_android", - ], + export_header_lib_headers: ["bpf_headers"], + export_shared_lib_headers: ["libbase"], cppflags: [ "-Wall", "-Werror", diff --git a/services/gpuservice/tests/unittests/Android.bp b/services/gpuservice/tests/unittests/Android.bp index 5b69f960a2..4fb0d2e734 100644 --- a/services/gpuservice/tests/unittests/Android.bp +++ b/services/gpuservice/tests/unittests/Android.bp @@ -32,10 +32,10 @@ cc_test { "GpuMemTracerTest.cpp", "GpuStatsTest.cpp", ], + header_libs: ["bpf_headers"], shared_libs: [ "libbase", "libbpf_bcc", - "libbpf_android", "libcutils", "libgfxstats", "libgpumem", diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index cf774fd9b8..0c5d61b8d5 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -624,8 +624,10 @@ void CreateInfoWrapper::FilterExtension(const char* name) { switch (ext_bit) { case ProcHook::KHR_android_surface: case ProcHook::KHR_surface: + case ProcHook::KHR_surface_protected_capabilities: case ProcHook::EXT_swapchain_colorspace: case ProcHook::KHR_get_surface_capabilities2: + case ProcHook::GOOGLE_surfaceless_query: hook_extensions_.set(ext_bit); // return now as these extensions do not require HAL support return; @@ -701,8 +703,10 @@ void CreateInfoWrapper::FilterExtension(const char* name) { case ProcHook::KHR_external_fence_capabilities: case ProcHook::KHR_get_surface_capabilities2: case ProcHook::KHR_surface: + case ProcHook::KHR_surface_protected_capabilities: case ProcHook::EXT_debug_report: case ProcHook::EXT_swapchain_colorspace: + case ProcHook::GOOGLE_surfaceless_query: case ProcHook::ANDROID_native_buffer: case ProcHook::EXTENSION_CORE_1_0: case ProcHook::EXTENSION_CORE_1_1: @@ -913,6 +917,9 @@ VkResult EnumerateInstanceExtensionProperties( loader_extensions.push_back({ VK_KHR_SURFACE_EXTENSION_NAME, VK_KHR_SURFACE_SPEC_VERSION}); + loader_extensions.push_back( + {VK_KHR_SURFACE_PROTECTED_CAPABILITIES_EXTENSION_NAME, + VK_KHR_SURFACE_PROTECTED_CAPABILITIES_SPEC_VERSION}); loader_extensions.push_back({ VK_KHR_ANDROID_SURFACE_EXTENSION_NAME, VK_KHR_ANDROID_SURFACE_SPEC_VERSION}); @@ -922,6 +929,8 @@ VkResult EnumerateInstanceExtensionProperties( loader_extensions.push_back({ VK_KHR_GET_SURFACE_CAPABILITIES_2_EXTENSION_NAME, VK_KHR_GET_SURFACE_CAPABILITIES_2_SPEC_VERSION}); + loader_extensions.push_back({VK_GOOGLE_SURFACELESS_QUERY_EXTENSION_NAME, + VK_GOOGLE_SURFACELESS_QUERY_SPEC_VERSION}); static const VkExtensionProperties loader_debug_report_extension = { VK_EXT_DEBUG_REPORT_EXTENSION_NAME, VK_EXT_DEBUG_REPORT_SPEC_VERSION, diff --git a/vulkan/libvulkan/driver_gen.cpp b/vulkan/libvulkan/driver_gen.cpp index 5f37a50a03..b436db1de7 100644 --- a/vulkan/libvulkan/driver_gen.cpp +++ b/vulkan/libvulkan/driver_gen.cpp @@ -565,11 +565,13 @@ ProcHook::Extension GetProcHookExtension(const char* name) { if (strcmp(name, "VK_EXT_hdr_metadata") == 0) return ProcHook::EXT_hdr_metadata; if (strcmp(name, "VK_EXT_swapchain_colorspace") == 0) return ProcHook::EXT_swapchain_colorspace; if (strcmp(name, "VK_GOOGLE_display_timing") == 0) return ProcHook::GOOGLE_display_timing; + if (strcmp(name, "VK_GOOGLE_surfaceless_query") == 0) return ProcHook::GOOGLE_surfaceless_query; if (strcmp(name, "VK_KHR_android_surface") == 0) return ProcHook::KHR_android_surface; if (strcmp(name, "VK_KHR_get_surface_capabilities2") == 0) return ProcHook::KHR_get_surface_capabilities2; if (strcmp(name, "VK_KHR_incremental_present") == 0) return ProcHook::KHR_incremental_present; if (strcmp(name, "VK_KHR_shared_presentable_image") == 0) return ProcHook::KHR_shared_presentable_image; if (strcmp(name, "VK_KHR_surface") == 0) return ProcHook::KHR_surface; + if (strcmp(name, "VK_KHR_surface_protected_capabilities") == 0) return ProcHook::KHR_surface_protected_capabilities; if (strcmp(name, "VK_KHR_swapchain") == 0) return ProcHook::KHR_swapchain; if (strcmp(name, "VK_ANDROID_external_memory_android_hardware_buffer") == 0) return ProcHook::ANDROID_external_memory_android_hardware_buffer; if (strcmp(name, "VK_KHR_bind_memory2") == 0) return ProcHook::KHR_bind_memory2; diff --git a/vulkan/libvulkan/driver_gen.h b/vulkan/libvulkan/driver_gen.h index 047e774004..688630c0a5 100644 --- a/vulkan/libvulkan/driver_gen.h +++ b/vulkan/libvulkan/driver_gen.h @@ -41,11 +41,13 @@ struct ProcHook { EXT_hdr_metadata, EXT_swapchain_colorspace, GOOGLE_display_timing, + GOOGLE_surfaceless_query, KHR_android_surface, KHR_get_surface_capabilities2, KHR_incremental_present, KHR_shared_presentable_image, KHR_surface, + KHR_surface_protected_capabilities, KHR_swapchain, ANDROID_external_memory_android_hardware_buffer, KHR_bind_memory2, diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp index 6191063894..ec673123b0 100644 --- a/vulkan/libvulkan/swapchain.cpp +++ b/vulkan/libvulkan/swapchain.cpp @@ -614,42 +614,65 @@ VkResult GetPhysicalDeviceSurfaceSupportKHR(VkPhysicalDevice /*pdev*/, VKAPI_ATTR VkResult GetPhysicalDeviceSurfaceCapabilitiesKHR( - VkPhysicalDevice /*pdev*/, + VkPhysicalDevice pdev, VkSurfaceKHR surface, VkSurfaceCapabilitiesKHR* capabilities) { ATRACE_CALL(); int err; - ANativeWindow* window = SurfaceFromHandle(surface)->window.get(); - int width, height; - err = window->query(window, NATIVE_WINDOW_DEFAULT_WIDTH, &width); - if (err != android::OK) { - ALOGE("NATIVE_WINDOW_DEFAULT_WIDTH query failed: %s (%d)", - strerror(-err), err); - return VK_ERROR_SURFACE_LOST_KHR; - } - err = window->query(window, NATIVE_WINDOW_DEFAULT_HEIGHT, &height); - if (err != android::OK) { - ALOGE("NATIVE_WINDOW_DEFAULT_WIDTH query failed: %s (%d)", - strerror(-err), err); - return VK_ERROR_SURFACE_LOST_KHR; - } - int transform_hint; - err = window->query(window, NATIVE_WINDOW_TRANSFORM_HINT, &transform_hint); - if (err != android::OK) { - ALOGE("NATIVE_WINDOW_TRANSFORM_HINT query failed: %s (%d)", - strerror(-err), err); - return VK_ERROR_SURFACE_LOST_KHR; - } - int max_buffer_count; - err = window->query(window, NATIVE_WINDOW_MAX_BUFFER_COUNT, &max_buffer_count); - if (err != android::OK) { - ALOGE("NATIVE_WINDOW_MAX_BUFFER_COUNT query failed: %s (%d)", - strerror(-err), err); - return VK_ERROR_SURFACE_LOST_KHR; + if (surface == VK_NULL_HANDLE) { + const InstanceData& instance_data = GetData(pdev); + ProcHook::Extension surfaceless = ProcHook::GOOGLE_surfaceless_query; + bool surfaceless_enabled = + instance_data.hook_extensions.test(surfaceless); + if (!surfaceless_enabled) { + // It is an error to pass a surface==VK_NULL_HANDLE unless the + // VK_GOOGLE_surfaceless_query extension is enabled + return VK_ERROR_SURFACE_LOST_KHR; + } + // Support for VK_GOOGLE_surfaceless_query. The primary purpose of this + // extension for this function is for + // VkSurfaceProtectedCapabilitiesKHR::supportsProtected. The following + // four values cannot be known without a surface. Default values will + // be supplied anyway, but cannot be relied upon. + width = 1000; + height = 1000; + transform_hint = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR; + max_buffer_count = 10; + } else { + ANativeWindow* window = SurfaceFromHandle(surface)->window.get(); + + err = window->query(window, NATIVE_WINDOW_DEFAULT_WIDTH, &width); + if (err != android::OK) { + ALOGE("NATIVE_WINDOW_DEFAULT_WIDTH query failed: %s (%d)", + strerror(-err), err); + return VK_ERROR_SURFACE_LOST_KHR; + } + err = window->query(window, NATIVE_WINDOW_DEFAULT_HEIGHT, &height); + if (err != android::OK) { + ALOGE("NATIVE_WINDOW_DEFAULT_WIDTH query failed: %s (%d)", + strerror(-err), err); + return VK_ERROR_SURFACE_LOST_KHR; + } + + err = window->query(window, NATIVE_WINDOW_TRANSFORM_HINT, + &transform_hint); + if (err != android::OK) { + ALOGE("NATIVE_WINDOW_TRANSFORM_HINT query failed: %s (%d)", + strerror(-err), err); + return VK_ERROR_SURFACE_LOST_KHR; + } + + err = window->query(window, NATIVE_WINDOW_MAX_BUFFER_COUNT, + &max_buffer_count); + if (err != android::OK) { + ALOGE("NATIVE_WINDOW_MAX_BUFFER_COUNT query failed: %s (%d)", + strerror(-err), err); + return VK_ERROR_SURFACE_LOST_KHR; + } } capabilities->minImageCount = std::min(max_buffer_count, 3); capabilities->maxImageCount = static_cast<uint32_t>(max_buffer_count); @@ -690,23 +713,43 @@ VkResult GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice pdev, const InstanceData& instance_data = GetData(pdev); bool wide_color_support = false; - Surface& surface = *SurfaceFromHandle(surface_handle); - int err = native_window_get_wide_color_support(surface.window.get(), - &wide_color_support); - if (err) { - return VK_ERROR_SURFACE_LOST_KHR; - } - ALOGV("wide_color_support is: %d", wide_color_support); - wide_color_support = - wide_color_support && + uint64_t consumer_usage = 0; + bool swapchain_ext = instance_data.hook_extensions.test(ProcHook::EXT_swapchain_colorspace); + if (surface_handle == VK_NULL_HANDLE) { + ProcHook::Extension surfaceless = ProcHook::GOOGLE_surfaceless_query; + bool surfaceless_enabled = + instance_data.hook_extensions.test(surfaceless); + if (!surfaceless_enabled) { + return VK_ERROR_SURFACE_LOST_KHR; + } + // Support for VK_GOOGLE_surfaceless_query. The EGL loader + // unconditionally supports wide color formats, even if they will cause + // a SurfaceFlinger fallback. Based on that, wide_color_support will be + // set to true in this case. + wide_color_support = true; + + // TODO(b/203826952): research proper value; temporarily use the + // values seen on Pixel + consumer_usage = AHARDWAREBUFFER_USAGE_COMPOSER_OVERLAY; + } else { + Surface& surface = *SurfaceFromHandle(surface_handle); + int err = native_window_get_wide_color_support(surface.window.get(), + &wide_color_support); + if (err) { + return VK_ERROR_SURFACE_LOST_KHR; + } + ALOGV("wide_color_support is: %d", wide_color_support); + + consumer_usage = surface.consumer_usage; + } + wide_color_support = wide_color_support && swapchain_ext; AHardwareBuffer_Desc desc = {}; desc.width = 1; desc.height = 1; desc.layers = 1; - desc.usage = surface.consumer_usage | - AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE | + desc.usage = consumer_usage | AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE | AHARDWAREBUFFER_USAGE_GPU_FRAMEBUFFER; // We must support R8G8B8A8 @@ -721,6 +764,10 @@ VkResult GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice pdev, VK_FORMAT_R8G8B8A8_SRGB, VK_COLOR_SPACE_DISPLAY_P3_NONLINEAR_EXT}); } + // NOTE: Any new formats that are added must be coordinated across different + // Android users. This includes the ANGLE team (a layered implementation of + // OpenGL-ES). + desc.format = AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM; if (AHardwareBuffer_isSupported(&desc)) { all_formats.emplace_back(VkSurfaceFormatKHR{ @@ -753,6 +800,10 @@ VkResult GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice pdev, } } + // NOTE: Any new formats that are added must be coordinated across different + // Android users. This includes the ANGLE team (a layered implementation of + // OpenGL-ES). + VkResult result = VK_SUCCESS; if (formats) { uint32_t transfer_count = all_formats.size(); @@ -797,6 +848,12 @@ VkResult GetPhysicalDeviceSurfaceCapabilities2KHR( .supportedUsageFlags; } break; + case VK_STRUCTURE_TYPE_SURFACE_PROTECTED_CAPABILITIES_KHR: { + VkSurfaceProtectedCapabilitiesKHR* protected_caps = + reinterpret_cast<VkSurfaceProtectedCapabilitiesKHR*>(caps); + protected_caps->supportsProtected = VK_TRUE; + } break; + default: // Ignore all other extension structs break; @@ -848,31 +905,51 @@ VkResult GetPhysicalDeviceSurfacePresentModesKHR(VkPhysicalDevice pdev, int err; int query_value; - ANativeWindow* window = SurfaceFromHandle(surface)->window.get(); + std::vector<VkPresentModeKHR> present_modes; + if (surface == VK_NULL_HANDLE) { + const InstanceData& instance_data = GetData(pdev); + ProcHook::Extension surfaceless = ProcHook::GOOGLE_surfaceless_query; + bool surfaceless_enabled = + instance_data.hook_extensions.test(surfaceless); + if (!surfaceless_enabled) { + return VK_ERROR_SURFACE_LOST_KHR; + } + // Support for VK_GOOGLE_surfaceless_query. The primary purpose of this + // extension for this function is for + // VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR and + // VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR. We technically cannot + // know if VK_PRESENT_MODE_SHARED_MAILBOX_KHR is supported without a + // surface, and that cannot be relied upon. + present_modes.push_back(VK_PRESENT_MODE_MAILBOX_KHR); + present_modes.push_back(VK_PRESENT_MODE_FIFO_KHR); + } else { + ANativeWindow* window = SurfaceFromHandle(surface)->window.get(); - err = window->query(window, NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS, - &query_value); - if (err != android::OK || query_value < 0) { - ALOGE( - "NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS query failed: %s (%d) " - "value=%d", - strerror(-err), err, query_value); - return VK_ERROR_SURFACE_LOST_KHR; - } - uint32_t min_undequeued_buffers = static_cast<uint32_t>(query_value); + err = window->query(window, NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS, + &query_value); + if (err != android::OK || query_value < 0) { + ALOGE( + "NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS query failed: %s (%d) " + "value=%d", + strerror(-err), err, query_value); + return VK_ERROR_SURFACE_LOST_KHR; + } + uint32_t min_undequeued_buffers = static_cast<uint32_t>(query_value); + + err = + window->query(window, NATIVE_WINDOW_MAX_BUFFER_COUNT, &query_value); + if (err != android::OK || query_value < 0) { + ALOGE( + "NATIVE_WINDOW_MAX_BUFFER_COUNT query failed: %s (%d) value=%d", + strerror(-err), err, query_value); + return VK_ERROR_SURFACE_LOST_KHR; + } + uint32_t max_buffer_count = static_cast<uint32_t>(query_value); - err = window->query(window, NATIVE_WINDOW_MAX_BUFFER_COUNT, &query_value); - if (err != android::OK || query_value < 0) { - ALOGE("NATIVE_WINDOW_MAX_BUFFER_COUNT query failed: %s (%d) value=%d", - strerror(-err), err, query_value); - return VK_ERROR_SURFACE_LOST_KHR; + if (min_undequeued_buffers + 1 < max_buffer_count) + present_modes.push_back(VK_PRESENT_MODE_MAILBOX_KHR); + present_modes.push_back(VK_PRESENT_MODE_FIFO_KHR); } - uint32_t max_buffer_count = static_cast<uint32_t>(query_value); - - std::vector<VkPresentModeKHR> present_modes; - if (min_undequeued_buffers + 1 < max_buffer_count) - present_modes.push_back(VK_PRESENT_MODE_MAILBOX_KHR); - present_modes.push_back(VK_PRESENT_MODE_FIFO_KHR); VkPhysicalDevicePresentationPropertiesANDROID present_properties; QueryPresentationProperties(pdev, &present_properties); diff --git a/vulkan/scripts/driver_generator.py b/vulkan/scripts/driver_generator.py index 6a73023193..af56764a21 100644 --- a/vulkan/scripts/driver_generator.py +++ b/vulkan/scripts/driver_generator.py @@ -27,11 +27,13 @@ _INTERCEPTED_EXTENSIONS = [ 'VK_EXT_hdr_metadata', 'VK_EXT_swapchain_colorspace', 'VK_GOOGLE_display_timing', + 'VK_GOOGLE_surfaceless_query', 'VK_KHR_android_surface', 'VK_KHR_get_surface_capabilities2', 'VK_KHR_incremental_present', 'VK_KHR_shared_presentable_image', 'VK_KHR_surface', + 'VK_KHR_surface_protected_capabilities', 'VK_KHR_swapchain', ] |