Merge "Just request TEXTURABLE"
diff --git a/cmds/dumpstate/Android.bp b/cmds/dumpstate/Android.bp
index 1f055f3..80d14ac 100644
--- a/cmds/dumpstate/Android.bp
+++ b/cmds/dumpstate/Android.bp
@@ -106,6 +106,7 @@
defaults: ["dumpstate_defaults"],
srcs: [
"DumpPool.cpp",
+ "TaskQueue.cpp",
"dumpstate.cpp",
"main.cpp",
],
@@ -134,6 +135,7 @@
defaults: ["dumpstate_defaults"],
srcs: [
"DumpPool.cpp",
+ "TaskQueue.cpp",
"dumpstate.cpp",
"tests/dumpstate_test.cpp",
],
@@ -151,6 +153,7 @@
defaults: ["dumpstate_defaults"],
srcs: [
"DumpPool.cpp",
+ "TaskQueue.cpp",
"dumpstate.cpp",
"tests/dumpstate_smoke_test.cpp",
],
diff --git a/cmds/dumpstate/TaskQueue.cpp b/cmds/dumpstate/TaskQueue.cpp
new file mode 100644
index 0000000..8550aec
--- /dev/null
+++ b/cmds/dumpstate/TaskQueue.cpp
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "TaskQueue.h"
+
+namespace android {
+namespace os {
+namespace dumpstate {
+
+TaskQueue::~TaskQueue() {
+ run(/* do_cancel = */true);
+}
+
+void TaskQueue::run(bool do_cancel) {
+ std::unique_lock lock(lock_);
+ while (!tasks_.empty()) {
+ auto task = tasks_.front();
+ tasks_.pop();
+ lock.unlock();
+ std::invoke(task, do_cancel);
+ lock.lock();
+ }
+}
+
+} // namespace dumpstate
+} // namespace os
+} // namespace android
diff --git a/cmds/dumpstate/TaskQueue.h b/cmds/dumpstate/TaskQueue.h
new file mode 100644
index 0000000..b7e72f1
--- /dev/null
+++ b/cmds/dumpstate/TaskQueue.h
@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef FRAMEWORK_NATIVE_CMD_TASKQUEUE_H_
+#define FRAMEWORK_NATIVE_CMD_TASKQUEUE_H_
+
+#include <mutex>
+#include <queue>
+
+#include <android-base/macros.h>
+
+namespace android {
+namespace os {
+namespace dumpstate {
+
+/*
+ * A task queue for dumpstate to collect tasks such as adding file to the zip
+ * which are needed to run in a single thread. The task is a callable function
+ * included a cancel task boolean parameter. The TaskQueue could
+ * cancel the task in the destructor if the task has never been called.
+ */
+class TaskQueue {
+ public:
+ TaskQueue() = default;
+ ~TaskQueue();
+
+ /*
+ * Adds a task into the queue.
+ *
+ * |f| Callable function to execute the task. The function must include a
+ * boolean parameter for TaskQueue to notify whether the task is
+ * cancelled or not.
+ * |args| A list of arguments.
+ */
+ template<class F, class... Args> void add(F&& f, Args&&... args) {
+ auto func = std::bind(std::forward<F>(f), std::forward<Args>(args)...);
+ std::unique_lock lock(lock_);
+ tasks_.emplace([=](bool cancelled) {
+ std::invoke(func, cancelled);
+ });
+ }
+
+ /*
+ * Invokes all tasks in the task queue.
+ *
+ * |do_cancel| true to cancel all tasks in the queue.
+ */
+ void run(bool do_cancel);
+
+ private:
+ using Task = std::function<void(bool)>;
+
+ std::mutex lock_;
+ std::queue<Task> tasks_;
+
+ DISALLOW_COPY_AND_ASSIGN(TaskQueue);
+};
+
+} // namespace dumpstate
+} // namespace os
+} // namespace android
+
+#endif //FRAMEWORK_NATIVE_CMD_TASKQUEUE_H_
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 7d195b4..d108e00 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -116,6 +116,7 @@
using android::os::dumpstate::DumpFileToFd;
using android::os::dumpstate::DumpPool;
using android::os::dumpstate::PropertiesHelper;
+using android::os::dumpstate::TaskQueue;
// Keep in sync with
// frameworks/base/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -128,8 +129,8 @@
static Dumpstate& ds = Dumpstate::GetInstance();
static int RunCommand(const std::string& title, const std::vector<std::string>& full_command,
const CommandOptions& options = CommandOptions::DEFAULT,
- bool verbose_duration = false) {
- return ds.RunCommand(title, full_command, options, verbose_duration);
+ bool verbose_duration = false, int out_fd = STDOUT_FILENO) {
+ return ds.RunCommand(title, full_command, options, verbose_duration, out_fd);
}
// Reasonable value for max stats.
@@ -212,11 +213,21 @@
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) \
+ RETURN_IF_USER_DENIED_CONSENT(); \
+ pool_ptr->waitForTask(task_name); \
+ RETURN_IF_USER_DENIED_CONSENT();
+
static const char* WAKE_LOCK_NAME = "dumpstate_wakelock";
// Names of parallel tasks, they are used for the DumpPool to identify the dump
// task and the log title of the duration report.
static const std::string DUMP_TRACES_TASK = "DUMP TRACES";
+static const std::string DUMP_INCIDENT_REPORT_TASK = "INCIDENT REPORT";
+static const std::string DUMP_HALS_TASK = "DUMP HALS";
+static const std::string DUMP_BOARD_TASK = "dumpstate_board()";
+static const std::string DUMP_CHECKINS_TASK = "DUMP CHECKINS";
+static const std::string DUMP_APP_INFOS_TASK = "DUMP APP INFOS";
namespace android {
namespace os {
@@ -326,8 +337,12 @@
static void RunDumpsys(const std::string& title, const std::vector<std::string>& dumpsysArgs,
const CommandOptions& options = Dumpstate::DEFAULT_DUMPSYS,
- long dumpsysTimeoutMs = 0) {
- return ds.RunDumpsys(title, dumpsysArgs, options, dumpsysTimeoutMs);
+ long dumpsysTimeoutMs = 0, int out_fd = STDOUT_FILENO) {
+ return ds.RunDumpsys(title, dumpsysArgs, options, dumpsysTimeoutMs, out_fd);
+}
+static void RunDumpsys(const std::string& title, const std::vector<std::string>& dumpsysArgs,
+ int out_fd) {
+ return ds.RunDumpsys(title, dumpsysArgs, Dumpstate::DEFAULT_DUMPSYS, 0, out_fd);
}
static int DumpFile(const std::string& title, const std::string& path) {
return ds.DumpFile(title, path);
@@ -1014,7 +1029,6 @@
MYLOGD("Not dumping incident report because it's not a zipped bugreport\n");
return;
}
- DurationReporter duration_reporter("INCIDENT REPORT");
const std::string path = ds.bugreport_internal_dir_ + "/tmp_incident_report";
auto fd = android::base::unique_fd(TEMP_FAILURE_RETRY(open(path.c_str(),
O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
@@ -1029,9 +1043,11 @@
// Use a different name from "incident.proto"
// /proto/incident.proto is reserved for incident service dump
// i.e. metadata for debugging.
- ds.AddZipEntry(kProtoPath + "incident_report" + kProtoExt, path);
+ ds.EnqueueAddZipEntryAndCleanupIfNeeded(kProtoPath + "incident_report" + kProtoExt,
+ path);
+ } else {
+ unlink(path.c_str());
}
- unlink(path.c_str());
}
static void DumpVisibleWindowViews() {
@@ -1326,15 +1342,21 @@
/* timeout= */ 90s, /* service_timeout= */ 10s);
}
-static void DumpHals() {
+/*
+ * |out_fd| A fd to support the DumpPool to output results to a temporary file.
+ * Dumpstate can pick up later and output to the bugreport. Using STDOUT_FILENO
+ * if it's not running in the parallel task.
+ */
+static void DumpHals(int out_fd = STDOUT_FILENO) {
if (!ds.IsZipping()) {
RunCommand("HARDWARE HALS", {"lshal", "--all", "--types=all", "--debug"},
- CommandOptions::WithTimeout(10).AsRootIfAvailable().Build());
+ CommandOptions::WithTimeout(10).AsRootIfAvailable().Build(),
+ false, out_fd);
return;
}
- DurationReporter duration_reporter("DUMP HALS");
RunCommand("HARDWARE HALS", {"lshal", "--all", "--types=all"},
- CommandOptions::WithTimeout(10).AsRootIfAvailable().Build());
+ CommandOptions::WithTimeout(10).AsRootIfAvailable().Build(),
+ false, out_fd);
using android::hidl::manager::V1_0::IServiceManager;
using android::hardware::defaultServiceManager;
@@ -1356,6 +1378,7 @@
}, '_');
const std::string path = ds.bugreport_internal_dir_ + "/lshal_debug_" + cleanName;
+ bool empty = false;
{
auto fd = android::base::unique_fd(
TEMP_FAILURE_RETRY(open(path.c_str(),
@@ -1370,13 +1393,14 @@
{"lshal", "debug", "-E", interface},
CommandOptions::WithTimeout(2).AsRootIfAvailable().Build());
- bool empty = 0 == lseek(fd, 0, SEEK_END);
- if (!empty) {
- ds.AddZipEntry("lshal-debug/" + cleanName + ".txt", path);
- }
+ empty = 0 == lseek(fd, 0, SEEK_END);
}
-
- unlink(path.c_str());
+ if (!empty) {
+ ds.EnqueueAddZipEntryAndCleanupIfNeeded("lshal-debug/" + cleanName + ".txt",
+ path);
+ } else {
+ unlink(path.c_str());
+ }
}
});
@@ -1464,6 +1488,73 @@
printf("========================================================\n");
}
+/*
+ * |out_fd| A fd to support the DumpPool to output results to a temporary file.
+ * Dumpstate can pick up later and output to the bugreport. Using STDOUT_FILENO
+ * if it's not running in the parallel task.
+ */
+static void DumpCheckins(int out_fd = STDOUT_FILENO) {
+ dprintf(out_fd, "========================================================\n");
+ dprintf(out_fd, "== Checkins\n");
+ dprintf(out_fd, "========================================================\n");
+
+ RunDumpsys("CHECKIN BATTERYSTATS", {"batterystats", "-c"}, out_fd);
+ RunDumpsys("CHECKIN MEMINFO", {"meminfo", "--checkin"}, out_fd);
+ RunDumpsys("CHECKIN NETSTATS", {"netstats", "--checkin"}, out_fd);
+ RunDumpsys("CHECKIN PROCSTATS", {"procstats", "-c"}, out_fd);
+ RunDumpsys("CHECKIN USAGESTATS", {"usagestats", "-c"}, out_fd);
+ RunDumpsys("CHECKIN PACKAGE", {"package", "--checkin"}, out_fd);
+}
+
+/*
+ * Runs dumpsys on activity service to dump all application activities, services
+ * and providers in the device.
+ *
+ * |out_fd| A fd to support the DumpPool to output results to a temporary file.
+ * Dumpstate can pick up later and output to the bugreport. Using STDOUT_FILENO
+ * if it's not running in the parallel task.
+ */
+static void DumpAppInfos(int out_fd = STDOUT_FILENO) {
+ dprintf(out_fd, "========================================================\n");
+ dprintf(out_fd, "== Running Application Activities\n");
+ dprintf(out_fd, "========================================================\n");
+
+ // The following dumpsys internally collects output from running apps, so it can take a long
+ // time. So let's extend the timeout.
+
+ const CommandOptions DUMPSYS_COMPONENTS_OPTIONS = CommandOptions::WithTimeout(60).Build();
+
+ RunDumpsys("APP ACTIVITIES", {"activity", "-v", "all"}, DUMPSYS_COMPONENTS_OPTIONS, 0, out_fd);
+
+ dprintf(out_fd, "========================================================\n");
+ dprintf(out_fd, "== Running Application Services (platform)\n");
+ dprintf(out_fd, "========================================================\n");
+
+ RunDumpsys("APP SERVICES PLATFORM", {"activity", "service", "all-platform-non-critical"},
+ DUMPSYS_COMPONENTS_OPTIONS, 0, out_fd);
+
+ dprintf(out_fd, "========================================================\n");
+ dprintf(out_fd, "== Running Application Services (non-platform)\n");
+ dprintf(out_fd, "========================================================\n");
+
+ RunDumpsys("APP SERVICES NON-PLATFORM", {"activity", "service", "all-non-platform"},
+ DUMPSYS_COMPONENTS_OPTIONS, 0, out_fd);
+
+ dprintf(out_fd, "========================================================\n");
+ dprintf(out_fd, "== Running Application Providers (platform)\n");
+ dprintf(out_fd, "========================================================\n");
+
+ RunDumpsys("APP PROVIDERS PLATFORM", {"activity", "provider", "all-platform"},
+ DUMPSYS_COMPONENTS_OPTIONS, out_fd);
+
+ dprintf(out_fd, "========================================================\n");
+ dprintf(out_fd, "== Running Application Providers (non-platform)\n");
+ dprintf(out_fd, "========================================================\n");
+
+ RunDumpsys("APP PROVIDERS NON-PLATFORM", {"activity", "provider", "all-non-platform"},
+ DUMPSYS_COMPONENTS_OPTIONS, 0, out_fd);
+}
+
// Dumps various things. Returns early with status USER_CONSENT_DENIED if user denies consent
// via the consent they are shown. Ignores other errors that occur while running various
// commands. The consent checking is currently done around long running tasks, which happen to
@@ -1471,6 +1562,19 @@
static Dumpstate::RunStatus dumpstate() {
DurationReporter duration_reporter("DUMPSTATE");
+ // Enqueue slow functions into the thread pool, if the parallel run is enabled.
+ 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);
+ ds.dump_pool_->enqueueTaskWithFd(DUMP_APP_INFOS_TASK, &DumpAppInfos, _1);
+ }
+
// Dump various things. Note that anything that takes "long" (i.e. several seconds) should
// check intermittently (if it's intrerruptable like a foreach on pids) and/or should be wrapped
// in a consent check (via RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK).
@@ -1502,7 +1606,11 @@
RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK(RunCommand, "LIBRANK", {"librank"},
CommandOptions::AS_ROOT);
- DumpHals();
+ if (ds.dump_pool_) {
+ WAIT_TASK_WITH_CONSENT_CHECK(DUMP_HALS_TASK, ds.dump_pool_);
+ } else {
+ RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_HALS_TASK, DumpHals);
+ }
RunCommand("PRINTENV", {"printenv"});
RunCommand("NETSTAT", {"netstat", "-nW"});
@@ -1583,7 +1691,11 @@
ds.AddDir(SNAPSHOTCTL_LOG_DIR, false);
- RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK(ds.DumpstateBoard);
+ if (ds.dump_pool_) {
+ WAIT_TASK_WITH_CONSENT_CHECK(DUMP_BOARD_TASK, ds.dump_pool_);
+ } else {
+ RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_BOARD_TASK, ds.DumpstateBoard);
+ }
/* Migrate the ril_dumpstate to a device specific dumpstate? */
int rilDumpstateTimeout = android::base::GetIntProperty("ril.dumpstate.timeout", 0);
@@ -1605,57 +1717,17 @@
RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK(RunDumpsysNormal);
- printf("========================================================\n");
- printf("== Checkins\n");
- printf("========================================================\n");
+ if (ds.dump_pool_) {
+ WAIT_TASK_WITH_CONSENT_CHECK(DUMP_CHECKINS_TASK, ds.dump_pool_);
+ } else {
+ RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_CHECKINS_TASK, DumpCheckins);
+ }
- RunDumpsys("CHECKIN BATTERYSTATS", {"batterystats", "-c"});
-
- RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK(RunDumpsys, "CHECKIN MEMINFO", {"meminfo", "--checkin"});
-
- RunDumpsys("CHECKIN NETSTATS", {"netstats", "--checkin"});
- RunDumpsys("CHECKIN PROCSTATS", {"procstats", "-c"});
- RunDumpsys("CHECKIN USAGESTATS", {"usagestats", "-c"});
- RunDumpsys("CHECKIN PACKAGE", {"package", "--checkin"});
-
- printf("========================================================\n");
- printf("== Running Application Activities\n");
- printf("========================================================\n");
-
- // The following dumpsys internally collects output from running apps, so it can take a long
- // time. So let's extend the timeout.
-
- const CommandOptions DUMPSYS_COMPONENTS_OPTIONS = CommandOptions::WithTimeout(60).Build();
-
- RunDumpsys("APP ACTIVITIES", {"activity", "-v", "all"}, DUMPSYS_COMPONENTS_OPTIONS);
-
- printf("========================================================\n");
- printf("== Running Application Services (platform)\n");
- printf("========================================================\n");
-
- RunDumpsys("APP SERVICES PLATFORM", {"activity", "service", "all-platform-non-critical"},
- DUMPSYS_COMPONENTS_OPTIONS);
-
- printf("========================================================\n");
- printf("== Running Application Services (non-platform)\n");
- printf("========================================================\n");
-
- RunDumpsys("APP SERVICES NON-PLATFORM", {"activity", "service", "all-non-platform"},
- DUMPSYS_COMPONENTS_OPTIONS);
-
- printf("========================================================\n");
- printf("== Running Application Providers (platform)\n");
- printf("========================================================\n");
-
- RunDumpsys("APP PROVIDERS PLATFORM", {"activity", "provider", "all-platform"},
- DUMPSYS_COMPONENTS_OPTIONS);
-
- printf("========================================================\n");
- printf("== Running Application Providers (non-platform)\n");
- printf("========================================================\n");
-
- RunDumpsys("APP PROVIDERS NON-PLATFORM", {"activity", "provider", "all-non-platform"},
- DUMPSYS_COMPONENTS_OPTIONS);
+ if (ds.dump_pool_) {
+ WAIT_TASK_WITH_CONSENT_CHECK(DUMP_APP_INFOS_TASK, ds.dump_pool_);
+ } else {
+ RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_APP_INFOS_TASK, DumpAppInfos);
+ }
printf("========================================================\n");
printf("== Dropbox crashes\n");
@@ -1680,7 +1752,12 @@
// Add linker configuration directory
ds.AddDir(LINKERCONFIG_DIR, true);
- RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK(DumpIncidentReport);
+ if (ds.dump_pool_) {
+ WAIT_TASK_WITH_CONSENT_CHECK(DUMP_INCIDENT_REPORT_TASK, ds.dump_pool_);
+ } else {
+ RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK_AND_LOG(DUMP_INCIDENT_REPORT_TASK,
+ DumpIncidentReport);
+ }
return Dumpstate::RunStatus::OK;
}
@@ -1775,31 +1852,39 @@
return status;
}
+// Common states for telephony and wifi which are needed to be collected before
+// dumpstate drop the root user.
+static void DumpstateRadioAsRoot() {
+ DumpIpTablesAsRoot();
+ ds.AddDir(LOGPERSIST_DATA_DIR, false);
+}
+
// This method collects common dumpsys for telephony and wifi. Typically, wifi
// reports are fine to include all information, but telephony reports on user
// builds need to strip some content (see DumpstateTelephonyOnly).
static void DumpstateRadioCommon(bool include_sensitive_info = true) {
- DumpIpTablesAsRoot();
-
- ds.AddDir(LOGPERSIST_DATA_DIR, false);
-
- if (!DropRootUser()) {
- return;
- }
-
// We need to be picky about some stuff for telephony reports on user builds.
if (!include_sensitive_info) {
// Only dump the radio log buffer (other buffers and dumps contain too much unrelated info).
DoRadioLogcat();
} else {
+ // DumpHals takes long time, post it to the another thread in the pool,
+ // if pool is available.
+ if (ds.dump_pool_) {
+ ds.dump_pool_->enqueueTaskWithFd(DUMP_HALS_TASK, &DumpHals, _1);
+ }
// Contains various system properties and process startup info.
do_dmesg();
// Logs other than the radio buffer may contain package/component names and potential PII.
DoLogcat();
// Too broad for connectivity problems.
DoKmsg();
- // Contains unrelated hardware info (camera, NFC, biometrics, ...).
- DumpHals();
+ // DumpHals contains unrelated hardware info (camera, NFC, biometrics, ...).
+ if (ds.dump_pool_) {
+ ds.dump_pool_->waitForTask(DUMP_HALS_TASK);
+ } else {
+ RUN_SLOW_FUNCTION_AND_LOG(DUMP_HALS_TASK, DumpHals);
+ }
}
DumpPacketStats();
@@ -1823,6 +1908,21 @@
const bool include_sensitive_info = !PropertiesHelper::IsUserBuild();
+ DumpstateRadioAsRoot();
+ if (!DropRootUser()) {
+ return;
+ }
+
+ // Starts thread pool after the root user is dropped, and two additional threads
+ // are created for DumpHals in the DumpstateRadioCommon and DumpstateBoard.
+ 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);
+ }
+
DumpstateRadioCommon(include_sensitive_info);
if (include_sensitive_info) {
@@ -1899,12 +1999,29 @@
printf("========================================================\n");
printf("== dumpstate: done (id %d)\n", ds.id_);
printf("========================================================\n");
+
+ if (ds.dump_pool_) {
+ ds.dump_pool_->waitForTask(DUMP_BOARD_TASK);
+ } else {
+ RUN_SLOW_FUNCTION_AND_LOG(DUMP_BOARD_TASK, ds.DumpstateBoard);
+ }
}
// This method collects dumpsys for wifi debugging only
static void DumpstateWifiOnly() {
DurationReporter duration_reporter("DUMPSTATE");
+ DumpstateRadioAsRoot();
+ if (!DropRootUser()) {
+ return;
+ }
+
+ // Starts thread pool after the root user is dropped. Only one additional
+ // thread is needed for DumpHals in the DumpstateRadioCommon.
+ if (ds.dump_pool_) {
+ ds.dump_pool_->start(/*thread_counts =*/1);
+ }
+
DumpstateRadioCommon();
printf("========================================================\n");
@@ -2029,11 +2146,10 @@
return RunStatus::OK;
}
-void Dumpstate::DumpstateBoard() {
- DurationReporter duration_reporter("dumpstate_board()");
- printf("========================================================\n");
- printf("== Board\n");
- printf("========================================================\n");
+void Dumpstate::DumpstateBoard(int out_fd) {
+ dprintf(out_fd, "========================================================\n");
+ dprintf(out_fd, "== Board\n");
+ dprintf(out_fd, "========================================================\n");
if (!IsZipping()) {
MYLOGD("Not dumping board info because it's not a zipped bugreport\n");
@@ -2159,8 +2275,9 @@
MYLOGE("Ignoring empty %s\n", kDumpstateBoardFiles[i].c_str());
continue;
}
- AddZipEntry(kDumpstateBoardFiles[i], paths[i]);
- printf("*** See %s entry ***\n", kDumpstateBoardFiles[i].c_str());
+ remover[i].Disable();
+ EnqueueAddZipEntryAndCleanupIfNeeded(kDumpstateBoardFiles[i], paths[i]);
+ dprintf(out_fd, "*** See %s entry ***\n", kDumpstateBoardFiles[i].c_str());
}
}
@@ -2190,6 +2307,11 @@
}
bool Dumpstate::FinishZipFile() {
+ // Runs all enqueued adding zip entry and cleanup tasks before finishing the zip file.
+ if (zip_entry_tasks_) {
+ zip_entry_tasks_->run(/* do_cancel = */false);
+ }
+
std::string entry_name = base_name_ + "-" + name_ + ".txt";
MYLOGD("Adding main entry (%s) from %s to .zip bugreport\n", entry_name.c_str(),
tmp_path_.c_str());
@@ -2393,7 +2515,6 @@
break;
case Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE:
// Currently, the dumpstate binder is only used by Shell to update progress.
- options->do_start_service = true;
options->do_progress_updates = true;
options->do_screenshot = is_screenshot_requested;
options->dumpstate_hal_mode = DumpstateMode::INTERACTIVE;
@@ -2405,7 +2526,6 @@
options->dumpstate_hal_mode = DumpstateMode::REMOTE;
break;
case Dumpstate::BugreportMode::BUGREPORT_WEAR:
- options->do_start_service = true;
options->do_progress_updates = true;
options->do_zip_file = true;
options->do_screenshot = is_screenshot_requested;
@@ -2432,12 +2552,12 @@
static void LogDumpOptions(const Dumpstate::DumpOptions& options) {
MYLOGI(
"do_zip_file: %d do_vibrate: %d use_socket: %d use_control_socket: %d do_screenshot: %d "
- "is_remote_mode: %d show_header_only: %d do_start_service: %d telephony_only: %d "
+ "is_remote_mode: %d show_header_only: %d telephony_only: %d "
"wifi_only: %d do_progress_updates: %d fd: %d bugreport_mode: %s dumpstate_hal_mode: %s "
"limited_only: %d args: %s\n",
options.do_zip_file, options.do_vibrate, options.use_socket, options.use_control_socket,
options.do_screenshot, options.is_remote_mode, options.show_header_only,
- options.do_start_service, options.telephony_only, options.wifi_only,
+ options.telephony_only, options.wifi_only,
options.do_progress_updates, options.bugreport_fd.get(), options.bugreport_mode.c_str(),
toString(options.dumpstate_hal_mode).c_str(), options.limited_only, options.args.c_str());
}
@@ -2653,15 +2773,6 @@
register_sig_handler();
- // TODO(b/111441001): maybe skip if already started?
- if (options_->do_start_service) {
- MYLOGI("Starting 'dumpstate' service\n");
- android::status_t ret;
- if ((ret = android::os::DumpstateService::Start()) != android::OK) {
- MYLOGE("Unable to start DumpstateService: %d\n", ret);
- }
- }
-
if (PropertiesHelper::IsDryRun()) {
MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n");
}
@@ -2773,7 +2884,6 @@
onUiIntensiveBugreportDumpsFinished(calling_uid, calling_package);
MaybeCheckUserConsent(calling_uid, calling_package);
DumpstateTelephonyOnly(calling_package);
- DumpstateBoard();
} else if (options_->wifi_only) {
MaybeTakeEarlyScreenshot();
onUiIntensiveBugreportDumpsFinished(calling_uid, calling_package);
@@ -2936,6 +3046,7 @@
return;
}
dump_pool_ = std::make_unique<DumpPool>(bugreport_internal_dir_);
+ zip_entry_tasks_ = std::make_unique<TaskQueue>();
}
void Dumpstate::ShutdownDumpPool() {
@@ -2943,6 +3054,27 @@
dump_pool_->shutdown();
dump_pool_ = nullptr;
}
+ if (zip_entry_tasks_) {
+ zip_entry_tasks_->run(/* do_cancel = */true);
+ zip_entry_tasks_ = nullptr;
+ }
+}
+
+void Dumpstate::EnqueueAddZipEntryAndCleanupIfNeeded(const std::string& entry_name,
+ const std::string& entry_path) {
+ auto func_add_zip_entry_and_cleanup = [=](bool task_cancelled) {
+ if (!task_cancelled) {
+ AddZipEntry(entry_name, entry_path);
+ }
+ android::os::UnlinkAndLogOnError(entry_path);
+ };
+ if (zip_entry_tasks_) {
+ // Enqueues AddZipEntryAndCleanup function if the parallel run is enabled.
+ zip_entry_tasks_->add(func_add_zip_entry_and_cleanup, _1);
+ } else {
+ // Invokes AddZipEntryAndCleanup immediately
+ std::invoke(func_add_zip_entry_and_cleanup, /* task_cancelled = */false);
+ }
}
Dumpstate::RunStatus Dumpstate::HandleUserConsentDenied() {
@@ -3666,10 +3798,11 @@
}
int Dumpstate::RunCommand(const std::string& title, const std::vector<std::string>& full_command,
- const CommandOptions& options, bool verbose_duration) {
- DurationReporter duration_reporter(title, false /* logcat_only */, verbose_duration);
+ const CommandOptions& options, bool verbose_duration, int out_fd) {
+ DurationReporter duration_reporter(title, false /* logcat_only */,
+ verbose_duration, out_fd);
- int status = RunCommandToFd(STDOUT_FILENO, title, full_command, options);
+ int status = RunCommandToFd(out_fd, title, full_command, options);
/* TODO: for now we're simplifying the progress calculation by using the
* timeout as the weight. It's a good approximation for most cases, except when calling dumpsys,
@@ -3681,11 +3814,11 @@
}
void Dumpstate::RunDumpsys(const std::string& title, const std::vector<std::string>& dumpsys_args,
- const CommandOptions& options, long dumpsysTimeoutMs) {
+ const CommandOptions& options, long dumpsysTimeoutMs, int out_fd) {
long timeout_ms = dumpsysTimeoutMs > 0 ? dumpsysTimeoutMs : options.TimeoutInMs();
std::vector<std::string> dumpsys = {"/system/bin/dumpsys", "-T", std::to_string(timeout_ms)};
dumpsys.insert(dumpsys.end(), dumpsys_args.begin(), dumpsys_args.end());
- RunCommand(title, dumpsys, options);
+ RunCommand(title, dumpsys, options, false, out_fd);
}
int open_socket(const char *service) {
@@ -3808,12 +3941,16 @@
fclose(fp);
}
-// TODO: make this function thread safe if sections are generated in parallel.
void Dumpstate::UpdateProgress(int32_t delta_sec) {
if (progress_ == nullptr) {
MYLOGE("UpdateProgress: progress_ not set\n");
return;
}
+ // This function updates progress related members of the dumpstate and reports
+ // progress percentage to the bugreport client. Since it could be called by
+ // different dump tasks at the same time if the parallel run is enabled, a
+ // mutex lock is necessary here to synchronize the call.
+ std::lock_guard<std::recursive_mutex> lock(mutex_);
// Always update progess so stats can be tuned...
progress_->Inc(delta_sec);
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index d400dc7..9582c9d 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -36,6 +36,7 @@
#include "DumpstateUtil.h"
#include "DumpPool.h"
+#include "TaskQueue.h"
// Workaround for const char *args[MAX_ARGS_ARRAY_SIZE] variables until they're converted to
// std::vector<std::string>
@@ -229,11 +230,13 @@
* |full_command| array containing the command (first entry) and its arguments.
* Must contain at least one element.
* |options| optional argument defining the command's behavior.
+ * |out_fd| A fd to support the DumpPool to output results to a temporary
+ * file. Using STDOUT_FILENO if it's not running in the parallel task.
*/
int RunCommand(const std::string& title, const std::vector<std::string>& fullCommand,
const android::os::dumpstate::CommandOptions& options =
android::os::dumpstate::CommandOptions::DEFAULT,
- bool verbose_duration = false);
+ bool verbose_duration = false, int out_fd = STDOUT_FILENO);
/*
* Runs `dumpsys` with the given arguments, automatically setting its timeout
@@ -246,10 +249,12 @@
* |options| optional argument defining the command's behavior.
* |dumpsys_timeout| when > 0, defines the value passed to `dumpsys -T` (otherwise it uses the
* timeout from `options`)
+ * |out_fd| A fd to support the DumpPool to output results to a temporary
+ * file. Using STDOUT_FILENO if it's not running in the parallel task.
*/
void RunDumpsys(const std::string& title, const std::vector<std::string>& dumpsys_args,
const android::os::dumpstate::CommandOptions& options = DEFAULT_DUMPSYS,
- long dumpsys_timeout_ms = 0);
+ long dumpsys_timeout_ms = 0, int out_fd = STDOUT_FILENO);
/*
* Prints the contents of a file.
@@ -306,7 +311,12 @@
// Returns OK in all other cases.
RunStatus DumpTraces(const char** path);
- void DumpstateBoard();
+ /*
+ * |out_fd| A fd to support the DumpPool to output results to a temporary file.
+ * Dumpstate can pick up later and output to the bugreport. Using STDOUT_FILENO
+ * if it's not running in the parallel task.
+ */
+ void DumpstateBoard(int out_fd = STDOUT_FILENO);
/*
* Updates the overall progress of the bugreport generation by the given weight increment.
@@ -363,6 +373,18 @@
bool CalledByApi() const;
/*
+ * Enqueues a task to the dumpstate's TaskQueue if the parallel run is enabled,
+ * otherwise invokes it immediately. The task adds file at path entry_path
+ * as a zip file entry with name entry_name. Unlinks entry_path when done.
+ *
+ * All enqueued tasks will be executed in the dumpstate's FinishZipFile method
+ * before the zip file is finished. Tasks will be cancelled in dumpstate's
+ * ShutdownDumpPool method if they have never been called.
+ */
+ void EnqueueAddZipEntryAndCleanupIfNeeded(const std::string& entry_name,
+ const std::string& entry_path);
+
+ /*
* Structure to hold options that determine the behavior of dumpstate.
*/
struct DumpOptions {
@@ -376,7 +398,6 @@
bool is_screenshot_copied = false;
bool is_remote_mode = false;
bool show_header_only = false;
- bool do_start_service = false;
bool telephony_only = false;
bool wifi_only = false;
// Trimmed-down version of dumpstate to only include whitelisted logs.
@@ -495,6 +516,10 @@
// A thread pool to execute dump tasks simultaneously if the parallel run is enabled.
std::unique_ptr<android::os::dumpstate::DumpPool> dump_pool_;
+ // A task queue to collect adding zip entry tasks inside dump tasks if the
+ // parallel run is enabled.
+ std::unique_ptr<android::os::dumpstate::TaskQueue> zip_entry_tasks_;
+
// A callback to IncidentCompanion service, which checks user consent for sharing the
// bugreport with the calling app. If the user has not responded yet to the dialog it will
// be neither confirmed nor denied.
@@ -548,6 +573,8 @@
android::sp<ConsentCallback> consent_callback_;
+ std::recursive_mutex mutex_;
+
DISALLOW_COPY_AND_ASSIGN(Dumpstate);
};
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index b3cb434..6b93692 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -258,7 +258,6 @@
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
EXPECT_FALSE(options_.use_socket);
- EXPECT_FALSE(options_.do_start_service);
EXPECT_FALSE(options_.limited_only);
}
@@ -267,7 +266,6 @@
EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.do_progress_updates);
- EXPECT_TRUE(options_.do_start_service);
EXPECT_TRUE(options_.do_screenshot);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::INTERACTIVE);
@@ -303,7 +301,6 @@
EXPECT_TRUE(options_.do_screenshot);
EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.do_progress_updates);
- EXPECT_TRUE(options_.do_start_service);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::WEAR);
// Other options retain default values
@@ -1026,6 +1023,7 @@
SetParallelRun(true);
EnableParallelRunIfNeeded();
EXPECT_TRUE(ds.options_->OutputToFile());
+ EXPECT_TRUE(ds.zip_entry_tasks_);
EXPECT_TRUE(ds.dump_pool_);
}
@@ -1033,12 +1031,14 @@
ds.options_->use_socket = true;
EnableParallelRunIfNeeded();
EXPECT_FALSE(ds.options_->OutputToFile());
+ EXPECT_FALSE(ds.zip_entry_tasks_);
EXPECT_FALSE(ds.dump_pool_);
}
TEST_F(DumpstateTest, DumpPool_withParallelRunDisabled_isNull) {
SetParallelRun(false);
EnableParallelRunIfNeeded();
+ EXPECT_FALSE(ds.zip_entry_tasks_);
EXPECT_FALSE(ds.dump_pool_);
}
@@ -1749,6 +1749,57 @@
EXPECT_THAT(getTempFileCounts(kTestDataPath), Eq(0));
}
+class TaskQueueTest : public DumpstateBaseTest {
+public:
+ void SetUp() {
+ DumpstateBaseTest::SetUp();
+ }
+
+ TaskQueue task_queue_;
+};
+
+TEST_F(TaskQueueTest, runTask) {
+ bool is_task1_run = false;
+ bool is_task2_run = false;
+ auto task_1 = [&](bool task_cancelled) {
+ if (task_cancelled) {
+ return;
+ }
+ is_task1_run = true;
+ };
+ auto task_2 = [&](bool task_cancelled) {
+ if (task_cancelled) {
+ return;
+ }
+ is_task2_run = true;
+ };
+ task_queue_.add(task_1, std::placeholders::_1);
+ task_queue_.add(task_2, std::placeholders::_1);
+
+ task_queue_.run(/* do_cancel = */false);
+
+ EXPECT_TRUE(is_task1_run);
+ EXPECT_TRUE(is_task2_run);
+}
+
+TEST_F(TaskQueueTest, runTask_withCancelled) {
+ bool is_task1_cancelled = false;
+ bool is_task2_cancelled = false;
+ auto task_1 = [&](bool task_cancelled) {
+ is_task1_cancelled = task_cancelled;
+ };
+ auto task_2 = [&](bool task_cancelled) {
+ is_task2_cancelled = task_cancelled;
+ };
+ task_queue_.add(task_1, std::placeholders::_1);
+ task_queue_.add(task_2, std::placeholders::_1);
+
+ task_queue_.run(/* do_cancel = */true);
+
+ EXPECT_TRUE(is_task1_cancelled);
+ EXPECT_TRUE(is_task2_cancelled);
+}
+
} // namespace dumpstate
} // namespace os
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 7d9fd51..51fd84c 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <android/binder_context.h>
#include <android/binder_ibinder.h>
#include <android/binder_ibinder_platform.h>
#include "ibinder_internal.h"
diff --git a/libs/binder/ndk/include_platform/android/binder_context.h b/libs/binder/ndk/include_platform/android/binder_context.h
new file mode 100644
index 0000000..a99d555
--- /dev/null
+++ b/libs/binder/ndk/include_platform/android/binder_context.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <android/binder_ibinder.h>
+
+__BEGIN_DECLS
+
+/**
+ * Makes calls to AIBinder_getCallingSid work if the kernel supports it. This
+ * must be called on a local binder server before it is sent out to any othe
+ * process. If this is a remote binder, it will abort. If the kernel doesn't
+ * support this feature, you'll always get null from AIBinder_getCallingSid.
+ *
+ * \param binder local server binder to request security contexts on
+ */
+__attribute__((weak)) void AIBinder_setRequestingSid(AIBinder* binder, bool requestingSid)
+ __INTRODUCED_IN(31);
+
+/**
+ * Returns the selinux context of the callee.
+ *
+ * In order for this to work, the following conditions must be met:
+ * - The kernel must be new enough to support this feature.
+ * - The server must have called AIBinder_setRequestingSid.
+ * - The callee must be a remote process.
+ *
+ * \return security context or null if unavailable. The lifetime of this context
+ * is the lifetime of the transaction.
+ */
+__attribute__((weak, warn_unused_result)) const char* AIBinder_getCallingSid() __INTRODUCED_IN(31);
+
+__END_DECLS
diff --git a/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h b/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h
index 5811760..2af65cf 100644
--- a/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h
+++ b/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h
@@ -16,41 +16,14 @@
#pragma once
+// binder_context.h used to be part of this header and is included for backwards
+// compatibility.
+#include <android/binder_context.h>
+
+#if !defined(__ANDROID_APEX__) && !defined(__ANDROID_VNDK__)
+
#include <android/binder_ibinder.h>
-
-#if !defined(__ANDROID_APEX__) && !defined(__ANDROID_VNDK__)
#include <binder/IBinder.h>
-#endif
-
-__BEGIN_DECLS
-
-/**
- * Makes calls to AIBinder_getCallingSid work if the kernel supports it. This
- * must be called on a local binder server before it is sent out to any othe
- * process. If this is a remote binder, it will abort. If the kernel doesn't
- * support this feature, you'll always get null from AIBinder_getCallingSid.
- *
- * \param binder local server binder to request security contexts on
- */
-__attribute__((weak)) void AIBinder_setRequestingSid(AIBinder* binder, bool requestingSid)
- __INTRODUCED_IN(31);
-
-/**
- * Returns the selinux context of the callee.
- *
- * In order for this to work, the following conditions must be met:
- * - The kernel must be new enough to support this feature.
- * - The server must have called AIBinder_setRequestingSid.
- * - The callee must be a remote process.
- *
- * \return security context or null if unavailable. The lifetime of this context
- * is the lifetime of the transaction.
- */
-__attribute__((weak, warn_unused_result)) const char* AIBinder_getCallingSid() __INTRODUCED_IN(31);
-
-__END_DECLS
-
-#if !defined(__ANDROID_APEX__) && !defined(__ANDROID_VNDK__)
/**
* Get libbinder version of binder from AIBinder.
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index 2bd5248..160b9f2 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -18,6 +18,7 @@
#include <aidl/BnBinderNdkUnitTest.h>
#include <aidl/BnEmpty.h>
#include <android-base/logging.h>
+#include <android/binder_context.h>
#include <android/binder_ibinder_jni.h>
#include <android/binder_ibinder_platform.h>
#include <android/binder_manager.h>
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index 6bf9cd5..ceb6ade 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -98,6 +98,9 @@
/// Send a ping transaction to this object
fn ping_binder(&mut self) -> Result<()>;
+ /// Indicate that the service intends to receive caller security contexts.
+ fn set_requesting_sid(&mut self, enable: bool);
+
/// Dump this object to the given file handle
fn dump<F: AsRawFd>(&mut self, fp: &F, args: &[&str]) -> Result<()>;
diff --git a/libs/binder/rust/src/parcel.rs b/libs/binder/rust/src/parcel.rs
index a248f5c..2c1e5a4 100644
--- a/libs/binder/rust/src/parcel.rs
+++ b/libs/binder/rust/src/parcel.rs
@@ -345,7 +345,6 @@
fn test_read_write() {
use crate::binder::Interface;
use crate::native::Binder;
- use std::ffi::CString;
let mut service = Binder::new(()).as_binder();
let mut parcel = Parcel::new_for_test(&mut service).unwrap();
@@ -360,7 +359,7 @@
assert_eq!(parcel.read::<u64>(), Err(StatusCode::NOT_ENOUGH_DATA));
assert_eq!(parcel.read::<f32>(), Err(StatusCode::NOT_ENOUGH_DATA));
assert_eq!(parcel.read::<f64>(), Err(StatusCode::NOT_ENOUGH_DATA));
- assert_eq!(parcel.read::<Option<CString>>(), Ok(None));
+ assert_eq!(parcel.read::<Option<String>>(), Ok(None));
assert_eq!(parcel.read::<String>(), Err(StatusCode::UNEXPECTED_NULL));
assert_eq!(parcel.read_binder().err(), Some(StatusCode::BAD_TYPE));
@@ -470,11 +469,24 @@
}
assert_eq!(
parcel.read::<Option<String>>().unwrap().unwrap(),
- "Hello, Binder!"
+ "Hello, Binder!",
);
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
+
+ assert!(parcel.write("Embedded null \0 inside a string").is_ok());
+ unsafe {
+ assert!(parcel.set_data_position(start).is_ok());
+ }
+ assert_eq!(
+ parcel.read::<Option<String>>().unwrap().unwrap(),
+ "Embedded null \0 inside a string",
+ );
+ unsafe {
+ assert!(parcel.set_data_position(start).is_ok());
+ }
+
assert!(parcel.write(&["str1", "str2", "str3"][..]).is_ok());
assert!(parcel
.write(
diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs
index 78b3d2c..138b360 100644
--- a/libs/binder/rust/src/parcel/parcelable.rs
+++ b/libs/binder/rust/src/parcel/parcelable.rs
@@ -21,7 +21,8 @@
use crate::sys;
use std::convert::TryInto;
-use std::ffi::{c_void, CStr, CString};
+use std::ffi::c_void;
+use std::os::raw::c_char;
use std::ptr;
/// A struct whose instances can be written to a [`Parcel`].
@@ -340,7 +341,7 @@
}
}
-impl SerializeOption for CStr {
+impl SerializeOption for str {
fn serialize_option(this: Option<&Self>, parcel: &mut Parcel) -> Result<()> {
match this {
None => unsafe {
@@ -356,14 +357,17 @@
},
Some(s) => unsafe {
// Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. `AParcel_writeString` assumes that we pass a
- // null-terminated C string pointer with no nulls in the middle
- // of the string. Rust guarantees exactly that for a valid CStr
- // instance.
+ // `AParcel`. `AParcel_writeString` assumes that we pass a utf-8
+ // string pointer of `length` bytes, which is what str in Rust
+ // is. The docstring for `AParcel_writeString` says that the
+ // string input should be null-terminated, but it doesn't
+ // actually rely on that fact in the code. If this ever becomes
+ // necessary, we will need to null-terminate the str buffer
+ // before sending it.
status_result(sys::AParcel_writeString(
parcel.as_native_mut(),
- s.as_ptr(),
- s.to_bytes()
+ s.as_ptr() as *const c_char,
+ s.as_bytes()
.len()
.try_into()
.or(Err(StatusCode::BAD_VALUE))?,
@@ -373,29 +377,15 @@
}
}
-impl SerializeArray for Option<&CStr> {}
+impl SerializeArray for Option<&str> {}
-impl Serialize for CStr {
+impl Serialize for str {
fn serialize(&self, parcel: &mut Parcel) -> Result<()> {
Some(self).serialize(parcel)
}
}
-impl Serialize for CString {
- fn serialize(&self, parcel: &mut Parcel) -> Result<()> {
- Some(self.as_c_str()).serialize(parcel)
- }
-}
-
-impl SerializeArray for CString {}
-
-impl SerializeOption for CString {
- fn serialize_option(this: Option<&Self>, parcel: &mut Parcel) -> Result<()> {
- SerializeOption::serialize_option(this.map(CString::as_c_str), parcel)
- }
-}
-
-impl SerializeArray for Option<CString> {}
+impl SerializeArray for &str {}
impl Serialize for String {
fn serialize(&self, parcel: &mut Parcel) -> Result<()> {
@@ -413,7 +403,7 @@
impl SerializeArray for Option<String> {}
-impl Deserialize for Option<CString> {
+impl Deserialize for Option<String> {
fn deserialize(parcel: &Parcel) -> Result<Self> {
let mut vec: Option<Vec<u8>> = None;
let status = unsafe {
@@ -430,26 +420,15 @@
status_result(status)?;
vec.map(|mut s| {
- // The vector includes a null-terminator and CString::new requires
- // no nulls, including terminating.
+ // The vector includes a null-terminator and we don't want the
+ // string to be null-terminated for Rust.
s.pop();
- CString::new(s).or(Err(StatusCode::BAD_VALUE))
+ String::from_utf8(s).or(Err(StatusCode::BAD_VALUE))
})
.transpose()
}
}
-impl DeserializeArray for Option<CString> {}
-
-impl DeserializeOption for String {
- fn deserialize_option(parcel: &Parcel) -> Result<Option<Self>> {
- let c_str = <Option<CString>>::deserialize(parcel)?;
- c_str
- .map(|s| s.into_string().or(Err(StatusCode::BAD_VALUE)))
- .transpose()
- }
-}
-
impl DeserializeArray for Option<String> {}
impl Deserialize for String {
@@ -462,28 +441,6 @@
impl DeserializeArray for String {}
-impl SerializeOption for str {
- fn serialize_option(this: Option<&Self>, parcel: &mut Parcel) -> Result<()> {
- match this {
- None => parcel.write(&-1i32),
- Some(s) => {
- let c_str = CString::new(s).or(Err(StatusCode::BAD_VALUE))?;
- parcel.write(&c_str)
- }
- }
- }
-}
-
-impl Serialize for str {
- fn serialize(&self, parcel: &mut Parcel) -> Result<()> {
- Some(self).serialize(parcel)
- }
-}
-
-impl SerializeArray for &str {}
-
-impl SerializeArray for Option<&str> {}
-
impl<T: SerializeArray> Serialize for [T] {
fn serialize(&self, parcel: &mut Parcel) -> Result<()> {
SerializeArray::serialize_array(self, parcel)
@@ -905,8 +862,9 @@
let s1 = "Hello, Binder!";
let s2 = "This is a utf8 string.";
let s3 = "Some more text here.";
+ let s4 = "Embedded nulls \0 \0";
- let strs = [s1, s2, s3];
+ let strs = [s1, s2, s3, s4];
unsafe {
assert!(parcel.set_data_position(start).is_ok());
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 5602c96..82212d8 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -249,6 +249,12 @@
status_result(status)
}
+ fn set_requesting_sid(&mut self, enable: bool) {
+ unsafe {
+ sys::AIBinder_setRequestingSid(self.as_native_mut(), enable)
+ };
+ }
+
fn dump<F: AsRawFd>(&mut self, fp: &F, args: &[&str]) -> Result<()> {
let args: Vec<_> = args.iter().map(|a| CString::new(*a).unwrap()).collect();
let mut arg_ptrs: Vec<_> = args.iter().map(|a| a.as_ptr()).collect();
diff --git a/libs/binder/rust/src/state.rs b/libs/binder/rust/src/state.rs
index 992f074..0e05f10 100644
--- a/libs/binder/rust/src/state.rs
+++ b/libs/binder/rust/src/state.rs
@@ -98,4 +98,36 @@
sys::AIBinder_getCallingPid()
}
}
+
+ /// This function makes the client's security context available to the
+ /// service calling this function. This can be used for access control.
+ /// It does not suffer from the TOCTOU issues of get_calling_pid.
+ ///
+ /// Implementations of `check_permission` should use the given CStr
+ /// argument as context for selinux permission checks. If `None` is
+ /// given, the implementation should fall back to using the PID
+ /// instead.
+ ///
+ /// Note: `None` may be passed to the callback if the caller did not
+ /// `set_requesting_sid` on the serviced binder, or if the underlying
+ /// kernel is too old to support this feature.
+ pub fn with_calling_sid<T, F>(check_permission: F) -> T
+ where
+ for<'a> F: FnOnce(Option<&'a std::ffi::CStr>) -> T {
+ // Safety: AIBinder_getCallingSid returns a c-string pointer
+ // that is valid for a transaction. Also, the string returned
+ // is thread local. By restricting the lifetime of the CStr
+ // reference to the scope of the callback, we prevent it being
+ // used beyond the guaranteed lifetime.
+ check_permission(unsafe {
+ let sid = sys::AIBinder_getCallingSid();
+ // AIBinder_getCallingSid() returns a '\0' terminated string
+ // or NULL.
+ if sid.is_null() {
+ None
+ } else {
+ Some(std::ffi::CStr::from_ptr(sid))
+ }
+ })
+ }
}
diff --git a/libs/binder/rust/sys/BinderBindings.h b/libs/binder/rust/sys/BinderBindings.h
index c7a06d9..303f4a5 100644
--- a/libs/binder/rust/sys/BinderBindings.h
+++ b/libs/binder/rust/sys/BinderBindings.h
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <android/binder_context.h>
#include <android/binder_ibinder.h>
#include <android/binder_manager.h>
#include <android/binder_parcel.h>
diff --git a/libs/binder/rust/tests/Android.bp b/libs/binder/rust/tests/Android.bp
index 622604f..3db40ba 100644
--- a/libs/binder/rust/tests/Android.bp
+++ b/libs/binder/rust/tests/Android.bp
@@ -3,6 +3,10 @@
srcs: ["integration.rs"],
rustlibs: [
"libbinder_rs",
+ "libselinux_bindgen",
+ ],
+ shared_libs: [
+ "libselinux",
],
// For the binaries to be pushed properly as specified in AndroidTest.xml,
// this cannot be the same as the module name.
diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs
index fe59416..953d328 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -18,7 +18,8 @@
use binder::declare_binder_interface;
use binder::parcel::Parcel;
-use binder::{Binder, IBinder, Interface, SpIBinder, TransactionCode};
+use binder::{Binder, IBinder, Interface, SpIBinder, StatusCode, ThreadState, TransactionCode};
+use std::convert::{TryFrom, TryInto};
/// Name of service runner.
///
@@ -49,6 +50,7 @@
let mut service = Binder::new(BnTest(Box::new(TestService {
s: service_name.clone(),
})));
+ service.set_requesting_sid(true);
if let Some(extension_name) = extension_name {
let extension = BnTest::new_binder(TestService { s: extension_name });
service
@@ -79,18 +81,47 @@
s: String,
}
+#[repr(u32)]
+enum TestTransactionCode {
+ Test = SpIBinder::FIRST_CALL_TRANSACTION,
+ GetSelinuxContext,
+}
+
+impl TryFrom<u32> for TestTransactionCode {
+ type Error = StatusCode;
+
+ fn try_from(c: u32) -> Result<Self, Self::Error> {
+ match c {
+ _ if c == TestTransactionCode::Test as u32 => Ok(TestTransactionCode::Test),
+ _ if c == TestTransactionCode::GetSelinuxContext as u32 => {
+ Ok(TestTransactionCode::GetSelinuxContext)
+ }
+ _ => Err(StatusCode::UNKNOWN_TRANSACTION),
+ }
+ }
+}
+
impl Interface for TestService {}
impl ITest for TestService {
fn test(&self) -> binder::Result<String> {
Ok(self.s.clone())
}
+
+ fn get_selinux_context(&self) -> binder::Result<String> {
+ let sid =
+ ThreadState::with_calling_sid(|sid| sid.map(|s| s.to_string_lossy().into_owned()));
+ sid.ok_or(StatusCode::UNEXPECTED_NULL)
+ }
}
/// Trivial testing binder interface
pub trait ITest: Interface {
/// Returns a test string
fn test(&self) -> binder::Result<String>;
+
+ /// Returns the caller's SELinux context
+ fn get_selinux_context(&self) -> binder::Result<String>;
}
declare_binder_interface! {
@@ -104,19 +135,30 @@
fn on_transact(
service: &dyn ITest,
- _code: TransactionCode,
+ code: TransactionCode,
_data: &Parcel,
reply: &mut Parcel,
) -> binder::Result<()> {
- reply.write(&service.test()?)?;
- Ok(())
+ match code.try_into()? {
+ TestTransactionCode::Test => reply.write(&service.test()?),
+ TestTransactionCode::GetSelinuxContext => reply.write(&service.get_selinux_context()?),
+ }
}
impl ITest for BpTest {
fn test(&self) -> binder::Result<String> {
- let reply = self
- .binder
- .transact(SpIBinder::FIRST_CALL_TRANSACTION, 0, |_| Ok(()))?;
+ let reply =
+ self.binder
+ .transact(TestTransactionCode::Test as TransactionCode, 0, |_| Ok(()))?;
+ reply.read()
+ }
+
+ fn get_selinux_context(&self) -> binder::Result<String> {
+ let reply = self.binder.transact(
+ TestTransactionCode::GetSelinuxContext as TransactionCode,
+ 0,
+ |_| Ok(()),
+ )?;
reply.read()
}
}
@@ -125,12 +167,19 @@
fn test(&self) -> binder::Result<String> {
self.0.test()
}
+
+ fn get_selinux_context(&self) -> binder::Result<String> {
+ self.0.get_selinux_context()
+ }
}
#[cfg(test)]
mod tests {
+ use selinux_bindgen as selinux_sys;
+ use std::ffi::CStr;
use std::fs::File;
use std::process::{Child, Command};
+ use std::ptr;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use std::thread;
@@ -203,6 +252,24 @@
assert_eq!(test_client.test().unwrap(), "trivial_client_test");
}
+ #[test]
+ fn get_selinux_context() {
+ let service_name = "get_selinux_context";
+ let _process = ScopedServiceProcess::new(service_name);
+ let test_client: Box<dyn ITest> =
+ binder::get_interface(service_name).expect("Did not get manager binder service");
+ let expected_context = unsafe {
+ let mut out_ptr = ptr::null_mut();
+ assert_eq!(selinux_sys::getcon(&mut out_ptr), 0);
+ assert!(!out_ptr.is_null());
+ CStr::from_ptr(out_ptr)
+ };
+ assert_eq!(
+ test_client.get_selinux_context().unwrap(),
+ expected_context.to_str().expect("context was invalid UTF-8"),
+ );
+ }
+
fn register_death_notification(binder: &mut SpIBinder) -> (Arc<AtomicBool>, DeathRecipient) {
let binder_died = Arc::new(AtomicBool::new(false));
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index 0ac493d..c7c75da 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -244,10 +244,25 @@
{
Parcel data, reply;
data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
- data.writeString8(displayName);
- data.writeInt32(secure ? 1 : 0);
- remote()->transact(BnSurfaceComposer::CREATE_DISPLAY, data, &reply);
- return reply.readStrongBinder();
+ status_t status = data.writeString8(displayName);
+ if (status) {
+ return nullptr;
+ }
+ status = data.writeBool(secure);
+ if (status) {
+ return nullptr;
+ }
+
+ status = remote()->transact(BnSurfaceComposer::CREATE_DISPLAY, data, &reply);
+ if (status) {
+ return nullptr;
+ }
+ sp<IBinder> display;
+ status = reply.readNullableStrongBinder(&display);
+ if (status) {
+ return nullptr;
+ }
+ return display;
}
virtual void destroyDisplay(const sp<IBinder>& display)
@@ -1295,10 +1310,12 @@
}
case CREATE_DISPLAY: {
CHECK_INTERFACE(ISurfaceComposer, data, reply);
- String8 displayName = data.readString8();
- bool secure = bool(data.readInt32());
- sp<IBinder> display(createDisplay(displayName, secure));
- reply->writeStrongBinder(display);
+ String8 displayName;
+ SAFE_PARCEL(data.readString8, &displayName);
+ bool secure = false;
+ SAFE_PARCEL(data.readBool, &secure);
+ sp<IBinder> display = createDisplay(displayName, secure);
+ SAFE_PARCEL(reply->writeStrongBinder, display);
return NO_ERROR;
}
case DESTROY_DISPLAY: {
diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp
index 42e74df..3367664 100644
--- a/libs/renderengine/gl/GLESRenderEngine.cpp
+++ b/libs/renderengine/gl/GLESRenderEngine.cpp
@@ -639,13 +639,8 @@
}
}
-status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName,
- const sp<GraphicBuffer>& buffer,
- const sp<Fence>& bufferFence) {
- if (buffer == nullptr) {
- return BAD_VALUE;
- }
-
+void GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, const sp<GraphicBuffer>& buffer,
+ const sp<Fence>& bufferFence) {
ATRACE_CALL();
bool found = false;
@@ -661,7 +656,8 @@
if (!found) {
status_t cacheResult = mImageManager->cache(buffer);
if (cacheResult != NO_ERROR) {
- return cacheResult;
+ ALOGE("Error with caching buffer: %d", cacheResult);
+ return;
}
}
@@ -678,7 +674,7 @@
// We failed creating the image if we got here, so bail out.
ALOGE("Failed to create an EGLImage when rendering");
bindExternalTextureImage(texName, *createImage());
- return NO_INIT;
+ return;
}
bindExternalTextureImage(texName, *cachedImage->second);
@@ -691,22 +687,22 @@
base::unique_fd fenceFd(bufferFence->dup());
if (fenceFd == -1) {
ALOGE("error dup'ing fence fd: %d", errno);
- return -errno;
+ return;
}
if (!waitFence(std::move(fenceFd))) {
ALOGE("failed to wait on fence fd");
- return UNKNOWN_ERROR;
+ return;
}
} else {
status_t err = bufferFence->waitForever("RenderEngine::bindExternalTextureBuffer");
if (err != NO_ERROR) {
ALOGE("error waiting for fence: %d", err);
- return err;
+ return;
}
}
}
- return NO_ERROR;
+ return;
}
void GLESRenderEngine::cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) {
diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h
index 8f0df2c..c72b7f9 100644
--- a/libs/renderengine/gl/GLESRenderEngine.h
+++ b/libs/renderengine/gl/GLESRenderEngine.h
@@ -60,8 +60,6 @@
void primeCache() const override;
void genTextures(size_t count, uint32_t* names) override;
void deleteTextures(size_t count, uint32_t const* names) override;
- status_t bindExternalTextureBuffer(uint32_t texName, const sp<GraphicBuffer>& buffer,
- const sp<Fence>& fence) EXCLUDES(mRenderingMutex);
void cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) EXCLUDES(mRenderingMutex);
void unbindExternalTextureBuffer(uint64_t bufferId) EXCLUDES(mRenderingMutex);
@@ -135,6 +133,8 @@
status_t bindFrameBuffer(Framebuffer* framebuffer);
void unbindFrameBuffer(Framebuffer* framebuffer);
void bindExternalTextureImage(uint32_t texName, const Image& image);
+ void bindExternalTextureBuffer(uint32_t texName, const sp<GraphicBuffer>& buffer,
+ const sp<Fence>& fence) EXCLUDES(mRenderingMutex);
void cleanFramebufferCache() EXCLUDES(mFramebufferImageCacheMutex) override;
// A data space is considered HDR data space if it has BT2020 color space
diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h
index 3c90a3a..9b87229 100644
--- a/libs/renderengine/include/renderengine/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/RenderEngine.h
@@ -92,11 +92,6 @@
virtual bool useNativeFenceSync() const = 0;
virtual void genTextures(size_t count, uint32_t* names) = 0;
virtual void deleteTextures(size_t count, uint32_t const* names) = 0;
- // Legacy public method used by devices that don't support native fence
- // synchronization in their GPU driver, as this method provides implicit
- // synchronization for latching buffers.
- virtual status_t bindExternalTextureBuffer(uint32_t texName, const sp<GraphicBuffer>& buffer,
- const sp<Fence>& fence) = 0;
// Caches Image resources for this buffer, but does not bind the buffer to
// a particular texture.
// Note that work is deferred to an additional thread, i.e. this call
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index c92e817..d795616 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -1242,31 +1242,6 @@
EXPECT_EQ(NO_ERROR, barrier->result);
}
-TEST_F(RenderEngineTest, bindExternalBuffer_withNullBuffer) {
- status_t result = sRE->bindExternalTextureBuffer(0, nullptr, nullptr);
- ASSERT_EQ(BAD_VALUE, result);
-}
-
-TEST_F(RenderEngineTest, bindExternalBuffer_cachesImages) {
- sp<GraphicBuffer> buf = allocateSourceBuffer(1, 1);
- uint32_t texName;
- sRE->genTextures(1, &texName);
- mTexNames.push_back(texName);
-
- sRE->bindExternalTextureBuffer(texName, buf, nullptr);
- uint64_t bufferId = buf->getId();
- EXPECT_TRUE(sRE->isImageCachedForTesting(bufferId));
- std::shared_ptr<renderengine::gl::ImageManager::Barrier> barrier =
- sRE->unbindExternalTextureBufferForTesting(bufferId);
- std::lock_guard<std::mutex> lock(barrier->mutex);
- ASSERT_TRUE(barrier->condition.wait_for(barrier->mutex, std::chrono::seconds(5),
- [&]() REQUIRES(barrier->mutex) {
- return barrier->isOpen;
- }));
- EXPECT_EQ(NO_ERROR, barrier->result);
- EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId));
-}
-
TEST_F(RenderEngineTest, cacheExternalBuffer_withNullBuffer) {
std::shared_ptr<renderengine::gl::ImageManager::Barrier> barrier =
sRE->cacheExternalTextureBufferForTesting(nullptr);
diff --git a/libs/renderengine/tests/RenderEngineThreadedTest.cpp b/libs/renderengine/tests/RenderEngineThreadedTest.cpp
index 78fec29..ba5175d 100644
--- a/libs/renderengine/tests/RenderEngineThreadedTest.cpp
+++ b/libs/renderengine/tests/RenderEngineThreadedTest.cpp
@@ -62,21 +62,6 @@
mThreadedRE->deleteTextures(1, &texName);
}
-TEST_F(RenderEngineThreadedTest, bindExternalBuffer_nullptrBuffer) {
- EXPECT_CALL(*mRenderEngine, bindExternalTextureBuffer(0, Eq(nullptr), Eq(nullptr)))
- .WillOnce(Return(BAD_VALUE));
- status_t result = mThreadedRE->bindExternalTextureBuffer(0, nullptr, nullptr);
- ASSERT_EQ(BAD_VALUE, result);
-}
-
-TEST_F(RenderEngineThreadedTest, bindExternalBuffer_withBuffer) {
- sp<GraphicBuffer> buf = new GraphicBuffer();
- EXPECT_CALL(*mRenderEngine, bindExternalTextureBuffer(0, buf, Eq(nullptr)))
- .WillOnce(Return(NO_ERROR));
- status_t result = mThreadedRE->bindExternalTextureBuffer(0, buf, nullptr);
- ASSERT_EQ(NO_ERROR, result);
-}
-
TEST_F(RenderEngineThreadedTest, cacheExternalTextureBuffer_nullptr) {
EXPECT_CALL(*mRenderEngine, cacheExternalTextureBuffer(Eq(nullptr)));
mThreadedRE->cacheExternalTextureBuffer(nullptr);
diff --git a/libs/renderengine/threaded/RenderEngineThreaded.cpp b/libs/renderengine/threaded/RenderEngineThreaded.cpp
index 9b79943..953cbc6 100644
--- a/libs/renderengine/threaded/RenderEngineThreaded.cpp
+++ b/libs/renderengine/threaded/RenderEngineThreaded.cpp
@@ -161,24 +161,6 @@
resultFuture.wait();
}
-status_t RenderEngineThreaded::bindExternalTextureBuffer(uint32_t texName,
- const sp<GraphicBuffer>& buffer,
- const sp<Fence>& fence) {
- std::promise<status_t> resultPromise;
- std::future<status_t> resultFuture = resultPromise.get_future();
- {
- std::lock_guard lock(mThreadMutex);
- mFunctionCalls.push(
- [&resultPromise, texName, &buffer, &fence](renderengine::RenderEngine& instance) {
- ATRACE_NAME("REThreaded::bindExternalTextureBuffer");
- status_t status = instance.bindExternalTextureBuffer(texName, buffer, fence);
- resultPromise.set_value(status);
- });
- }
- mCondition.notify_one();
- return resultFuture.get();
-}
-
void RenderEngineThreaded::cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) {
std::promise<void> resultPromise;
std::future<void> resultFuture = resultPromise.get_future();
diff --git a/libs/renderengine/threaded/RenderEngineThreaded.h b/libs/renderengine/threaded/RenderEngineThreaded.h
index b02cdd9..ac9f606 100644
--- a/libs/renderengine/threaded/RenderEngineThreaded.h
+++ b/libs/renderengine/threaded/RenderEngineThreaded.h
@@ -48,8 +48,6 @@
bool useNativeFenceSync() const override;
void genTextures(size_t count, uint32_t* names) override;
void deleteTextures(size_t count, uint32_t const* names) override;
- status_t bindExternalTextureBuffer(uint32_t texName, const sp<GraphicBuffer>& buffer,
- const sp<Fence>& fence) override;
void cacheExternalTextureBuffer(const sp<GraphicBuffer>& buffer) override;
void unbindExternalTextureBuffer(uint64_t bufferId) override;
size_t getMaxTextureSize() const override;
diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
index e867c6f..b00e870 100644
--- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
@@ -192,6 +192,20 @@
info->addMotionRange(mOrientedRanges.y);
info->addMotionRange(mOrientedRanges.pressure);
+ if (mDeviceMode == DeviceMode::UNSCALED && mSource == AINPUT_SOURCE_TOUCHPAD) {
+ // Populate RELATIVE_X and RELATIVE_Y motion ranges for touchpad capture mode.
+ //
+ // RELATIVE_X and RELATIVE_Y motion ranges should be the largest possible relative
+ // motion, i.e. the hardware dimensions, as the finger could move completely across the
+ // touchpad in one sample cycle.
+ const InputDeviceInfo::MotionRange& x = mOrientedRanges.x;
+ const InputDeviceInfo::MotionRange& y = mOrientedRanges.y;
+ info->addMotionRange(AMOTION_EVENT_AXIS_RELATIVE_X, mSource, -x.max, x.max, x.flat,
+ x.fuzz, x.resolution);
+ info->addMotionRange(AMOTION_EVENT_AXIS_RELATIVE_Y, mSource, -y.max, y.max, y.flat,
+ y.fuzz, y.resolution);
+ }
+
if (mOrientedRanges.haveSize) {
info->addMotionRange(mOrientedRanges.size);
}
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index f56735a..4b0a945 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -7652,6 +7652,20 @@
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
ASSERT_EQ(AINPUT_SOURCE_TOUCHPAD, mapper.getSources());
+ InputDeviceInfo deviceInfo;
+ mDevice->getDeviceInfo(&deviceInfo);
+
+ const InputDeviceInfo::MotionRange* relRangeX =
+ deviceInfo.getMotionRange(AMOTION_EVENT_AXIS_RELATIVE_X, AINPUT_SOURCE_TOUCHPAD);
+ ASSERT_NE(relRangeX, nullptr);
+ ASSERT_EQ(relRangeX->min, -(RAW_X_MAX - RAW_X_MIN));
+ ASSERT_EQ(relRangeX->max, RAW_X_MAX - RAW_X_MIN);
+ const InputDeviceInfo::MotionRange* relRangeY =
+ deviceInfo.getMotionRange(AMOTION_EVENT_AXIS_RELATIVE_Y, AINPUT_SOURCE_TOUCHPAD);
+ ASSERT_NE(relRangeY, nullptr);
+ ASSERT_EQ(relRangeY->min, -(RAW_Y_MAX - RAW_Y_MIN));
+ ASSERT_EQ(relRangeY->max, RAW_Y_MAX - RAW_Y_MIN);
+
// run captured pointer tests - note that this is unscaled, so input listener events should be
// identical to what the hardware sends (accounting for any
// calibration).
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index 54c060b..d460d3d 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -187,7 +187,6 @@
virtual bool hasFrameUpdate() const = 0;
- virtual status_t bindTextureImage() = 0;
virtual status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
nsecs_t expectedPresentTime) = 0;
diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp
index 94cbfa1..7c73df2 100644
--- a/services/surfaceflinger/BufferLayerConsumer.cpp
+++ b/services/surfaceflinger/BufferLayerConsumer.cpp
@@ -153,25 +153,9 @@
if (err != NO_ERROR) {
return err;
}
-
- if (!mRE.useNativeFenceSync()) {
- // Bind the new buffer to the GL texture.
- //
- // Older devices require the "implicit" synchronization provided
- // by glEGLImageTargetTexture2DOES, which this method calls. Newer
- // devices will either call this in Layer::onDraw, or (if it's not
- // a GL-composited layer) not at all.
- err = bindTextureImageLocked();
- }
-
return err;
}
-status_t BufferLayerConsumer::bindTextureImage() {
- Mutex::Autolock lock(mMutex);
- return bindTextureImageLocked();
-}
-
void BufferLayerConsumer::setReleaseFence(const sp<Fence>& fence) {
if (!fence->isValid()) {
return;
@@ -292,17 +276,6 @@
return err;
}
-status_t BufferLayerConsumer::bindTextureImageLocked() {
- ATRACE_CALL();
-
- if (mCurrentTextureBuffer != nullptr && mCurrentTextureBuffer->graphicBuffer() != nullptr) {
- return mRE.bindExternalTextureBuffer(mTexName, mCurrentTextureBuffer->graphicBuffer(),
- mCurrentFence);
- }
-
- return NO_INIT;
-}
-
void BufferLayerConsumer::getTransformMatrix(float mtx[16]) {
Mutex::Autolock lock(mMutex);
memcpy(mtx, mCurrentTransformMatrix, sizeof(mCurrentTransformMatrix));
diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h
index a28902d..dd39214 100644
--- a/services/surfaceflinger/BufferLayerConsumer.h
+++ b/services/surfaceflinger/BufferLayerConsumer.h
@@ -94,9 +94,6 @@
status_t updateTexImage(BufferRejecter* rejecter, nsecs_t expectedPresentTime,
bool* autoRefresh, bool* queuedBuffer, uint64_t maxFrameNumber);
- // See BufferLayerConsumer::bindTextureImageLocked().
- status_t bindTextureImage();
-
// setReleaseFence stores a fence that will signal when the current buffer
// is no longer being read. This fence will be returned to the producer
// when the current buffer is released by updateTexImage(). Multiple
@@ -214,10 +211,6 @@
PendingRelease* pendingRelease = nullptr)
EXCLUDES(mImagesMutex);
- // Binds mTexName and the current buffer to TEXTURE_EXTERNAL target.
- // If the bind succeeds, this calls doFenceWait.
- status_t bindTextureImageLocked();
-
private:
// Utility class for managing GraphicBuffer references into renderengine
class Image {
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 89284f2..97ad212 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -229,10 +229,6 @@
return mQueuedFrames > 0;
}
-status_t BufferQueueLayer::bindTextureImage() {
- return mConsumer->bindTextureImage();
-}
-
status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
nsecs_t expectedPresentTime) {
// This boolean is used to make sure that SurfaceFlinger's shadow copy
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index 1ac0453..7d06128 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -94,7 +94,6 @@
bool hasFrameUpdate() const override;
- status_t bindTextureImage() override;
status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
nsecs_t expectedPresentTime) override;
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 884cc0c..ea1f78c 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -508,13 +508,6 @@
return mCurrentStateModified && (c.buffer != nullptr || c.bgColorLayer != nullptr);
}
-status_t BufferStateLayer::bindTextureImage() {
- const State& s(getDrawingState());
- auto& engine(mFlinger->getRenderEngine());
-
- return engine.bindExternalTextureBuffer(mTextureName, s.buffer, s.acquireFence);
-}
-
status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime,
nsecs_t /*expectedPresentTime*/) {
const State& s(getDrawingState());
@@ -559,20 +552,6 @@
handle->frameNumber = mDrawingState.frameNumber;
}
- if (!SyncFeatures::getInstance().useNativeFenceSync()) {
- // Bind the new buffer to the GL texture.
- //
- // Older devices require the "implicit" synchronization provided
- // by glEGLImageTargetTexture2DOES, which this method calls. Newer
- // devices will either call this in Layer::onDraw, or (if it's not
- // a GL-composited layer) not at all.
- status_t err = bindTextureImage();
- if (err != NO_ERROR) {
- mFlinger->mTimeStats->onDestroy(layerId);
- return BAD_VALUE;
- }
- }
-
mFlinger->mTimeStats->setAcquireFence(layerId, mDrawingState.frameNumber,
std::make_shared<FenceTime>(mDrawingState.acquireFence));
mFlinger->mTimeStats->setLatchTime(layerId, mDrawingState.frameNumber, latchTime);
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 89d9a00..81959ae 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -129,7 +129,6 @@
bool hasFrameUpdate() const override;
- status_t bindTextureImage() override;
status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
nsecs_t expectedPresentTime) override;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index eced6bd..b19f7a5 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2426,7 +2426,15 @@
xSurfaceInset = (xSurfaceInset >= 0) ? std::min(xSurfaceInset, layerBounds.getWidth() / 2) : 0;
ySurfaceInset = (ySurfaceInset >= 0) ? std::min(ySurfaceInset, layerBounds.getHeight() / 2) : 0;
- layerBounds.inset(xSurfaceInset, ySurfaceInset, xSurfaceInset, ySurfaceInset);
+ // inset while protecting from overflow TODO(b/161235021): What is going wrong
+ // in the overflow scenario?
+ {
+ int32_t tmp;
+ if (!__builtin_add_overflow(layerBounds.left, xSurfaceInset, &tmp)) layerBounds.left = tmp;
+ if (!__builtin_sub_overflow(layerBounds.right, xSurfaceInset, &tmp)) layerBounds.right = tmp;
+ if (!__builtin_add_overflow(layerBounds.top, ySurfaceInset, &tmp)) layerBounds.top = tmp;
+ if (!__builtin_sub_overflow(layerBounds.bottom, ySurfaceInset, &tmp)) layerBounds.bottom = tmp;
+ }
// Input coordinate should match the layer bounds.
info.frameLeft = layerBounds.left;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9d35a3f..819f3e2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -509,6 +509,15 @@
}
sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool secure) {
+ // onTransact already checks for some permissions, but adding an additional check here.
+ // This is to ensure that only system and graphics can request to create a secure
+ // display. Secure displays can show secure content so we add an additional restriction on it.
+ const int uid = IPCThreadState::self()->getCallingUid();
+ if (secure && uid != AID_GRAPHICS && uid != AID_SYSTEM) {
+ ALOGE("Only privileged processes can create a secure display");
+ return nullptr;
+ }
+
class DisplayToken : public BBinder {
sp<SurfaceFlinger> flinger;
virtual ~DisplayToken() {
diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp
index e2c038d..7f541e2 100644
--- a/services/surfaceflinger/tests/Credentials_test.cpp
+++ b/services/surfaceflinger/tests/Credentials_test.cpp
@@ -98,26 +98,6 @@
t.setLayer(mBGSurfaceControl, INT_MAX - 3).show(mBGSurfaceControl).apply());
}
- void setupVirtualDisplay() {
- mVirtualDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true);
- const ssize_t displayWidth = 100;
- const ssize_t displayHeight = 100;
-
- // Background surface
- mVirtualSurfaceControl =
- mComposerClient->createSurface(SURFACE_NAME, displayWidth, displayHeight,
- PIXEL_FORMAT_RGBA_8888, 0);
- ASSERT_TRUE(mVirtualSurfaceControl != nullptr);
- ASSERT_TRUE(mVirtualSurfaceControl->isValid());
-
- Transaction t;
- t.setDisplayLayerStack(mVirtualDisplay, 0);
- ASSERT_EQ(NO_ERROR,
- t.setLayer(mVirtualSurfaceControl, INT_MAX - 3)
- .show(mVirtualSurfaceControl)
- .apply());
- }
-
/**
* Sets UID to imitate Graphic's process.
*/
@@ -165,6 +145,10 @@
// Check as a non-supported user.
setBinUID();
ASSERT_EQ(unprivilegedValue, condition());
+
+ // Check as shell since shell has some additional permissions
+ seteuid(AID_SHELL);
+ ASSERT_EQ(unprivilegedValue, condition());
}
};
@@ -262,11 +246,31 @@
}
TEST_F(CredentialsTest, CreateDisplayTest) {
+ // Only graphics and system processes can create a secure display.
std::function<bool()> condition = [=]() {
sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true);
return testDisplay.get() != nullptr;
};
- ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false));
+
+ // Check with root.
+ seteuid(AID_ROOT);
+ ASSERT_FALSE(condition());
+
+ // Check as a Graphics user.
+ setGraphicsUID();
+ ASSERT_TRUE(condition());
+
+ // Check as a system user.
+ setSystemUID();
+ ASSERT_TRUE(condition());
+
+ // Check as a non-supported user.
+ setBinUID();
+ ASSERT_FALSE(condition());
+
+ // Check as shell since shell has some additional permissions
+ seteuid(AID_SHELL);
+ ASSERT_FALSE(condition());
condition = [=]() {
sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, false);
diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
index 8d97f27..8570032 100644
--- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
+++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
@@ -422,7 +422,7 @@
}
void SurfaceInterceptorTest::displayCreation(Transaction&) {
- sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true);
+ sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, false);
SurfaceComposerClient::destroyDisplay(testDisplay);
}
@@ -819,7 +819,7 @@
bool SurfaceInterceptorTest::displayCreationFound(const Increment& increment, bool foundDisplay) {
bool isMatch(increment.display_creation().name() == DISPLAY_NAME.string() &&
- increment.display_creation().is_secure());
+ !increment.display_creation().is_secure());
if (isMatch && !foundDisplay) {
foundDisplay = true;
} else if (isMatch && foundDisplay) {
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index b939b9a..049264e 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -24,6 +24,7 @@
#include <type_traits>
#include <android/hardware/power/Boost.h>
+#include <binder/IPCThreadState.h>
#include <compositionengine/Display.h>
#include <compositionengine/DisplayColorProfile.h>
#include <compositionengine/impl/Display.h>
@@ -37,6 +38,7 @@
#include <gui/mock/GraphicBufferConsumer.h>
#include <gui/mock/GraphicBufferProducer.h>
#include <log/log.h>
+#include <private/android_filesystem_config.h>
#include <renderengine/mock/RenderEngine.h>
#include <ui/DebugUtils.h>
@@ -1230,8 +1232,12 @@
// --------------------------------------------------------------------
// Invocation
-
+ int64_t oldId = IPCThreadState::self()->clearCallingIdentity();
+ // Set the calling identity to graphics so captureDisplay with secure is allowed.
+ IPCThreadState::self()->restoreCallingIdentity(static_cast<int64_t>(AID_GRAPHICS) << 32 |
+ AID_GRAPHICS);
sp<IBinder> displayToken = mFlinger.createDisplay(name, true);
+ IPCThreadState::self()->restoreCallingIdentity(oldId);
// --------------------------------------------------------------------
// Postconditions
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index b024568..2d12451 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -362,6 +362,10 @@
auto flushTransactionQueues() { return mFlinger->flushTransactionQueues(); };
+ auto onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) {
+ return mFlinger->onTransact(code, data, reply, flags);
+ }
+
/* ------------------------------------------------------------------------
* Read-only access to private data to assert post-conditions.
*/