Merge "Add scroggo to SF owners"
diff --git a/cmds/atrace/atrace.cpp b/cmds/atrace/atrace.cpp
index 6459443..7aee795 100644
--- a/cmds/atrace/atrace.cpp
+++ b/cmds/atrace/atrace.cpp
@@ -245,7 +245,7 @@
{ 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 c2c8a72..4d3a67b 100644
--- a/cmds/dumpstate/DumpPool.cpp
+++ b/cmds/dumpstate/DumpPool.cpp
@@ -33,6 +33,20 @@
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() {
- 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 @@
}
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;
+void DumpPool::start(int thread_counts) {
+ assert(thread_counts > 0);
+ assert(threads_.empty());
+ if (thread_counts > MAX_THREAD_COUNT) {
+ thread_counts = MAX_THREAD_COUNT;
}
- Future future = iterator->second;
- futures_map_.erase(iterator);
-
- 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));
+ 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 0c3c2cc..9fb0fcc 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 @@
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 @@
* }
* ...
* 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 @@
* 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 @@
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 @@
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 @@
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 c833d0e..aa42541 100644
--- a/cmds/dumpstate/DumpstateUtil.cpp
+++ b/cmds/dumpstate/DumpstateUtil.cpp
@@ -48,11 +48,26 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
/* 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 a951c4f..0e9ce89 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -120,6 +120,7 @@
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 @@
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 @@
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 @@
// (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 @@
// 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 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
} 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 @@
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 @@
// 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 @@
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 @@
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::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 d0acb31..ee6b1ae 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -157,12 +157,6 @@
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 @@
// 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 42beb2b..70b4e5c 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -1720,14 +1720,13 @@
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 @@
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 @@
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 00babc3..c9f680b 100644
--- a/cmds/installd/Android.bp
+++ b/cmds/installd/Android.bp
@@ -51,6 +51,7 @@
],
static_libs: [
"libasync_safe",
+ "libext2_uuid",
],
export_shared_lib_headers: [
"libbinder",
@@ -262,6 +263,7 @@
"libasync_safe",
"libdiskusage",
"libotapreoptparameters",
+ "libext2_uuid",
],
shared_libs: [
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index c3256fc..4e3aae4 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -103,11 +103,6 @@
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 @@
} \
}
-#define ASSERT_PAGE_SIZE_4K() { \
- if (getpagesize() != kVerityPageSize) { \
- return error("FSVerity only supports 4K pages"); \
- } \
-}
-
#ifdef GRANULAR_LOCKS
/**
@@ -698,9 +687,6 @@
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 @@
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 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 @@
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 @@
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 @@
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 @@
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 09581bb..96783c3 100644
--- a/cmds/installd/InstalldNativeService.h
+++ b/cmds/installd/InstalldNativeService.h
@@ -164,10 +164,6 @@
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 @@
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 637a9f2..f4fd9a9 100644
--- a/cmds/installd/binder/android/os/IInstalld.aidl
+++ b/cmds/installd/binder/android/os/IInstalld.aidl
@@ -97,9 +97,6 @@
@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 @@
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 c796da6..9647865 100644
--- a/cmds/installd/dexopt.cpp
+++ b/cmds/installd/dexopt.cpp
@@ -74,10 +74,19 @@
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 @@
}
}
+// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
_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 @@
}
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 @@
}
/* 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 @@
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 @@
_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 @@
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 @@
}
/* 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 @@
}
/* 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 @@
}
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 @@
}
/* 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 51f7716..e390bab 100644
--- a/cmds/installd/tests/Android.bp
+++ b/cmds/installd/tests/Android.bp
@@ -25,6 +25,7 @@
static_libs: [
"libasync_safe",
"libdiskusage",
+ "libext2_uuid",
"libinstalld",
"liblog",
],
@@ -53,6 +54,7 @@
static_libs: [
"libasync_safe",
"libdiskusage",
+ "libext2_uuid",
"libinstalld",
"libziparchive",
"liblog",
@@ -99,6 +101,7 @@
static_libs: [
"libasync_safe",
"libdiskusage",
+ "libext2_uuid",
"libinstalld",
"libziparchive",
"liblog",
@@ -144,6 +147,7 @@
static_libs: [
"libasync_safe",
"libdiskusage",
+ "libext2_uuid",
"libinstalld",
"liblog",
"liblogwrap",
@@ -204,6 +208,7 @@
"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 a937436..aeba451 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -50,6 +50,8 @@
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 @@
_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 @@
_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 b831515..9702d42 100644
--- a/cmds/installd/tests/installd_service_test.cpp
+++ b/cmds/installd/tests/installd_service_test.cpp
@@ -75,6 +75,7 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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 ed87b67..514b881 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 @@
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 @@
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 0f8a732..f04ee33 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 @@
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 @@
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 @@
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 @@
}
}
+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 549fc6c..3973894 100644
--- a/cmds/installd/utils.h
+++ b/cmds/installd/utils.h
@@ -120,6 +120,11 @@
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 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 @@
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 60d6492..8c000a1 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 @@
_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 f84f639..6a138e3 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -200,7 +200,6 @@
}
flat_binder_object obj;
- obj.flags = FLAT_BINDER_FLAG_ACCEPTS_FDS;
int schedBits = 0;
if (!IPCThreadState::self()->backgroundSchedulingDisabled()) {
@@ -221,6 +220,7 @@
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 @@
// 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 @@
}
} 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 406fee0..1504715 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 269b086..baa817c 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 @@
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 1d7de98..dbfb1c2 100644
--- a/libs/binder/rust/src/lib.rs
+++ b/libs/binder/rust/src/lib.rs
@@ -114,8 +114,8 @@
};
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 12bfde7..e3e4730 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -28,9 +28,10 @@
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 @@
}
}
+/// 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 50daf1c..c9d6af0 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -484,6 +484,21 @@
}
#[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 @@
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 700940a..b1e17b7 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -57,6 +57,7 @@
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 @@
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 @@
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 @@
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 @@
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 1446802..38bde3a 100644
--- a/libs/binder/tests/parcel_fuzzer/Android.bp
+++ b/libs/binder/tests/parcel_fuzzer/Android.bp
@@ -26,13 +26,9 @@
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 4f63194..79cc15f 100644
--- a/libs/cputimeinstate/Android.bp
+++ b/libs/cputimeinstate/Android.bp
@@ -13,12 +13,13 @@
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 @@
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 93879d8..72ff978 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 e2f072a..81a529d 100644
--- a/libs/gralloc/types/Gralloc4.cpp
+++ b/libs/gralloc/types/Gralloc4.cpp
@@ -196,6 +196,35 @@
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 @@
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;
}
- return encodeMetadata(metadataType, *input, output, encodeHelper);
+
+ 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;
+ }
+
+ 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 @@
if (!output) {
return BAD_VALUE;
}
- if (input.size() <= 0) {
- output->reset();
- return NO_ERROR;
+
+ InputHidlVec inputHidlVec{&input};
+
+ status_t err = validateMetadataType(&inputHidlVec, metadataType);
+ if (err) {
+ return err;
}
- T tmp;
- status_t err = decodeMetadata(metadataType, input, &tmp, decodeHelper);
- if (!err) {
+
+ uint32_t present = 0;
+ err = decodeInteger<uint32_t>(&inputHidlVec, &present);
+ if (err) {
+ return err;
+ }
+
+ if (present) {
+ T tmp;
+ err = decodeHelper(&inputHidlVec, &tmp);
+ if (err) {
+ return err;
+ }
+
*output = tmp;
}
- 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) {
+ err = inputHidlVec.hasRemainingData();
+ if (err) {
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));
+ 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 e2f32e3..ef7602f 100644
--- a/libs/nativewindow/AHardwareBuffer.cpp
+++ b/libs/nativewindow/AHardwareBuffer.cpp
@@ -588,6 +588,8 @@
"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 @@
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 @@
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 d93a84c..78c56d9 100644
--- a/libs/nativewindow/include/android/hardware_buffer.h
+++ b/libs/nativewindow/include/android/hardware_buffer.h
@@ -158,6 +158,14 @@
* 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 a9bd4bb..a47fb9a 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 9842ed7..076affd 100644
--- a/services/gpuservice/bpfprogs/Android.bp
+++ b/services/gpuservice/bpfprogs/Android.bp
@@ -24,6 +24,7 @@
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 24087ac..d0ea856 100644
--- a/services/gpuservice/gpumem/Android.bp
+++ b/services/gpuservice/gpumem/Android.bp
@@ -26,19 +26,17 @@
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 5b69f96..4fb0d2e 100644
--- a/services/gpuservice/tests/unittests/Android.bp
+++ b/services/gpuservice/tests/unittests/Android.bp
@@ -32,10 +32,10 @@
"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 cf774fd..0c5d61b 100644
--- a/vulkan/libvulkan/driver.cpp
+++ b/vulkan/libvulkan/driver.cpp
@@ -624,8 +624,10 @@
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 @@
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 @@
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 @@
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 5f37a50..b436db1 100644
--- a/vulkan/libvulkan/driver_gen.cpp
+++ b/vulkan/libvulkan/driver_gen.cpp
@@ -565,11 +565,13 @@
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 047e774..688630c 100644
--- a/vulkan/libvulkan/driver_gen.h
+++ b/vulkan/libvulkan/driver_gen.h
@@ -41,11 +41,13 @@
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 6191063..ec67312 100644
--- a/vulkan/libvulkan/swapchain.cpp
+++ b/vulkan/libvulkan/swapchain.cpp
@@ -614,42 +614,65 @@
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 @@
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 @@
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 @@
}
}
+ // 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 @@
.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 @@
int err;
int query_value;
- 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_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);
-
std::vector<VkPresentModeKHR> present_modes;
- if (min_undequeued_buffers + 1 < max_buffer_count)
+ 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);
+ 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_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);
+
+ 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 6a73023..af56764 100644
--- a/vulkan/scripts/driver_generator.py
+++ b/vulkan/scripts/driver_generator.py
@@ -27,11 +27,13 @@
'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',
]