Merge "Update OWNERS file to use COGS component" into main
diff --git a/cmds/atrace/atrace.rc b/cmds/atrace/atrace.rc
index c3cf2c2..fc0801c 100644
--- a/cmds/atrace/atrace.rc
+++ b/cmds/atrace/atrace.rc
@@ -228,10 +228,6 @@
chmod 0666 /sys/kernel/debug/tracing/events/thermal/cdev_update/enable
chmod 0666 /sys/kernel/tracing/events/thermal/cdev_update/enable
-# Tracing disabled by default
- write /sys/kernel/debug/tracing/tracing_on 0
- write /sys/kernel/tracing/tracing_on 0
-
# Read and truncate the kernel trace.
chmod 0666 /sys/kernel/debug/tracing/trace
chmod 0666 /sys/kernel/tracing/trace
@@ -310,11 +306,9 @@
chmod 0666 /sys/kernel/tracing/events/synthetic/suspend_resume_minimal/enable
chmod 0666 /sys/kernel/debug/tracing/events/synthetic/suspend_resume_minimal/enable
-on late-init && property:ro.boot.fastboot.boottrace=enabled
- setprop debug.atrace.tags.enableflags 802922
- setprop persist.traced.enable 0
- write /sys/kernel/debug/tracing/tracing_on 1
- write /sys/kernel/tracing/tracing_on 1
+on late-init && property:ro.boot.fastboot.boottrace=
+ write /sys/kernel/debug/tracing/tracing_on 0
+ write /sys/kernel/tracing/tracing_on 0
# Only create the tracing instance if persist.mm_events.enabled
# Attempting to remove the tracing instance after it has been created
@@ -527,7 +521,6 @@
chmod 0440 /sys/kernel/debug/tracing/hyp/events/hyp/host_mem_abort/id
chmod 0440 /sys/kernel/tracing/hyp/events/hyp/host_mem_abort/id
-
on property:persist.debug.atrace.boottrace=1
start boottrace
@@ -536,10 +529,3 @@
user root
disabled
oneshot
-
-on property:sys.boot_completed=1 && property:ro.boot.fastboot.boottrace=enabled
- setprop debug.atrace.tags.enableflags 0
- setprop persist.traced.enable 1
- write /sys/kernel/debug/tracing/tracing_on 0
- write /sys/kernel/tracing/tracing_on 0
-
diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp
index aa42541..615701c 100644
--- a/cmds/dumpstate/DumpstateUtil.cpp
+++ b/cmds/dumpstate/DumpstateUtil.cpp
@@ -207,6 +207,7 @@
int PropertiesHelper::dry_run_ = -1;
int PropertiesHelper::unroot_ = -1;
int PropertiesHelper::parallel_run_ = -1;
+int PropertiesHelper::strict_run_ = -1;
bool PropertiesHelper::IsUserBuild() {
if (build_type_.empty()) {
@@ -237,6 +238,14 @@
return parallel_run_ == 1;
}
+bool PropertiesHelper::IsStrictRun() {
+ if (strict_run_ == -1) {
+ // Defaults to using stricter timeouts.
+ strict_run_ = android::base::GetBoolProperty("dumpstate.strict_run", true) ? 1 : 0;
+ }
+ return strict_run_ == 1;
+}
+
int DumpFileToFd(int out_fd, const std::string& title, const std::string& path) {
android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC)));
if (fd.get() < 0) {
diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h
index b00c46e..9e955e3 100644
--- a/cmds/dumpstate/DumpstateUtil.h
+++ b/cmds/dumpstate/DumpstateUtil.h
@@ -193,11 +193,19 @@
*/
static bool IsParallelRun();
+ /*
+ * Strict-run mode is determined by the `dumpstate.strict_run` sysprop which
+ * will default to true. This results in shortened timeouts for flaky
+ * sections.
+ */
+ static bool IsStrictRun();
+
private:
static std::string build_type_;
static int dry_run_;
static int unroot_;
static int parallel_run_;
+ static int strict_run_;
};
/*
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index dc0e26b..9444729 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -820,9 +820,12 @@
RunCommandToFd(STDOUT_FILENO, "", {"uptime", "-p"},
CommandOptions::WithTimeout(1).Always().Build());
printf("Bugreport format version: %s\n", version_.c_str());
- printf("Dumpstate info: id=%d pid=%d dry_run=%d parallel_run=%d args=%s bugreport_mode=%s\n",
- id_, pid_, PropertiesHelper::IsDryRun(), PropertiesHelper::IsParallelRun(),
- options_->args.c_str(), options_->bugreport_mode_string.c_str());
+ printf(
+ "Dumpstate info: id=%d pid=%d dry_run=%d parallel_run=%d strict_run=%d args=%s "
+ "bugreport_mode=%s\n",
+ id_, pid_, PropertiesHelper::IsDryRun(), PropertiesHelper::IsParallelRun(),
+ PropertiesHelper::IsStrictRun(), options_->args.c_str(),
+ options_->bugreport_mode_string.c_str());
printf("\n");
}
@@ -1044,7 +1047,8 @@
MYLOGE("Could not open %s to dump incident report.\n", path.c_str());
return;
}
- RunCommandToFd(fd, "", {"incident", "-u"}, CommandOptions::WithTimeout(20).Build());
+ RunCommandToFd(fd, "", {"incident", "-u"},
+ CommandOptions::WithTimeout(PropertiesHelper::IsStrictRun() ? 20 : 120).Build());
bool empty = 0 == lseek(fd, 0, SEEK_END);
if (!empty) {
// Use a different name from "incident.proto"
@@ -1414,12 +1418,12 @@
auto ret = sm->list([&](const auto& interfaces) {
for (const std::string& interface : interfaces) {
std::string cleanName = interface;
- std::replace_if(cleanName.begin(),
- cleanName.end(),
- [](char c) {
- return !isalnum(c) &&
- std::string("@-_:.").find(c) == std::string::npos;
- }, '_');
+ std::replace_if(
+ cleanName.begin(), cleanName.end(),
+ [](char c) {
+ return !isalnum(c) && std::string("@-_.").find(c) == std::string::npos;
+ },
+ '_');
const std::string path = ds.bugreport_internal_dir_ + "/lshal_debug_" + cleanName;
bool empty = false;
@@ -1750,6 +1754,20 @@
RunCommand("SYSTEM PROPERTIES", {"getprop"});
+ DumpFile("SYSTEM BUILD-TIME RELEASE FLAGS", "/system/etc/build_flags.json");
+ DumpFile("SYSTEM_EXT BUILD-TIME RELEASE FLAGS", "/system_ext/etc/build_flags.json");
+ DumpFile("PRODUCT BUILD-TIME RELEASE FLAGS", "/product/etc/build_flags.json");
+ DumpFile("VENDOR BUILD-TIME RELEASE FLAGS", "/vendor/etc/build_flags.json");
+
+ DumpFile("SYSTEM BUILD-TIME ACONFIG FLAGS (check dumpstate build_config for runtime values)",
+ "/system/etc/aconfig_flags.textproto");
+ DumpFile("SYSTEM_EXT BUILD-TIME ACONFIG FLAGS (check dumpstate build_config for runtime"
+ " values)", "/system_ext/etc/aconfig_flags.textproto");
+ DumpFile("PRODUCT BUILD-TIME ACONFIG FLAGS (check dumpstate build_config for runtime values)",
+ "/product/etc/aconfig_flags.textproto");
+ DumpFile("VENDOR BUILD-TIME ACONFIG FLAGS (check dumpstate build_config for runtime values)",
+ "/vendor/etc/aconfig_flags.textproto");
+
RunCommand("STORAGED IO INFO", {"storaged", "-u", "-p"});
RunCommand("FILESYSTEMS & FREE SPACE", {"df"});
@@ -3053,6 +3071,12 @@
MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n");
}
+ if (PropertiesHelper::IsStrictRun()) {
+ MYLOGI(
+ "Running on strict-run mode, which has shorter timeouts "
+ "(to disable, call 'setprop dumpstate.strict_run false')\n");
+ }
+
MYLOGI("dumpstate info: id=%d, args='%s', bugreport_mode= %s bugreport format version: %s\n",
id_, options_->args.c_str(), options_->bugreport_mode_string.c_str(), version_.c_str());
diff --git a/cmds/installd/CrateManager.cpp b/cmds/installd/CrateManager.cpp
index b17cba1..fd1df35 100644
--- a/cmds/installd/CrateManager.cpp
+++ b/cmds/installd/CrateManager.cpp
@@ -29,9 +29,10 @@
#include <sys/xattr.h>
#include <unistd.h>
-#include <fstream>
-#include <string>
#include <utils.h>
+#include <fstream>
+#include <functional>
+#include <string>
#include "utils.h"
diff --git a/cmds/installd/CrateManager.h b/cmds/installd/CrateManager.h
index 1f30b5d..d9b590f 100644
--- a/cmds/installd/CrateManager.h
+++ b/cmds/installd/CrateManager.h
@@ -25,6 +25,7 @@
#include <sys/stat.h>
#include <sys/types.h>
+#include <functional>
#include <optional>
#include <string>
#include <vector>
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index c6132e8..b302f52 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -236,6 +236,16 @@
} \
}
+// we could have tighter checks, but this is only to avoid hard errors. Negative values are defined
+// in UserHandle.java and carry specific meanings that may not be handled by certain APIs here.
+#define ENFORCE_VALID_USER(userId) \
+ { \
+ if (static_cast<uid_t>(std::abs(userId)) >= \
+ std::numeric_limits<uid_t>::max() / AID_USER_OFFSET) { \
+ return error("userId invalid: " + std::to_string(userId)); \
+ } \
+ }
+
#define CHECK_ARGUMENT_UUID(uuid) { \
binder::Status status = checkArgumentUuid((uuid)); \
if (!status.isOk()) { \
@@ -696,6 +706,7 @@
int32_t flags, int32_t appId, int32_t previousAppId, const std::string& seInfo,
int32_t targetSdkVersion, int64_t* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
@@ -790,6 +801,8 @@
binder::Status InstalldNativeService::createSdkSandboxDataPackageDirectory(
const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
int32_t appId, int32_t flags) {
+ ENFORCE_VALID_USER(userId);
+
int32_t sdkSandboxUid = multiuser_get_sdk_sandbox_uid(userId, appId);
if (sdkSandboxUid == -1) {
// There no valid sdk sandbox process for this app. Skip creation of data directory
@@ -828,6 +841,7 @@
int32_t flags, int32_t appId, int32_t previousAppId, const std::string& seInfo,
int32_t targetSdkVersion, int64_t* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -839,6 +853,7 @@
const android::os::CreateAppDataArgs& args,
android::os::CreateAppDataResult* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(args.userId);
// Locking is performed depeer in the callstack.
int64_t ceDataInode = -1;
@@ -854,6 +869,10 @@
const std::vector<android::os::CreateAppDataArgs>& args,
std::vector<android::os::CreateAppDataResult>* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ for (const auto& arg : args) {
+ ENFORCE_VALID_USER(arg.userId);
+ }
+
// Locking is performed depeer in the callstack.
std::vector<android::os::CreateAppDataResult> results;
@@ -868,6 +887,7 @@
binder::Status InstalldNativeService::reconcileSdkData(
const android::os::ReconcileSdkDataArgs& args) {
+ ENFORCE_VALID_USER(args.userId);
// Locking is performed depeer in the callstack.
return reconcileSdkData(args.uuid, args.packageName, args.subDirNames, args.userId, args.appId,
@@ -891,6 +911,7 @@
int userId, int appId, int previousAppId,
const std::string& seInfo, int flags) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -974,6 +995,7 @@
binder::Status InstalldNativeService::migrateAppData(const std::optional<std::string>& uuid,
const std::string& packageName, int32_t userId, int32_t flags) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -1041,6 +1063,7 @@
binder::Status InstalldNativeService::clearAppData(const std::optional<std::string>& uuid,
const std::string& packageName, int32_t userId, int32_t flags, int64_t ceDataInode) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -1132,6 +1155,7 @@
binder::Status InstalldNativeService::clearSdkSandboxDataPackageDirectory(
const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
int32_t flags) {
+ ENFORCE_VALID_USER(userId);
const char* uuid_ = uuid ? uuid->c_str() : nullptr;
const char* pkgname = packageName.c_str();
@@ -1218,6 +1242,7 @@
binder::Status InstalldNativeService::destroyAppData(const std::optional<std::string>& uuid,
const std::string& packageName, int32_t userId, int32_t flags, int64_t ceDataInode) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -1288,6 +1313,8 @@
binder::Status InstalldNativeService::destroySdkSandboxDataPackageDirectory(
const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
int32_t flags) {
+ ENFORCE_VALID_USER(userId);
+
const char* uuid_ = uuid ? uuid->c_str() : nullptr;
const char* pkgname = packageName.c_str();
@@ -1435,6 +1462,7 @@
int32_t userId, int32_t snapshotId,
int32_t storageFlags, int64_t* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -1569,6 +1597,7 @@
const int32_t appId, const std::string& seInfo, const int32_t userId,
const int32_t snapshotId, int32_t storageFlags) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -1641,6 +1670,7 @@
const int32_t userId, const int64_t ceSnapshotInode, const int32_t snapshotId,
int32_t storageFlags) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -1674,6 +1704,7 @@
const std::optional<std::string>& volumeUuid, const int32_t userId,
const std::vector<int32_t>& retainSnapshotIds) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
LOCK_USER();
@@ -1864,6 +1895,7 @@
binder::Status InstalldNativeService::createUserData(const std::optional<std::string>& uuid,
int32_t userId, int32_t userSerial ATTRIBUTE_UNUSED, int32_t flags) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
LOCK_USER();
@@ -1884,6 +1916,7 @@
binder::Status InstalldNativeService::destroyUserData(const std::optional<std::string>& uuid,
int32_t userId, int32_t flags) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
LOCK_USER();
@@ -2671,6 +2704,7 @@
int32_t userId, int32_t flags, const std::vector<int32_t>& appIds,
std::vector<int64_t>* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
// NOTE: Locking is relaxed on this method, since it's limited to
// read-only measurements without mutation.
@@ -2806,6 +2840,7 @@
int32_t userId, int32_t flags, const std::vector<int32_t>& appIds,
std::vector<int64_t>* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
// NOTE: Locking is relaxed on this method, since it's limited to
// read-only measurements without mutation.
@@ -2926,6 +2961,7 @@
const std::vector<std::string>& packageNames, int32_t userId,
std::optional<std::vector<std::optional<CrateMetadata>>>* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
for (const auto& packageName : packageNames) {
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
@@ -2975,6 +3011,7 @@
const std::optional<std::string>& uuid, int32_t userId,
std::optional<std::vector<std::optional<CrateMetadata>>>* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
#ifdef ENABLE_STORAGE_CRATES
LOCK_USER();
@@ -3018,6 +3055,7 @@
binder::Status InstalldNativeService::setAppQuota(const std::optional<std::string>& uuid,
int32_t userId, int32_t appId, int64_t cacheQuota) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
std::lock_guard<std::recursive_mutex> lock(mQuotasLock);
@@ -3261,6 +3299,7 @@
const std::string& packageName, int32_t userId, int32_t flags, int32_t appId,
const std::string& seInfo) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
LOCK_PACKAGE_USER();
@@ -3271,6 +3310,7 @@
const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
int32_t flags, int32_t appId, const std::string& seInfo) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
@@ -3302,6 +3342,7 @@
const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
int32_t flags, int32_t appId, const std::string& seInfo) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
@@ -3753,6 +3794,7 @@
int32_t userId, int32_t appId, const std::string& profileName, const std::string& codePath,
const std::optional<std::string>& dexMetadata, bool* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ ENFORCE_VALID_USER(userId);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
CHECK_ARGUMENT_PATH(codePath);
LOCK_PACKAGE_USER();
@@ -3775,6 +3817,7 @@
binder::Status InstalldNativeService::cleanupInvalidPackageDirs(
const std::optional<std::string>& uuid, int32_t userId, int32_t flags) {
+ ENFORCE_VALID_USER(userId);
const char* uuid_cstr = uuid ? uuid->c_str() : nullptr;
if (flags & FLAG_STORAGE_CE) {
diff --git a/cmds/installd/dexopt.h b/cmds/installd/dexopt.h
index 5cf402c..df02588 100644
--- a/cmds/installd/dexopt.h
+++ b/cmds/installd/dexopt.h
@@ -18,6 +18,7 @@
#define DEXOPT_H_
#include "installd_constants.h"
+#include "unique_file.h"
#include <sys/types.h>
@@ -156,6 +157,10 @@
// artifacts.
int get_odex_visibility(const char* apk_path, const char* instruction_set, const char* oat_dir);
+UniqueFile maybe_open_reference_profile(const std::string& pkgname, const std::string& dex_path,
+ const char* profile_name, bool profile_guided,
+ bool is_public, int uid, bool is_secondary_dex);
+
} // namespace installd
} // namespace android
diff --git a/cmds/installd/otapreopt.cpp b/cmds/installd/otapreopt.cpp
index 7cabdb0..818fd80 100644
--- a/cmds/installd/otapreopt.cpp
+++ b/cmds/installd/otapreopt.cpp
@@ -14,20 +14,21 @@
** limitations under the License.
*/
-#include <algorithm>
#include <inttypes.h>
-#include <limits>
-#include <random>
-#include <regex>
#include <selinux/android.h>
#include <selinux/avc.h>
#include <stdlib.h>
#include <string.h>
#include <sys/capability.h>
+#include <sys/mman.h>
#include <sys/prctl.h>
#include <sys/stat.h>
-#include <sys/mman.h>
#include <sys/wait.h>
+#include <algorithm>
+#include <iterator>
+#include <limits>
+#include <random>
+#include <regex>
#include <android-base/logging.h>
#include <android-base/macros.h>
@@ -47,6 +48,7 @@
#include "otapreopt_parameters.h"
#include "otapreopt_utils.h"
#include "system_properties.h"
+#include "unique_file.h"
#include "utils.h"
#ifndef LOG_TAG
@@ -87,6 +89,9 @@
static_assert(DEXOPT_MASK == (0x3dfe | DEXOPT_IDLE_BACKGROUND_JOB),
"DEXOPT_MASK unexpected.");
+constexpr const char* kAotCompilerFilters[]{
+ "space-profile", "space", "speed-profile", "speed", "everything-profile", "everything",
+};
template<typename T>
static constexpr bool IsPowerOfTwo(T x) {
@@ -415,6 +420,32 @@
return (strcmp(arg, "!") == 0) ? nullptr : arg;
}
+ bool IsAotCompilation() const {
+ if (std::find(std::begin(kAotCompilerFilters), std::end(kAotCompilerFilters),
+ parameters_.compiler_filter) == std::end(kAotCompilerFilters)) {
+ return false;
+ }
+
+ int dexopt_flags = parameters_.dexopt_flags;
+ bool profile_guided = (dexopt_flags & DEXOPT_PROFILE_GUIDED) != 0;
+ bool is_secondary_dex = (dexopt_flags & DEXOPT_SECONDARY_DEX) != 0;
+ bool is_public = (dexopt_flags & DEXOPT_PUBLIC) != 0;
+
+ if (profile_guided) {
+ UniqueFile reference_profile =
+ maybe_open_reference_profile(parameters_.pkgName, parameters_.apk_path,
+ parameters_.profile_name, profile_guided,
+ is_public, parameters_.uid, is_secondary_dex);
+ struct stat sbuf;
+ if (reference_profile.fd() == -1 ||
+ (fstat(reference_profile.fd(), &sbuf) != -1 && sbuf.st_size == 0)) {
+ return false;
+ }
+ }
+
+ return true;
+ }
+
bool ShouldSkipPreopt() const {
// There's one thing we have to be careful about: we may/will be asked to compile an app
// living in the system image. This may be a valid request - if the app wasn't compiled,
@@ -439,9 +470,12 @@
// (This is ugly as it's the only thing where we need to understand the contents
// of parameters_, but it beats postponing the decision or using the call-
// backs to do weird things.)
+
+ // In addition, no need to preopt for "verify". The existing vdex files in the OTA package
+ // and the /data partition will still be usable after the OTA update is applied.
const char* apk_path = parameters_.apk_path;
CHECK(apk_path != nullptr);
- if (StartsWith(apk_path, android_root_)) {
+ if (StartsWith(apk_path, android_root_) || !IsAotCompilation()) {
const char* last_slash = strrchr(apk_path, '/');
if (last_slash != nullptr) {
std::string path(apk_path, last_slash - apk_path + 1);
@@ -471,13 +505,18 @@
// TODO(calin): embed the profile name in the parameters.
int Dexopt() {
std::string error;
+
+ int dexopt_flags = parameters_.dexopt_flags;
+ // Make sure dex2oat is run with background priority.
+ dexopt_flags |= DEXOPT_BOOTCOMPLETE | DEXOPT_IDLE_BACKGROUND_JOB;
+
int res = dexopt(parameters_.apk_path,
parameters_.uid,
parameters_.pkgName,
parameters_.instruction_set,
parameters_.dexopt_needed,
parameters_.oat_dir,
- parameters_.dexopt_flags,
+ dexopt_flags,
parameters_.compiler_filter,
parameters_.volume_uuid,
parameters_.shared_libraries,
diff --git a/cmds/installd/otapreopt_chroot.cpp b/cmds/installd/otapreopt_chroot.cpp
index c86993c..c40caf5 100644
--- a/cmds/installd/otapreopt_chroot.cpp
+++ b/cmds/installd/otapreopt_chroot.cpp
@@ -19,9 +19,12 @@
#include <sys/mount.h>
#include <sys/stat.h>
#include <sys/wait.h>
+#include <unistd.h>
+#include <algorithm>
#include <array>
#include <fstream>
+#include <iostream>
#include <sstream>
#include <android-base/file.h>
@@ -29,6 +32,7 @@
#include <android-base/macros.h>
#include <android-base/scopeguard.h>
#include <android-base/stringprintf.h>
+#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <libdm/dm.h>
#include <selinux/android.h>
@@ -37,7 +41,7 @@
#include "otapreopt_utils.h"
#ifndef LOG_TAG
-#define LOG_TAG "otapreopt"
+#define LOG_TAG "otapreopt_chroot"
#endif
using android::base::StringPrintf;
@@ -49,20 +53,22 @@
// so just try the possibilities one by one.
static constexpr std::array kTryMountFsTypes = {"ext4", "erofs"};
-static void CloseDescriptor(int fd) {
- if (fd >= 0) {
- int result = close(fd);
- UNUSED(result); // Ignore result. Printing to logcat will open a new descriptor
- // that we do *not* want.
- }
-}
-
static void CloseDescriptor(const char* descriptor_string) {
int fd = -1;
std::istringstream stream(descriptor_string);
stream >> fd;
if (!stream.fail()) {
- CloseDescriptor(fd);
+ if (fd >= 0) {
+ if (close(fd) < 0) {
+ PLOG(ERROR) << "Failed to close " << fd;
+ }
+ }
+ }
+}
+
+static void SetCloseOnExec(int fd) {
+ if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) {
+ PLOG(ERROR) << "Failed to set FD_CLOEXEC on " << fd;
}
}
@@ -129,24 +135,39 @@
}
// Entry for otapreopt_chroot. Expected parameters are:
-// [cmd] [status-fd] [target-slot] "dexopt" [dexopt-params]
-// The file descriptor denoted by status-fd will be closed. The rest of the parameters will
-// be passed on to otapreopt in the chroot.
+//
+// [cmd] [status-fd] [target-slot-suffix]
+//
+// The file descriptor denoted by status-fd will be closed. Dexopt commands on
+// the form
+//
+// "dexopt" [dexopt-params]
+//
+// are then read from stdin until EOF and passed on to /system/bin/otapreopt one
+// by one. After each call a line with the current command count is written to
+// stdout and flushed.
static int otapreopt_chroot(const int argc, char **arg) {
// Validate arguments
- // We need the command, status channel and target slot, at a minimum.
- if(argc < 3) {
- PLOG(ERROR) << "Not enough arguments.";
+ if (argc == 2 && std::string_view(arg[1]) == "--version") {
+ // Accept a single --version flag, to allow the script to tell this binary
+ // from the earlier one.
+ std::cout << "2" << std::endl;
+ return 0;
+ }
+ if (argc != 3) {
+ LOG(ERROR) << "Wrong number of arguments: " << argc;
exit(208);
}
- // Close all file descriptors. They are coming from the caller, we do not want to pass them
- // on across our fork/exec into a different domain.
- // 1) Default descriptors.
- CloseDescriptor(STDIN_FILENO);
- CloseDescriptor(STDOUT_FILENO);
- CloseDescriptor(STDERR_FILENO);
- // 2) The status channel.
- CloseDescriptor(arg[1]);
+ const char* status_fd = arg[1];
+ const char* slot_suffix = arg[2];
+
+ // Set O_CLOEXEC on standard fds. They are coming from the caller, we do not
+ // want to pass them on across our fork/exec into a different domain.
+ SetCloseOnExec(STDIN_FILENO);
+ SetCloseOnExec(STDOUT_FILENO);
+ SetCloseOnExec(STDERR_FILENO);
+ // Close the status channel.
+ CloseDescriptor(status_fd);
// We need to run the otapreopt tool from the postinstall partition. As such, set up a
// mount namespace and change root.
@@ -185,20 +206,20 @@
// 2) We're in a mount namespace here, so when we die, this will be cleaned up.
// 3) Ignore errors. Printing anything at this stage will open a file descriptor
// for logging.
- if (!ValidateTargetSlotSuffix(arg[2])) {
- LOG(ERROR) << "Target slot suffix not legal: " << arg[2];
+ if (!ValidateTargetSlotSuffix(slot_suffix)) {
+ LOG(ERROR) << "Target slot suffix not legal: " << slot_suffix;
exit(207);
}
- TryExtraMount("vendor", arg[2], "/postinstall/vendor");
+ TryExtraMount("vendor", slot_suffix, "/postinstall/vendor");
// Try to mount the product partition. update_engine doesn't do this for us, but we
// want it for product APKs. Same notes as vendor above.
- TryExtraMount("product", arg[2], "/postinstall/product");
+ TryExtraMount("product", slot_suffix, "/postinstall/product");
// Try to mount the system_ext partition. update_engine doesn't do this for
// us, but we want it for system_ext APKs. Same notes as vendor and product
// above.
- TryExtraMount("system_ext", arg[2], "/postinstall/system_ext");
+ TryExtraMount("system_ext", slot_suffix, "/postinstall/system_ext");
constexpr const char* kPostInstallLinkerconfig = "/postinstall/linkerconfig";
// Try to mount /postinstall/linkerconfig. we will set it up after performing the chroot
@@ -329,30 +350,37 @@
exit(218);
}
- // Now go on and run otapreopt.
+ // Now go on and read dexopt lines from stdin and pass them on to otapreopt.
- // Incoming: cmd + status-fd + target-slot + cmd... | Incoming | = argc
- // Outgoing: cmd + target-slot + cmd... | Outgoing | = argc - 1
- std::vector<std::string> cmd;
- cmd.reserve(argc);
- cmd.push_back("/system/bin/otapreopt");
+ int count = 1;
+ for (std::array<char, 1000> linebuf;
+ std::cin.clear(), std::cin.getline(&linebuf[0], linebuf.size()); ++count) {
+ // Subtract one from gcount() since getline() counts the newline.
+ std::string line(&linebuf[0], std::cin.gcount() - 1);
- // The first parameter is the status file descriptor, skip.
- for (size_t i = 2; i < static_cast<size_t>(argc); ++i) {
- cmd.push_back(arg[i]);
+ if (std::cin.fail()) {
+ LOG(ERROR) << "Command exceeds max length " << linebuf.size() << " - skipped: " << line;
+ continue;
+ }
+
+ std::vector<std::string> tokenized_line = android::base::Tokenize(line, " ");
+ std::vector<std::string> cmd{"/system/bin/otapreopt", slot_suffix};
+ std::move(tokenized_line.begin(), tokenized_line.end(), std::back_inserter(cmd));
+
+ LOG(INFO) << "Command " << count << ": " << android::base::Join(cmd, " ");
+
+ // Fork and execute otapreopt in its own process.
+ std::string error_msg;
+ bool exec_result = Exec(cmd, &error_msg);
+ if (!exec_result) {
+ LOG(ERROR) << "Running otapreopt failed: " << error_msg;
+ }
+
+ // Print the count to stdout and flush to indicate progress.
+ std::cout << count << std::endl;
}
- // Fork and execute otapreopt in its own process.
- std::string error_msg;
- bool exec_result = Exec(cmd, &error_msg);
- if (!exec_result) {
- LOG(ERROR) << "Running otapreopt failed: " << error_msg;
- }
-
- if (!exec_result) {
- exit(213);
- }
-
+ LOG(INFO) << "No more dexopt commands";
return 0;
}
diff --git a/cmds/installd/otapreopt_script.sh b/cmds/installd/otapreopt_script.sh
index db5c34e..28bd793 100644
--- a/cmds/installd/otapreopt_script.sh
+++ b/cmds/installd/otapreopt_script.sh
@@ -16,7 +16,9 @@
# limitations under the License.
#
-# This script will run as a postinstall step to drive otapreopt.
+# This script runs as a postinstall step to drive otapreopt. It comes with the
+# OTA package, but runs /system/bin/otapreopt_chroot in the (old) active system
+# image. See system/extras/postinst/postinst.sh for some docs.
TARGET_SLOT="$1"
STATUS_FD="$2"
@@ -31,12 +33,11 @@
BOOT_COMPLETE=$(getprop $BOOT_PROPERTY_NAME)
if [ "$BOOT_COMPLETE" != "1" ] ; then
- echo "Error: boot-complete not detected."
+ echo "$0: Error: boot-complete not detected."
# We must return 0 to not block sideload.
exit 0
fi
-
# Compute target slot suffix.
# TODO: Once bootctl is not restricted, we should query from there. Or get this from
# update_engine as a parameter.
@@ -45,45 +46,63 @@
elif [ "$TARGET_SLOT" = "1" ] ; then
TARGET_SLOT_SUFFIX="_b"
else
- echo "Unknown target slot $TARGET_SLOT"
+ echo "$0: Unknown target slot $TARGET_SLOT"
exit 1
fi
+if [ "$(/system/bin/otapreopt_chroot --version)" != 2 ]; then
+ # We require an updated chroot wrapper that reads dexopt commands from stdin.
+ # Even if we kept compat with the old binary, the OTA preopt wouldn't work due
+ # to missing sepolicy rules, so there's no use spending time trying to dexopt
+ # (b/291974157).
+ echo "$0: Current system image is too old to work with OTA preopt - skipping."
+ exit 0
+fi
PREPARE=$(cmd otadexopt prepare)
# Note: Ignore preparation failures. Step and done will fail and exit this.
# This is necessary to support suspends - the OTA service will keep
# the state around for us.
-PROGRESS=$(cmd otadexopt progress)
-print -u${STATUS_FD} "global_progress $PROGRESS"
-
-i=0
-while ((i<MAXIMUM_PACKAGES)) ; do
+# Create an array with all dexopt commands in advance, to know how many there are.
+otadexopt_cmds=()
+while (( ${#otadexopt_cmds[@]} < MAXIMUM_PACKAGES )) ; do
DONE=$(cmd otadexopt done)
if [ "$DONE" = "OTA complete." ] ; then
break
fi
-
- DEXOPT_PARAMS=$(cmd otadexopt next)
-
- /system/bin/otapreopt_chroot $STATUS_FD $TARGET_SLOT_SUFFIX $DEXOPT_PARAMS >&- 2>&-
-
- PROGRESS=$(cmd otadexopt progress)
- print -u${STATUS_FD} "global_progress $PROGRESS"
-
- sleep 1
- i=$((i+1))
+ otadexopt_cmds+=("$(cmd otadexopt next)")
done
DONE=$(cmd otadexopt done)
+cmd otadexopt cleanup
+
+echo "$0: Using streaming otapreopt_chroot on ${#otadexopt_cmds[@]} packages"
+
+function print_otadexopt_cmds {
+ for cmd in "${otadexopt_cmds[@]}" ; do
+ print "$cmd"
+ done
+}
+
+function report_progress {
+ while read count ; do
+ # mksh can't do floating point arithmetic, so emulate a fixed point calculation.
+ (( permilles = 1000 * count / ${#otadexopt_cmds[@]} ))
+ printf 'global_progress %d.%03d\n' $((permilles / 1000)) $((permilles % 1000)) >&${STATUS_FD}
+ done
+}
+
+print_otadexopt_cmds | \
+ /system/bin/otapreopt_chroot $STATUS_FD $TARGET_SLOT_SUFFIX | \
+ report_progress
+
if [ "$DONE" = "OTA incomplete." ] ; then
- echo "Incomplete."
+ echo "$0: Incomplete."
else
- echo "Complete or error."
+ echo "$0: Complete or error."
fi
print -u${STATUS_FD} "global_progress 1.0"
-cmd otadexopt cleanup
exit 0
diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h
index ecea1d2..c43fdbd 100644
--- a/cmds/installd/utils.h
+++ b/cmds/installd/utils.h
@@ -18,6 +18,7 @@
#ifndef UTILS_H_
#define UTILS_H_
+#include <functional>
#include <string>
#include <vector>
diff --git a/cmds/servicemanager/Android.bp b/cmds/servicemanager/Android.bp
index fb69513..d73a30b 100644
--- a/cmds/servicemanager/Android.bp
+++ b/cmds/servicemanager/Android.bp
@@ -93,22 +93,9 @@
libfuzzer_options: [
"max_len=50000",
],
- },
-}
-
-// Adding this new fuzzer to test the corpus generated by record_binder
-cc_fuzz {
- name: "servicemanager_test_fuzzer",
- defaults: [
- "servicemanager_defaults",
- "service_fuzzer_defaults",
- ],
- host_supported: true,
- srcs: ["fuzzers/ServiceManagerTestFuzzer.cpp"],
- fuzz_config: {
- libfuzzer_options: [
- "max_len=50000",
+ cc: [
+ "smoreland@google.com",
+ "waghpawan@google.com",
],
},
- corpus: ["fuzzers/servicemamanager_fuzzer_corpus/*"],
}
diff --git a/cmds/servicemanager/fuzzers/ServiceManagerTestFuzzer.cpp b/cmds/servicemanager/fuzzers/ServiceManagerTestFuzzer.cpp
deleted file mode 100644
index e19b6eb..0000000
--- a/cmds/servicemanager/fuzzers/ServiceManagerTestFuzzer.cpp
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- * Copyright (C) 2023 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 <fuzzbinder/libbinder_driver.h>
-#include <utils/StrongPointer.h>
-
-#include "Access.h"
-#include "ServiceManager.h"
-
-using ::android::Access;
-using ::android::Parcel;
-using ::android::ServiceManager;
-using ::android::sp;
-
-extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
- FuzzedDataProvider provider(data, size);
- auto accessPtr = std::make_unique<Access>();
- auto serviceManager = sp<ServiceManager>::make(std::move(accessPtr));
-
- // Reserved bytes
- provider.ConsumeBytes<uint8_t>(8);
- uint32_t code = provider.ConsumeIntegral<uint32_t>();
- uint32_t flag = provider.ConsumeIntegral<uint32_t>();
- std::vector<uint8_t> parcelData = provider.ConsumeRemainingBytes<uint8_t>();
-
- Parcel inputParcel;
- inputParcel.setData(parcelData.data(), parcelData.size());
-
- Parcel reply;
- serviceManager->transact(code, inputParcel, &reply, flag);
-
- serviceManager->clear();
-
- return 0;
-}
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_1 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_1
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_1
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_10 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_10
deleted file mode 100644
index 07319f8..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_10
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_11 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_11
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_11
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_12 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_12
deleted file mode 100644
index 07319f8..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_12
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_13 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_13
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_13
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_14 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_14
deleted file mode 100644
index 07319f8..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_14
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_15 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_15
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_15
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_16 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_16
deleted file mode 100644
index 07319f8..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_16
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_17 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_17
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_17
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_18 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_18
deleted file mode 100644
index 88ad474..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_18
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_19 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_19
deleted file mode 100644
index fae15a2..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_19
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_2 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_2
deleted file mode 100644
index e69ab49..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_2
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_20 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_20
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_20
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_21 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_21
deleted file mode 100644
index 88ad474..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_21
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_22 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_22
deleted file mode 100644
index fae15a2..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_22
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_23 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_23
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_23
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_24 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_24
deleted file mode 100644
index 88ad474..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_24
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_25 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_25
deleted file mode 100644
index fae15a2..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_25
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_26 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_26
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_26
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_27 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_27
deleted file mode 100644
index 88ad474..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_27
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_28 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_28
deleted file mode 100644
index fae15a2..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_28
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_29 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_29
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_29
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_3 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_3
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_3
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_30 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_30
deleted file mode 100644
index 88ad474..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_30
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_31 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_31
deleted file mode 100644
index fae15a2..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_31
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_32 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_32
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_32
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_33 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_33
deleted file mode 100644
index 88ad474..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_33
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_34 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_34
deleted file mode 100644
index fae15a2..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_34
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_35 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_35
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_35
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_36 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_36
deleted file mode 100644
index 88ad474..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_36
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_37 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_37
deleted file mode 100644
index fae15a2..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_37
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_38 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_38
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_38
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_39 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_39
deleted file mode 100644
index b326907..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_39
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_4 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_4
deleted file mode 100644
index 05b27bf..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_4
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_40 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_40
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_40
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_41 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_41
deleted file mode 100644
index b326907..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_41
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_42 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_42
deleted file mode 100644
index cdaa1f0..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_42
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_43 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_43
deleted file mode 100644
index ff0941b..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_43
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_44 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_44
deleted file mode 100644
index cdaa1f0..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_44
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_45 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_45
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_45
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_46 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_46
deleted file mode 100644
index 7e5f948..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_46
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_5 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_5
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_5
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_6 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_6
deleted file mode 100644
index 07319f8..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_6
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_7 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_7
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_7
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_8 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_8
deleted file mode 100644
index 07319f8..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_8
+++ /dev/null
Binary files differ
diff --git a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_9 b/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_9
deleted file mode 100644
index 39e5104..0000000
--- a/cmds/servicemanager/fuzzers/servicemanager_fuzzer_corpus/Transaction_9
+++ /dev/null
Binary files differ
diff --git a/data/etc/Android.bp b/data/etc/Android.bp
index a737bd3..92dc46e 100644
--- a/data/etc/Android.bp
+++ b/data/etc/Android.bp
@@ -173,6 +173,12 @@
}
prebuilt_etc {
+ name: "android.hardware.threadnetwork.prebuilt.xml",
+ src: "android.hardware.threadnetwork.xml",
+ defaults: ["frameworks_native_data_etc_defaults"],
+}
+
+prebuilt_etc {
name: "android.hardware.usb.accessory.prebuilt.xml",
src: "android.hardware.usb.accessory.xml",
defaults: ["frameworks_native_data_etc_defaults"],
diff --git a/data/etc/android.hardware.threadnetwork.xml b/data/etc/android.hardware.threadnetwork.xml
new file mode 100644
index 0000000..9cbdc90
--- /dev/null
+++ b/data/etc/android.hardware.threadnetwork.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2023 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.
+-->
+<!-- Adds the feature indicating support for the ThreadNetwork API -->
+<permissions>
+ <feature name="android.hardware.threadnetwork" />
+</permissions>
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index deff76b..f634c1d 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -190,6 +190,9 @@
"-performance-move-const-arg", // b/273486801
"portability*",
],
+ lto: {
+ thin: true,
+ },
}
cc_library_headers {
diff --git a/libs/binder/ndk/.clang-format b/libs/binder/ndk/.clang-format
index 9a9d936..6077414 100644
--- a/libs/binder/ndk/.clang-format
+++ b/libs/binder/ndk/.clang-format
@@ -2,9 +2,7 @@
ColumnLimit: 100
IndentWidth: 4
ContinuationIndentWidth: 8
-PointerAlignment: Left
TabWidth: 4
AllowShortFunctionsOnASingleLine: Inline
PointerAlignment: Left
-TabWidth: 4
UseTab: Never
diff --git a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
index d6937c2..ed53891 100644
--- a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
@@ -115,17 +115,29 @@
*/
AIBinder** getR() { return &mBinder; }
- bool operator!=(const SpAIBinder& rhs) const { return get() != rhs.get(); }
- bool operator<(const SpAIBinder& rhs) const { return get() < rhs.get(); }
- bool operator<=(const SpAIBinder& rhs) const { return get() <= rhs.get(); }
- bool operator==(const SpAIBinder& rhs) const { return get() == rhs.get(); }
- bool operator>(const SpAIBinder& rhs) const { return get() > rhs.get(); }
- bool operator>=(const SpAIBinder& rhs) const { return get() >= rhs.get(); }
-
private:
AIBinder* mBinder = nullptr;
};
+#define SP_AIBINDER_COMPARE(_op_) \
+ static inline bool operator _op_(const SpAIBinder& lhs, const SpAIBinder& rhs) { \
+ return lhs.get() _op_ rhs.get(); \
+ } \
+ static inline bool operator _op_(const SpAIBinder& lhs, const AIBinder* rhs) { \
+ return lhs.get() _op_ rhs; \
+ } \
+ static inline bool operator _op_(const AIBinder* lhs, const SpAIBinder& rhs) { \
+ return lhs _op_ rhs.get(); \
+ }
+
+SP_AIBINDER_COMPARE(!=)
+SP_AIBINDER_COMPARE(<)
+SP_AIBINDER_COMPARE(<=)
+SP_AIBINDER_COMPARE(==)
+SP_AIBINDER_COMPARE(>)
+SP_AIBINDER_COMPARE(>=)
+#undef SP_AIBINDER_COMPARE
+
namespace impl {
/**
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index 27ce615..25b8e97 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -377,18 +377,24 @@
}
TEST(NdkBinder, GetTestServiceStressTest) {
- // libbinder has some complicated logic to make sure only one instance of
- // ABpBinder is associated with each binder.
-
constexpr size_t kNumThreads = 10;
constexpr size_t kNumCalls = 1000;
std::vector<std::thread> threads;
+ // this is not a lazy service, but we must make sure that it's started before calling
+ // checkService on it, since the other process serving it might not be started yet.
+ {
+ // getService, not waitForService, to take advantage of timeout
+ auto binder = ndk::SpAIBinder(AServiceManager_getService(IFoo::kSomeInstanceName));
+ ASSERT_NE(nullptr, binder.get());
+ }
+
for (size_t i = 0; i < kNumThreads; i++) {
threads.push_back(std::thread([&]() {
for (size_t j = 0; j < kNumCalls; j++) {
auto binder =
ndk::SpAIBinder(AServiceManager_checkService(IFoo::kSomeInstanceName));
+ ASSERT_NE(nullptr, binder.get());
EXPECT_EQ(STATUS_OK, AIBinder_ping(binder.get()));
}
}));
@@ -755,9 +761,9 @@
// local
ndk::SharedRefBase::make<MyBinderNdkUnitTest>()->asBinder()}) {
// convert to platform binder
- EXPECT_NE(binder.get(), nullptr);
+ EXPECT_NE(binder, nullptr);
sp<IBinder> platformBinder = AIBinder_toPlatformBinder(binder.get());
- EXPECT_NE(platformBinder.get(), nullptr);
+ EXPECT_NE(platformBinder, nullptr);
auto proxy = interface_cast<IBinderNdkUnitTest>(platformBinder);
EXPECT_NE(proxy, nullptr);
@@ -768,7 +774,7 @@
// convert back
ndk::SpAIBinder backBinder = ndk::SpAIBinder(AIBinder_fromPlatformBinder(platformBinder));
- EXPECT_EQ(backBinder.get(), binder.get());
+ EXPECT_EQ(backBinder, binder);
}
}
diff --git a/libs/binder/rust/Android.bp b/libs/binder/rust/Android.bp
index d36ebac..672d6cf 100644
--- a/libs/binder/rust/Android.bp
+++ b/libs/binder/rust/Android.bp
@@ -97,34 +97,12 @@
crate_name: "binder_ndk_bindgen",
wrapper_src: "sys/BinderBindings.hpp",
source_stem: "bindings",
- bindgen_flags: [
+ bindgen_flag_files: [
// Unfortunately the only way to specify the rust_non_exhaustive enum
// style for a type is to make it the default
- "--default-enum-style",
- "rust_non_exhaustive",
// and then specify constified enums for the enums we don't want
// rustified
- "--constified-enum",
- "android::c_interface::consts::.*",
-
- "--allowlist-type",
- "android::c_interface::.*",
- "--allowlist-type",
- "AStatus",
- "--allowlist-type",
- "AIBinder_Class",
- "--allowlist-type",
- "AIBinder",
- "--allowlist-type",
- "AIBinder_Weak",
- "--allowlist-type",
- "AIBinder_DeathRecipient",
- "--allowlist-type",
- "AParcel",
- "--allowlist-type",
- "binder_status_t",
- "--allowlist-function",
- ".*",
+ "libbinder_ndk_bindgen_flags.txt",
],
shared_libs: [
"libbinder_ndk",
diff --git a/libs/binder/rust/binder_tokio/lib.rs b/libs/binder/rust/binder_tokio/lib.rs
index 2d2bf7c..1dc0b24 100644
--- a/libs/binder/rust/binder_tokio/lib.rs
+++ b/libs/binder/rust/binder_tokio/lib.rs
@@ -103,7 +103,12 @@
//
// This shouldn't cause issues with blocking the thread as only one task will run in a
// call to `block_on`, so there aren't other tasks to block.
- let result = spawn_me();
+ //
+ // If the `block_in_place` call fails, then you are driving a current-thread runtime on
+ // the binder threadpool. Instead, it is recommended to use `TokioRuntime<Handle>` when
+ // the runtime is a current-thread runtime, as the current-thread runtime can be driven
+ // only by `Runtime::block_on` calls and not by `Handle::block_on`.
+ let result = tokio::task::block_in_place(spawn_me);
Box::pin(after_spawn(result))
} else {
let handle = tokio::task::spawn_blocking(spawn_me);
diff --git a/libs/binder/rust/libbinder_ndk_bindgen_flags.txt b/libs/binder/rust/libbinder_ndk_bindgen_flags.txt
new file mode 100644
index 0000000..551c59f
--- /dev/null
+++ b/libs/binder/rust/libbinder_ndk_bindgen_flags.txt
@@ -0,0 +1,11 @@
+--default-enum-style=rust_non_exhaustive
+--constified-enum=android::c_interface::consts::.*
+--allowlist-type=android::c_interface::.*
+--allowlist-type=AStatus
+--allowlist-type=AIBinder_Class
+--allowlist-type=AIBinder
+--allowlist-type=AIBinder_Weak
+--allowlist-type=AIBinder_DeathRecipient
+--allowlist-type=AParcel
+--allowlist-type=binder_status_t
+--allowlist-function=.*
diff --git a/libs/binder/rust/rpcbinder/src/server.rs b/libs/binder/rust/rpcbinder/src/server.rs
index 81f68f5..6fda878 100644
--- a/libs/binder/rust/rpcbinder/src/server.rs
+++ b/libs/binder/rust/rpcbinder/src/server.rs
@@ -33,9 +33,9 @@
pub struct RpcServerRef;
}
-/// SAFETY - The opaque handle can be cloned freely.
+/// SAFETY: The opaque handle can be cloned freely.
unsafe impl Send for RpcServer {}
-/// SAFETY - The underlying C++ RpcServer class is thread-safe.
+/// SAFETY: The underlying C++ RpcServer class is thread-safe.
unsafe impl Sync for RpcServer {}
impl RpcServer {
@@ -59,7 +59,10 @@
/// Creates a binder RPC server, serving the supplied binder service implementation on the given
/// socket file descriptor. The socket should be bound to an address before calling this
/// function.
- pub fn new_bound_socket(mut service: SpIBinder, socket_fd: OwnedFd) -> Result<RpcServer, Error> {
+ pub fn new_bound_socket(
+ mut service: SpIBinder,
+ socket_fd: OwnedFd,
+ ) -> Result<RpcServer, Error> {
let service = service.as_native_mut();
// SAFETY: Service ownership is transferring to the server and won't be valid afterward.
@@ -67,7 +70,8 @@
// The server takes ownership of the socket FD.
unsafe {
Self::checked_from_ptr(binder_rpc_unstable_bindgen::ARpcServer_newBoundSocket(
- service, socket_fd.into_raw_fd(),
+ service,
+ socket_fd.into_raw_fd(),
))
}
}
@@ -120,7 +124,9 @@
if ptr.is_null() {
return Err(Error::new(ErrorKind::Other, "Failed to start server"));
}
- Ok(RpcServer::from_ptr(ptr))
+ // SAFETY: Our caller must pass us a valid or null pointer, and we've checked that it's not
+ // null.
+ Ok(unsafe { RpcServer::from_ptr(ptr) })
}
}
@@ -130,7 +136,7 @@
&self,
modes: &[FileDescriptorTransportMode],
) {
- // SAFETY - Does not keep the pointer after returning does, nor does it
+ // SAFETY: Does not keep the pointer after returning does, nor does it
// read past its boundary. Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcServer_setSupportedFileDescriptorTransportModes(
@@ -143,18 +149,21 @@
/// Starts a new background thread and calls join(). Returns immediately.
pub fn start(&self) {
+ // SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
unsafe { binder_rpc_unstable_bindgen::ARpcServer_start(self.as_ptr()) };
}
/// Joins the RpcServer thread. The call blocks until the server terminates.
/// This must be called from exactly one thread.
pub fn join(&self) {
+ // SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
unsafe { binder_rpc_unstable_bindgen::ARpcServer_join(self.as_ptr()) };
}
/// Shuts down the running RpcServer. Can be called multiple times and from
/// multiple threads. Called automatically during drop().
pub fn shutdown(&self) -> Result<(), Error> {
+ // SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
if unsafe { binder_rpc_unstable_bindgen::ARpcServer_shutdown(self.as_ptr()) } {
Ok(())
} else {
diff --git a/libs/binder/rust/rpcbinder/src/session.rs b/libs/binder/rust/rpcbinder/src/session.rs
index 28c5390..79a9510 100644
--- a/libs/binder/rust/rpcbinder/src/session.rs
+++ b/libs/binder/rust/rpcbinder/src/session.rs
@@ -36,15 +36,15 @@
pub struct RpcSessionRef;
}
-/// SAFETY - The opaque handle can be cloned freely.
+/// SAFETY: The opaque handle can be cloned freely.
unsafe impl Send for RpcSession {}
-/// SAFETY - The underlying C++ RpcSession class is thread-safe.
+/// SAFETY: The underlying C++ RpcSession class is thread-safe.
unsafe impl Sync for RpcSession {}
impl RpcSession {
/// Allocates a new RpcSession object.
pub fn new() -> RpcSession {
- // SAFETY - Takes ownership of the returned handle, which has correct refcount.
+ // SAFETY: Takes ownership of the returned handle, which has correct refcount.
unsafe { RpcSession::from_ptr(binder_rpc_unstable_bindgen::ARpcSession_new()) }
}
}
@@ -58,7 +58,7 @@
impl RpcSessionRef {
/// Sets the file descriptor transport mode for this session.
pub fn set_file_descriptor_transport_mode(&self, mode: FileDescriptorTransportMode) {
- // SAFETY - Only passes the 'self' pointer as an opaque handle.
+ // SAFETY: Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcSession_setFileDescriptorTransportMode(
self.as_ptr(),
@@ -69,7 +69,7 @@
/// Sets the maximum number of incoming threads.
pub fn set_max_incoming_threads(&self, threads: usize) {
- // SAFETY - Only passes the 'self' pointer as an opaque handle.
+ // SAFETY: Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcSession_setMaxIncomingThreads(self.as_ptr(), threads)
};
@@ -77,7 +77,7 @@
/// Sets the maximum number of outgoing connections.
pub fn set_max_outgoing_connections(&self, connections: usize) {
- // SAFETY - Only passes the 'self' pointer as an opaque handle.
+ // SAFETY: Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingConnections(
self.as_ptr(),
@@ -210,10 +210,10 @@
type RequestFd<'a> = &'a mut dyn FnMut() -> Option<RawFd>;
unsafe extern "C" fn request_fd_wrapper(param: *mut c_void) -> c_int {
+ let request_fd_ptr = param as *mut RequestFd;
// SAFETY: This is only ever called by RpcPreconnectedClient, within the lifetime of the
// BinderFdFactory reference, with param being a properly aligned non-null pointer to an
// initialized instance.
- let request_fd_ptr = param as *mut RequestFd;
- let request_fd = request_fd_ptr.as_mut().unwrap();
+ let request_fd = unsafe { request_fd_ptr.as_mut().unwrap() };
request_fd().unwrap_or(-1)
}
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index 993bdca..463c210 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -97,8 +97,8 @@
/// Interface stability promise
///
-/// An interface can promise to be a stable vendor interface ([`Vintf`]), or
-/// makes no stability guarantees ([`Local`]). [`Local`] is
+/// An interface can promise to be a stable vendor interface ([`Stability::Vintf`]),
+/// or makes no stability guarantees ([`Stability::Local`]). [`Stability::Local`] is
/// currently the default stability.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Default)]
pub enum Stability {
@@ -139,8 +139,8 @@
/// via `Binder::new(object)`.
///
/// This is a low-level interface that should normally be automatically
-/// generated from AIDL via the [`declare_binder_interface!`] macro. When using
-/// the AIDL backend, users need only implement the high-level AIDL-defined
+/// generated from AIDL via the [`crate::declare_binder_interface!`] macro.
+/// When using the AIDL backend, users need only implement the high-level AIDL-defined
/// interface. The AIDL compiler then generates a container struct that wraps
/// the user-defined service and implements `Remotable`.
pub trait Remotable: Send + Sync {
@@ -297,18 +297,17 @@
/// Note: the returned pointer will not be constant. Calling this method
/// multiple times for the same type will result in distinct class
/// pointers. A static getter for this value is implemented in
- /// [`declare_binder_interface!`].
+ /// [`crate::declare_binder_interface!`].
pub fn new<I: InterfaceClassMethods>() -> InterfaceClass {
let descriptor = CString::new(I::get_descriptor()).unwrap();
+ // Safety: `AIBinder_Class_define` expects a valid C string, and three
+ // valid callback functions, all non-null pointers. The C string is
+ // copied and need not be valid for longer than the call, so we can drop
+ // it after the call. We can safely assign null to the onDump and
+ // handleShellCommand callbacks as long as the class pointer was
+ // non-null. Rust None for a Option<fn> is guaranteed to be a NULL
+ // pointer. Rust retains ownership of the pointer after it is defined.
let ptr = unsafe {
- // Safety: `AIBinder_Class_define` expects a valid C string, and
- // three valid callback functions, all non-null pointers. The C
- // string is copied and need not be valid for longer than the call,
- // so we can drop it after the call. We can safely assign null to
- // the onDump and handleShellCommand callbacks as long as the class
- // pointer was non-null. Rust None for a Option<fn> is guaranteed to
- // be a NULL pointer. Rust retains ownership of the pointer after it
- // is defined.
let class = sys::AIBinder_Class_define(
descriptor.as_ptr(),
Some(I::on_create),
@@ -338,13 +337,12 @@
/// Get the interface descriptor string of this class.
pub fn get_descriptor(&self) -> String {
+ // SAFETY: The descriptor returned by AIBinder_Class_getDescriptor is
+ // always a two-byte null terminated sequence of u16s. Thus, we can
+ // continue reading from the pointer until we hit a null value, and this
+ // pointer can be a valid slice if the slice length is <= the number of
+ // u16 elements before the null terminator.
unsafe {
- // SAFETY: The descriptor returned by AIBinder_Class_getDescriptor
- // is always a two-byte null terminated sequence of u16s. Thus, we
- // can continue reading from the pointer until we hit a null value,
- // and this pointer can be a valid slice if the slice length is <=
- // the number of u16 elements before the null terminator.
-
let raw_descriptor: *const c_char = sys::AIBinder_Class_getDescriptor(self.0);
CStr::from_ptr(raw_descriptor)
.to_str()
@@ -542,17 +540,15 @@
static CLASS_INIT: std::sync::Once = std::sync::Once::new();
static mut CLASS: Option<$crate::binder_impl::InterfaceClass> = None;
+ // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
+ // variable, and therefore is thread-safe, as it can only occur
+ // once.
CLASS_INIT.call_once(|| unsafe {
- // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
- // variable, and therefore is thread-safe, as it can only occur
- // once.
CLASS = Some($constructor);
});
- unsafe {
- // Safety: The `CLASS` variable can only be mutated once, above,
- // and is subsequently safe to read from any thread.
- CLASS.unwrap()
- }
+ // Safety: The `CLASS` variable can only be mutated once, above, and
+ // is subsequently safe to read from any thread.
+ unsafe { CLASS.unwrap() }
}
};
}
@@ -664,6 +660,8 @@
fn as_native_mut(&mut self) -> *mut T;
}
+// Safety: If V is a valid Android C++ type then we can either use that or a
+// null pointer.
unsafe impl<T, V: AsNative<T>> AsNative<T> for Option<V> {
fn as_native(&self) -> *const T {
self.as_ref().map_or(ptr::null(), |v| v.as_native())
@@ -924,15 +922,15 @@
static CLASS_INIT: std::sync::Once = std::sync::Once::new();
static mut CLASS: Option<$crate::binder_impl::InterfaceClass> = None;
+ // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
+ // variable, and therefore is thread-safe, as it can only occur
+ // once.
CLASS_INIT.call_once(|| unsafe {
- // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
- // variable, and therefore is thread-safe, as it can only occur
- // once.
CLASS = Some($crate::binder_impl::InterfaceClass::new::<$crate::binder_impl::Binder<$native>>());
});
+ // Safety: The `CLASS` variable can only be mutated once, above,
+ // and is subsequently safe to read from any thread.
unsafe {
- // Safety: The `CLASS` variable can only be mutated once, above,
- // and is subsequently safe to read from any thread.
CLASS.unwrap()
}
}
@@ -1025,17 +1023,7 @@
}
if ibinder.associate_class(<$native as $crate::binder_impl::Remotable>::get_class()) {
- let service: std::result::Result<$crate::binder_impl::Binder<$native>, $crate::StatusCode> =
- std::convert::TryFrom::try_from(ibinder.clone());
- if let Ok(service) = service {
- // We were able to associate with our expected class and
- // the service is local.
- todo!()
- //return Ok($crate::Strong::new(Box::new(service)));
- } else {
- // Service is remote
- return Ok($crate::Strong::new(Box::new(<$proxy as $crate::binder_impl::Proxy>::from_binder(ibinder)?)));
- }
+ return Ok($crate::Strong::new(Box::new(<$proxy as $crate::binder_impl::Proxy>::from_binder(ibinder)?)));
}
Err($crate::StatusCode::BAD_TYPE.into())
diff --git a/libs/binder/rust/src/error.rs b/libs/binder/rust/src/error.rs
index ba26062..eb04cc3 100644
--- a/libs/binder/rust/src/error.rs
+++ b/libs/binder/rust/src/error.rs
@@ -112,41 +112,35 @@
impl Status {
/// Create a status object representing a successful transaction.
pub fn ok() -> Self {
- let ptr = unsafe {
- // Safety: `AStatus_newOk` always returns a new, heap allocated
- // pointer to an `ASTatus` object, so we know this pointer will be
- // valid.
- //
- // Rust takes ownership of the returned pointer.
- sys::AStatus_newOk()
- };
+ // Safety: `AStatus_newOk` always returns a new, heap allocated
+ // pointer to an `ASTatus` object, so we know this pointer will be
+ // valid.
+ //
+ // Rust takes ownership of the returned pointer.
+ let ptr = unsafe { sys::AStatus_newOk() };
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
/// Create a status object from a service specific error
pub fn new_service_specific_error(err: i32, message: Option<&CStr>) -> Status {
let ptr = if let Some(message) = message {
- unsafe {
- // Safety: Any i32 is a valid service specific error for the
- // error code parameter. We construct a valid, null-terminated
- // `CString` from the message, which must be a valid C-style
- // string to pass as the message. This function always returns a
- // new, heap allocated pointer to an `AStatus` object, so we
- // know the returned pointer will be valid.
- //
- // Rust takes ownership of the returned pointer.
- sys::AStatus_fromServiceSpecificErrorWithMessage(err, message.as_ptr())
- }
+ // Safety: Any i32 is a valid service specific error for the
+ // error code parameter. We construct a valid, null-terminated
+ // `CString` from the message, which must be a valid C-style
+ // string to pass as the message. This function always returns a
+ // new, heap allocated pointer to an `AStatus` object, so we
+ // know the returned pointer will be valid.
+ //
+ // Rust takes ownership of the returned pointer.
+ unsafe { sys::AStatus_fromServiceSpecificErrorWithMessage(err, message.as_ptr()) }
} else {
- unsafe {
- // Safety: Any i32 is a valid service specific error for the
- // error code parameter. This function always returns a new,
- // heap allocated pointer to an `AStatus` object, so we know the
- // returned pointer will be valid.
- //
- // Rust takes ownership of the returned pointer.
- sys::AStatus_fromServiceSpecificError(err)
- }
+ // Safety: Any i32 is a valid service specific error for the
+ // error code parameter. This function always returns a new,
+ // heap allocated pointer to an `AStatus` object, so we know the
+ // returned pointer will be valid.
+ //
+ // Rust takes ownership of the returned pointer.
+ unsafe { sys::AStatus_fromServiceSpecificError(err) }
};
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
@@ -159,6 +153,8 @@
/// Create a status object from an exception code
pub fn new_exception(exception: ExceptionCode, message: Option<&CStr>) -> Status {
if let Some(message) = message {
+ // Safety: the C string pointer is valid and not retained by the
+ // function.
let ptr = unsafe {
sys::AStatus_fromExceptionCodeWithMessage(exception as i32, message.as_ptr())
};
@@ -187,37 +183,31 @@
/// Returns `true` if this status represents a successful transaction.
pub fn is_ok(&self) -> bool {
- unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_isOk` here.
- sys::AStatus_isOk(self.as_native())
- }
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_isOk` here.
+ unsafe { sys::AStatus_isOk(self.as_native()) }
}
/// Returns a description of the status.
pub fn get_description(&self) -> String {
- let description_ptr = unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_getDescription`
- // here.
- //
- // `AStatus_getDescription` always returns a valid pointer to a null
- // terminated C string. Rust is responsible for freeing this pointer
- // via `AStatus_deleteDescription`.
- sys::AStatus_getDescription(self.as_native())
- };
- let description = unsafe {
- // Safety: `AStatus_getDescription` always returns a valid C string,
- // which can be safely converted to a `CStr`.
- CStr::from_ptr(description_ptr)
- };
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_getDescription`
+ // here.
+ //
+ // `AStatus_getDescription` always returns a valid pointer to a null
+ // terminated C string. Rust is responsible for freeing this pointer
+ // via `AStatus_deleteDescription`.
+ let description_ptr = unsafe { sys::AStatus_getDescription(self.as_native()) };
+ // Safety: `AStatus_getDescription` always returns a valid C string,
+ // which can be safely converted to a `CStr`.
+ let description = unsafe { CStr::from_ptr(description_ptr) };
let description = description.to_string_lossy().to_string();
+ // Safety: `description_ptr` was returned from
+ // `AStatus_getDescription` above, and must be freed via
+ // `AStatus_deleteDescription`. We must not access the pointer after
+ // this call, so we copy it into an owned string above and return
+ // that string.
unsafe {
- // Safety: `description_ptr` was returned from
- // `AStatus_getDescription` above, and must be freed via
- // `AStatus_deleteDescription`. We must not access the pointer after
- // this call, so we copy it into an owned string above and return
- // that string.
sys::AStatus_deleteDescription(description_ptr);
}
description
@@ -225,12 +215,10 @@
/// Returns the exception code of the status.
pub fn exception_code(&self) -> ExceptionCode {
- let code = unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_getExceptionCode`
- // here.
- sys::AStatus_getExceptionCode(self.as_native())
- };
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_getExceptionCode`
+ // here.
+ let code = unsafe { sys::AStatus_getExceptionCode(self.as_native()) };
parse_exception_code(code)
}
@@ -241,11 +229,9 @@
/// exception or a service specific error. To find out if this transaction
/// as a whole is okay, use [`is_ok`](Self::is_ok) instead.
pub fn transaction_error(&self) -> StatusCode {
- let code = unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_getStatus` here.
- sys::AStatus_getStatus(self.as_native())
- };
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_getStatus` here.
+ let code = unsafe { sys::AStatus_getStatus(self.as_native()) };
parse_status_code(code)
}
@@ -258,12 +244,10 @@
/// find out if this transaction as a whole is okay, use
/// [`is_ok`](Self::is_ok) instead.
pub fn service_specific_error(&self) -> i32 {
- unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to
- // `AStatus_getServiceSpecificError` here.
- sys::AStatus_getServiceSpecificError(self.as_native())
- }
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to
+ // `AStatus_getServiceSpecificError` here.
+ unsafe { sys::AStatus_getServiceSpecificError(self.as_native()) }
}
/// Calls `op` if the status was ok, otherwise returns an `Err` value of
@@ -321,24 +305,20 @@
impl From<status_t> for Status {
fn from(status: status_t) -> Status {
- let ptr = unsafe {
- // Safety: `AStatus_fromStatus` expects any `status_t` integer, so
- // this is a safe FFI call. Unknown values will be coerced into
- // UNKNOWN_ERROR.
- sys::AStatus_fromStatus(status)
- };
+ // Safety: `AStatus_fromStatus` expects any `status_t` integer, so
+ // this is a safe FFI call. Unknown values will be coerced into
+ // UNKNOWN_ERROR.
+ let ptr = unsafe { sys::AStatus_fromStatus(status) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
}
impl From<ExceptionCode> for Status {
fn from(code: ExceptionCode) -> Status {
- let ptr = unsafe {
- // Safety: `AStatus_fromExceptionCode` expects any
- // `binder_exception_t` (i32) integer, so this is a safe FFI call.
- // Unknown values will be coerced into EX_TRANSACTION_FAILED.
- sys::AStatus_fromExceptionCode(code as i32)
- };
+ // Safety: `AStatus_fromExceptionCode` expects any
+ // `binder_exception_t` (i32) integer, so this is a safe FFI call.
+ // Unknown values will be coerced into EX_TRANSACTION_FAILED.
+ let ptr = unsafe { sys::AStatus_fromExceptionCode(code as i32) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
}
@@ -363,20 +343,18 @@
impl Drop for Status {
fn drop(&mut self) {
+ // Safety: `Status` manages the lifetime of its inner `AStatus`
+ // pointee, so we need to delete it here. We know that the pointer
+ // will be valid here since `Status` always contains a valid pointer
+ // while it is alive.
unsafe {
- // Safety: `Status` manages the lifetime of its inner `AStatus`
- // pointee, so we need to delete it here. We know that the pointer
- // will be valid here since `Status` always contains a valid pointer
- // while it is alive.
sys::AStatus_delete(self.0.as_mut());
}
}
}
-/// # Safety
-///
-/// `Status` always contains a valid pointer to an `AStatus` object, so we can
-/// trivially convert it to a correctly-typed raw pointer.
+/// Safety: `Status` always contains a valid pointer to an `AStatus` object, so
+/// we can trivially convert it to a correctly-typed raw pointer.
///
/// Care must be taken that the returned pointer is only dereferenced while the
/// `Status` object is still alive.
@@ -386,11 +364,97 @@
}
fn as_native_mut(&mut self) -> *mut sys::AStatus {
- unsafe {
- // Safety: The pointer will be valid here since `Status` always
- // contains a valid and initialized pointer while it is alive.
- self.0.as_mut()
- }
+ // Safety: The pointer will be valid here since `Status` always contains
+ // a valid and initialized pointer while it is alive.
+ unsafe { self.0.as_mut() }
+ }
+}
+
+/// A conversion from `std::result::Result<T, E>` to `binder::Result<T>`. If this type is `Ok(T)`,
+/// it's returned as is. If this type is `Err(E)`, `E` is converted into `Status` which can be
+/// either a general binder exception, or a service-specific exception.
+///
+/// # Examples
+///
+/// ```
+/// // std::io::Error is formatted as the exception's message
+/// fn file_exists(name: &str) -> binder::Result<bool> {
+/// std::fs::metadata(name)
+/// .or_service_specific_exception(NOT_FOUND)?
+/// }
+///
+/// // A custom function is used to create the exception's message
+/// fn file_exists(name: &str) -> binder::Result<bool> {
+/// std::fs::metadata(name)
+/// .or_service_specific_exception_with(NOT_FOUND,
+/// |e| format!("file {} not found: {:?}", name, e))?
+/// }
+///
+/// // anyhow::Error is formatted as the exception's message
+/// use anyhow::{Context, Result};
+/// fn file_exists(name: &str) -> binder::Result<bool> {
+/// std::fs::metadata(name)
+/// .context("file {} not found")
+/// .or_service_specific_exception(NOT_FOUND)?
+/// }
+///
+/// // General binder exceptions can be created similarly
+/// fn file_exists(name: &str) -> binder::Result<bool> {
+/// std::fs::metadata(name)
+/// .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT)?
+/// }
+/// ```
+pub trait IntoBinderResult<T, E> {
+ /// Converts the embedded error into a general binder exception of code `exception`. The
+ /// message of the exception is set by formatting the error for debugging.
+ fn or_binder_exception(self, exception: ExceptionCode) -> result::Result<T, Status>;
+
+ /// Converts the embedded error into a general binder exception of code `exception`. The
+ /// message of the exception is set by lazily evaluating the `op` function.
+ fn or_binder_exception_with<M: AsRef<str>, O: FnOnce(E) -> M>(
+ self,
+ exception: ExceptionCode,
+ op: O,
+ ) -> result::Result<T, Status>;
+
+ /// Converts the embedded error into a service-specific binder exception. `error_code` is used
+ /// to distinguish different service-specific binder exceptions. The message of the exception
+ /// is set by formatting the error for debugging.
+ fn or_service_specific_exception(self, error_code: i32) -> result::Result<T, Status>;
+
+ /// Converts the embedded error into a service-specific binder exception. `error_code` is used
+ /// to distinguish different service-specific binder exceptions. The message of the exception
+ /// is set by lazily evaluating the `op` function.
+ fn or_service_specific_exception_with<M: AsRef<str>, O: FnOnce(E) -> M>(
+ self,
+ error_code: i32,
+ op: O,
+ ) -> result::Result<T, Status>;
+}
+
+impl<T, E: std::fmt::Debug> IntoBinderResult<T, E> for result::Result<T, E> {
+ fn or_binder_exception(self, exception: ExceptionCode) -> result::Result<T, Status> {
+ self.or_binder_exception_with(exception, |e| format!("{:?}", e))
+ }
+
+ fn or_binder_exception_with<M: AsRef<str>, O: FnOnce(E) -> M>(
+ self,
+ exception: ExceptionCode,
+ op: O,
+ ) -> result::Result<T, Status> {
+ self.map_err(|e| Status::new_exception_str(exception, Some(op(e))))
+ }
+
+ fn or_service_specific_exception(self, error_code: i32) -> result::Result<T, Status> {
+ self.or_service_specific_exception_with(error_code, |e| format!("{:?}", e))
+ }
+
+ fn or_service_specific_exception_with<M: AsRef<str>, O: FnOnce(E) -> M>(
+ self,
+ error_code: i32,
+ op: O,
+ ) -> result::Result<T, Status> {
+ self.map_err(|e| Status::new_service_specific_error_str(error_code, Some(op(e))))
}
}
@@ -430,4 +494,66 @@
assert_eq!(status.service_specific_error(), 0);
assert_eq!(status.get_description(), "Status(-5, EX_ILLEGAL_STATE): ''".to_string());
}
+
+ #[test]
+ fn convert_to_service_specific_exception() {
+ let res: std::result::Result<(), Status> =
+ Err("message").or_service_specific_exception(-42);
+
+ assert!(res.is_err());
+ let status = res.unwrap_err();
+ assert_eq!(status.exception_code(), ExceptionCode::SERVICE_SPECIFIC);
+ assert_eq!(status.service_specific_error(), -42);
+ assert_eq!(
+ status.get_description(),
+ "Status(-8, EX_SERVICE_SPECIFIC): '-42: \"message\"'".to_string()
+ );
+ }
+
+ #[test]
+ fn convert_to_service_specific_exception_with() {
+ let res: std::result::Result<(), Status> = Err("message")
+ .or_service_specific_exception_with(-42, |e| format!("outer message: {:?}", e));
+
+ assert!(res.is_err());
+ let status = res.unwrap_err();
+ assert_eq!(status.exception_code(), ExceptionCode::SERVICE_SPECIFIC);
+ assert_eq!(status.service_specific_error(), -42);
+ assert_eq!(
+ status.get_description(),
+ "Status(-8, EX_SERVICE_SPECIFIC): '-42: outer message: \"message\"'".to_string()
+ );
+ }
+
+ #[test]
+ fn convert_to_binder_exception() {
+ let res: std::result::Result<(), Status> =
+ Err("message").or_binder_exception(ExceptionCode::ILLEGAL_STATE);
+
+ assert!(res.is_err());
+ let status = res.unwrap_err();
+ assert_eq!(status.exception_code(), ExceptionCode::ILLEGAL_STATE);
+ assert_eq!(status.service_specific_error(), 0);
+ assert_eq!(
+ status.get_description(),
+ "Status(-5, EX_ILLEGAL_STATE): '\"message\"'".to_string()
+ );
+ }
+
+ #[test]
+ fn convert_to_binder_exception_with() {
+ let res: std::result::Result<(), Status> = Err("message")
+ .or_binder_exception_with(ExceptionCode::ILLEGAL_STATE, |e| {
+ format!("outer message: {:?}", e)
+ });
+
+ assert!(res.is_err());
+ let status = res.unwrap_err();
+ assert_eq!(status.exception_code(), ExceptionCode::ILLEGAL_STATE);
+ assert_eq!(status.service_specific_error(), 0);
+ assert_eq!(
+ status.get_description(),
+ "Status(-5, EX_ILLEGAL_STATE): 'outer message: \"message\"'".to_string()
+ );
+ }
}
diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs
index 0c8b48f..8841fe6 100644
--- a/libs/binder/rust/src/lib.rs
+++ b/libs/binder/rust/src/lib.rs
@@ -106,7 +106,7 @@
pub use crate::binder_async::{BinderAsyncPool, BoxFuture};
pub use binder::{BinderFeatures, FromIBinder, IBinder, Interface, Strong, Weak};
-pub use error::{ExceptionCode, Status, StatusCode};
+pub use error::{ExceptionCode, IntoBinderResult, Status, StatusCode};
pub use native::{
add_service, force_lazy_services_persist, is_handling_transaction, register_lazy_service,
LazyServiceGuard,
diff --git a/libs/binder/rust/src/native.rs b/libs/binder/rust/src/native.rs
index 5557168..b248f5e 100644
--- a/libs/binder/rust/src/native.rs
+++ b/libs/binder/rust/src/native.rs
@@ -42,7 +42,7 @@
rust_object: *mut T,
}
-/// # Safety
+/// Safety:
///
/// A `Binder<T>` is a pair of unique owning pointers to two values:
/// * a C++ ABBinder which the C++ API guarantees can be passed between threads
@@ -54,7 +54,7 @@
/// to how `Box<T>` is `Send` if `T` is `Send`.
unsafe impl<T: Remotable> Send for Binder<T> {}
-/// # Safety
+/// Safety:
///
/// A `Binder<T>` is a pair of unique owning pointers to two values:
/// * a C++ ABBinder which is thread-safe, i.e. `Send + Sync`
@@ -89,15 +89,13 @@
pub fn new_with_stability(rust_object: T, stability: Stability) -> Binder<T> {
let class = T::get_class();
let rust_object = Box::into_raw(Box::new(rust_object));
- let ibinder = unsafe {
- // Safety: `AIBinder_new` expects a valid class pointer (which we
- // initialize via `get_class`), and an arbitrary pointer
- // argument. The caller owns the returned `AIBinder` pointer, which
- // is a strong reference to a `BBinder`. This reference should be
- // decremented via `AIBinder_decStrong` when the reference lifetime
- // ends.
- sys::AIBinder_new(class.into(), rust_object as *mut c_void)
- };
+ // Safety: `AIBinder_new` expects a valid class pointer (which we
+ // initialize via `get_class`), and an arbitrary pointer
+ // argument. The caller owns the returned `AIBinder` pointer, which
+ // is a strong reference to a `BBinder`. This reference should be
+ // decremented via `AIBinder_decStrong` when the reference lifetime
+ // ends.
+ let ibinder = unsafe { sys::AIBinder_new(class.into(), rust_object as *mut c_void) };
let mut binder = Binder { ibinder, rust_object };
binder.mark_stability(stability);
binder
@@ -176,15 +174,14 @@
/// }
/// # }
pub fn set_extension(&mut self, extension: &mut SpIBinder) -> Result<()> {
- let status = unsafe {
- // Safety: `AIBinder_setExtension` expects two valid, mutable
- // `AIBinder` pointers. We are guaranteed that both `self` and
- // `extension` contain valid `AIBinder` pointers, because they
- // cannot be initialized without a valid
- // pointer. `AIBinder_setExtension` does not take ownership of
- // either parameter.
- sys::AIBinder_setExtension(self.as_native_mut(), extension.as_native_mut())
- };
+ let status =
+ // Safety: `AIBinder_setExtension` expects two valid, mutable
+ // `AIBinder` pointers. We are guaranteed that both `self` and
+ // `extension` contain valid `AIBinder` pointers, because they
+ // cannot be initialized without a valid
+ // pointer. `AIBinder_setExtension` does not take ownership of
+ // either parameter.
+ unsafe { sys::AIBinder_setExtension(self.as_native_mut(), extension.as_native_mut()) };
status_result(status)
}
@@ -199,9 +196,9 @@
match stability {
Stability::Local => self.mark_local_stability(),
Stability::Vintf => {
+ // Safety: Self always contains a valid `AIBinder` pointer, so
+ // we can always call this C API safely.
unsafe {
- // Safety: Self always contains a valid `AIBinder` pointer, so
- // we can always call this C API safely.
sys::AIBinder_markVintfStability(self.as_native_mut());
}
}
@@ -212,9 +209,9 @@
/// building for android_vendor and system otherwise.
#[cfg(android_vendor)]
fn mark_local_stability(&mut self) {
+ // Safety: Self always contains a valid `AIBinder` pointer, so we can
+ // always call this C API safely.
unsafe {
- // Safety: Self always contains a valid `AIBinder` pointer, so
- // we can always call this C API safely.
sys::AIBinder_markVendorStability(self.as_native_mut());
}
}
@@ -223,9 +220,9 @@
/// building for android_vendor and system otherwise.
#[cfg(not(android_vendor))]
fn mark_local_stability(&mut self) {
+ // Safety: Self always contains a valid `AIBinder` pointer, so we can
+ // always call this C API safely.
unsafe {
- // Safety: Self always contains a valid `AIBinder` pointer, so
- // we can always call this C API safely.
sys::AIBinder_markSystemStability(self.as_native_mut());
}
}
@@ -239,13 +236,13 @@
/// remotable object, which will prevent the object from being dropped while
/// the `SpIBinder` is alive.
fn as_binder(&self) -> SpIBinder {
+ // Safety: `self.ibinder` is guaranteed to always be a valid pointer
+ // to an `AIBinder` by the `Binder` constructor. We are creating a
+ // copy of the `self.ibinder` strong reference, but
+ // `SpIBinder::from_raw` assumes it receives an owned pointer with
+ // its own strong reference. We first increment the reference count,
+ // so that the new `SpIBinder` will be tracked as a new reference.
unsafe {
- // Safety: `self.ibinder` is guaranteed to always be a valid pointer
- // to an `AIBinder` by the `Binder` constructor. We are creating a
- // copy of the `self.ibinder` strong reference, but
- // `SpIBinder::from_raw` assumes it receives an owned pointer with
- // its own strong reference. We first increment the reference count,
- // so that the new `SpIBinder` will be tracked as a new reference.
sys::AIBinder_incStrong(self.ibinder);
SpIBinder::from_raw(self.ibinder).unwrap()
}
@@ -275,10 +272,20 @@
reply: *mut sys::AParcel,
) -> status_t {
let res = {
- let mut reply = BorrowedParcel::from_raw(reply).unwrap();
- let data = BorrowedParcel::from_raw(data as *mut sys::AParcel).unwrap();
- let object = sys::AIBinder_getUserData(binder);
- let binder: &T = &*(object as *const T);
+ // Safety: The caller must give us a parcel pointer which is either
+ // null or valid at least for the duration of this function call. We
+ // don't keep the resulting value beyond the function.
+ let mut reply = unsafe { BorrowedParcel::from_raw(reply).unwrap() };
+ // Safety: The caller must give us a parcel pointer which is either
+ // null or valid at least for the duration of this function call. We
+ // don't keep the resulting value beyond the function.
+ let data = unsafe { BorrowedParcel::from_raw(data as *mut sys::AParcel).unwrap() };
+ // Safety: Our caller promised that `binder` is a non-null, valid
+ // pointer to a local `AIBinder`.
+ let object = unsafe { sys::AIBinder_getUserData(binder) };
+ // Safety: Our caller promised that the binder has a `T` pointer in
+ // its user data.
+ let binder: &T = unsafe { &*(object as *const T) };
binder.on_transact(code, &data, &mut reply)
};
match res {
@@ -295,7 +302,9 @@
/// Must be called with a valid pointer to a `T` object. After this call,
/// the pointer will be invalid and should not be dereferenced.
unsafe extern "C" fn on_destroy(object: *mut c_void) {
- drop(Box::from_raw(object as *mut T));
+ // Safety: Our caller promised that `object` is a valid pointer to a
+ // `T`.
+ drop(unsafe { Box::from_raw(object as *mut T) });
}
/// Called whenever a new, local `AIBinder` object is needed of a specific
@@ -320,7 +329,7 @@
/// Must be called with a non-null, valid pointer to a local `AIBinder` that
/// contains a `T` pointer in its user data. fd should be a non-owned file
/// descriptor, and args must be an array of null-terminated string
- /// poiinters with length num_args.
+ /// pointers with length num_args.
unsafe extern "C" fn on_dump(
binder: *mut sys::AIBinder,
fd: i32,
@@ -330,8 +339,9 @@
if fd < 0 {
return StatusCode::UNEXPECTED_NULL as status_t;
}
- // We don't own this file, so we need to be careful not to drop it.
- let file = ManuallyDrop::new(File::from_raw_fd(fd));
+ // Safety: Our caller promised that fd is a file descriptor. We don't
+ // own this file descriptor, so we need to be careful not to drop it.
+ let file = unsafe { ManuallyDrop::new(File::from_raw_fd(fd)) };
if args.is_null() && num_args != 0 {
return StatusCode::UNEXPECTED_NULL as status_t;
@@ -340,14 +350,22 @@
let args = if args.is_null() || num_args == 0 {
vec![]
} else {
- slice::from_raw_parts(args, num_args as usize)
- .iter()
- .map(|s| CStr::from_ptr(*s))
- .collect()
+ // Safety: Our caller promised that `args` is an array of
+ // null-terminated string pointers with length `num_args`.
+ unsafe {
+ slice::from_raw_parts(args, num_args as usize)
+ .iter()
+ .map(|s| CStr::from_ptr(*s))
+ .collect()
+ }
};
- let object = sys::AIBinder_getUserData(binder);
- let binder: &T = &*(object as *const T);
+ // Safety: Our caller promised that `binder` is a non-null, valid
+ // pointer to a local `AIBinder`.
+ let object = unsafe { sys::AIBinder_getUserData(binder) };
+ // Safety: Our caller promised that the binder has a `T` pointer in its
+ // user data.
+ let binder: &T = unsafe { &*(object as *const T) };
let res = binder.on_dump(&file, &args);
match res {
@@ -363,11 +381,11 @@
// actually destroys the object, it calls `on_destroy` and we can drop the
// `rust_object` then.
fn drop(&mut self) {
+ // Safety: When `self` is dropped, we can no longer access the
+ // reference, so can decrement the reference count. `self.ibinder` is
+ // always a valid `AIBinder` pointer, so is valid to pass to
+ // `AIBinder_decStrong`.
unsafe {
- // Safety: When `self` is dropped, we can no longer access the
- // reference, so can decrement the reference count. `self.ibinder`
- // is always a valid `AIBinder` pointer, so is valid to pass to
- // `AIBinder_decStrong`.
sys::AIBinder_decStrong(self.ibinder);
}
}
@@ -377,14 +395,11 @@
type Target = T;
fn deref(&self) -> &Self::Target {
- unsafe {
- // Safety: While `self` is alive, the reference count of the
- // underlying object is > 0 and therefore `on_destroy` cannot be
- // called. Therefore while `self` is alive, we know that
- // `rust_object` is still a valid pointer to a heap allocated object
- // of type `T`.
- &*self.rust_object
- }
+ // Safety: While `self` is alive, the reference count of the underlying
+ // object is > 0 and therefore `on_destroy` cannot be called. Therefore
+ // while `self` is alive, we know that `rust_object` is still a valid
+ // pointer to a heap allocated object of type `T`.
+ unsafe { &*self.rust_object }
}
}
@@ -405,13 +420,10 @@
if Some(class) != ibinder.get_class() {
return Err(StatusCode::BAD_TYPE);
}
- let userdata = unsafe {
- // Safety: `SpIBinder` always holds a valid pointer pointer to an
- // `AIBinder`, which we can safely pass to
- // `AIBinder_getUserData`. `ibinder` retains ownership of the
- // returned pointer.
- sys::AIBinder_getUserData(ibinder.as_native_mut())
- };
+ // Safety: `SpIBinder` always holds a valid pointer pointer to an
+ // `AIBinder`, which we can safely pass to `AIBinder_getUserData`.
+ // `ibinder` retains ownership of the returned pointer.
+ let userdata = unsafe { sys::AIBinder_getUserData(ibinder.as_native_mut()) };
if userdata.is_null() {
return Err(StatusCode::UNEXPECTED_NULL);
}
@@ -422,12 +434,10 @@
}
}
-/// # Safety
-///
-/// The constructor for `Binder` guarantees that `self.ibinder` will contain a
-/// valid, non-null pointer to an `AIBinder`, so this implementation is type
-/// safe. `self.ibinder` will remain valid for the entire lifetime of `self`
-/// because we hold a strong reference to the `AIBinder` until `self` is
+/// Safety: The constructor for `Binder` guarantees that `self.ibinder` will
+/// contain a valid, non-null pointer to an `AIBinder`, so this implementation
+/// is type safe. `self.ibinder` will remain valid for the entire lifetime of
+/// `self` because we hold a strong reference to the `AIBinder` until `self` is
/// dropped.
unsafe impl<B: Remotable> AsNative<sys::AIBinder> for Binder<B> {
fn as_native(&self) -> *const sys::AIBinder {
@@ -447,14 +457,12 @@
/// This function will panic if the identifier contains a 0 byte (NUL).
pub fn add_service(identifier: &str, mut binder: SpIBinder) -> Result<()> {
let instance = CString::new(identifier).unwrap();
- let status = unsafe {
- // Safety: `AServiceManager_addService` expects valid `AIBinder` and C
- // string pointers. Caller retains ownership of both
- // pointers. `AServiceManager_addService` creates a new strong reference
- // and copies the string, so both pointers need only be valid until the
- // call returns.
- sys::AServiceManager_addService(binder.as_native_mut(), instance.as_ptr())
- };
+ let status =
+ // Safety: `AServiceManager_addService` expects valid `AIBinder` and C
+ // string pointers. Caller retains ownership of both pointers.
+ // `AServiceManager_addService` creates a new strong reference and copies
+ // the string, so both pointers need only be valid until the call returns.
+ unsafe { sys::AServiceManager_addService(binder.as_native_mut(), instance.as_ptr()) };
status_result(status)
}
@@ -470,13 +478,12 @@
/// This function will panic if the identifier contains a 0 byte (NUL).
pub fn register_lazy_service(identifier: &str, mut binder: SpIBinder) -> Result<()> {
let instance = CString::new(identifier).unwrap();
+ // Safety: `AServiceManager_registerLazyService` expects valid `AIBinder` and C
+ // string pointers. Caller retains ownership of both
+ // pointers. `AServiceManager_registerLazyService` creates a new strong reference
+ // and copies the string, so both pointers need only be valid until the
+ // call returns.
let status = unsafe {
- // Safety: `AServiceManager_registerLazyService` expects valid `AIBinder` and C
- // string pointers. Caller retains ownership of both
- // pointers. `AServiceManager_registerLazyService` creates a new strong reference
- // and copies the string, so both pointers need only be valid until the
- // call returns.
-
sys::AServiceManager_registerLazyService(binder.as_native_mut(), instance.as_ptr())
};
status_result(status)
@@ -491,10 +498,8 @@
///
/// Consider using [`LazyServiceGuard`] rather than calling this directly.
pub fn force_lazy_services_persist(persist: bool) {
- unsafe {
- // Safety: No borrowing or transfer of ownership occurs here.
- sys::AServiceManager_forceLazyServicesPersist(persist)
- }
+ // Safety: No borrowing or transfer of ownership occurs here.
+ unsafe { sys::AServiceManager_forceLazyServicesPersist(persist) }
}
/// An RAII object to ensure a process which registers lazy services is not killed. During the
@@ -576,8 +581,6 @@
/// Determine whether the current thread is currently executing an incoming
/// transaction.
pub fn is_handling_transaction() -> bool {
- unsafe {
- // Safety: This method is always safe to call.
- sys::AIBinder_isHandlingTransaction()
- }
+ // Safety: This method is always safe to call.
+ unsafe { sys::AIBinder_isHandlingTransaction() }
}
diff --git a/libs/binder/rust/src/parcel.rs b/libs/binder/rust/src/parcel.rs
index e4c568e..3c615ed 100644
--- a/libs/binder/rust/src/parcel.rs
+++ b/libs/binder/rust/src/parcel.rs
@@ -52,11 +52,8 @@
ptr: NonNull<sys::AParcel>,
}
-/// # Safety
-///
-/// This type guarantees that it owns the AParcel and that all access to
-/// the AParcel happens through the Parcel, so it is ok to send across
-/// threads.
+/// Safety: This type guarantees that it owns the AParcel and that all access to
+/// the AParcel happens through the Parcel, so it is ok to send across threads.
unsafe impl Send for Parcel {}
/// Container for a message (data and object references) that can be sent
@@ -73,11 +70,9 @@
impl Parcel {
/// Create a new empty `Parcel`.
pub fn new() -> Parcel {
- let ptr = unsafe {
- // Safety: If `AParcel_create` succeeds, it always returns
- // a valid pointer. If it fails, the process will crash.
- sys::AParcel_create()
- };
+ // Safety: If `AParcel_create` succeeds, it always returns
+ // a valid pointer. If it fails, the process will crash.
+ let ptr = unsafe { sys::AParcel_create() };
Self { ptr: NonNull::new(ptr).expect("AParcel_create returned null pointer") }
}
@@ -171,10 +166,8 @@
}
}
-/// # Safety
-///
-/// The `Parcel` constructors guarantee that a `Parcel` object will always
-/// contain a valid pointer to an `AParcel`.
+/// Safety: The `Parcel` constructors guarantee that a `Parcel` object will
+/// always contain a valid pointer to an `AParcel`.
unsafe impl AsNative<sys::AParcel> for Parcel {
fn as_native(&self) -> *const sys::AParcel {
self.ptr.as_ptr()
@@ -185,10 +178,8 @@
}
}
-/// # Safety
-///
-/// The `BorrowedParcel` constructors guarantee that a `BorrowedParcel` object
-/// will always contain a valid pointer to an `AParcel`.
+/// Safety: The `BorrowedParcel` constructors guarantee that a `BorrowedParcel`
+/// object will always contain a valid pointer to an `AParcel`.
unsafe impl<'a> AsNative<sys::AParcel> for BorrowedParcel<'a> {
fn as_native(&self) -> *const sys::AParcel {
self.ptr.as_ptr()
@@ -203,10 +194,8 @@
impl<'a> BorrowedParcel<'a> {
/// Data written to parcelable is zero'd before being deleted or reallocated.
pub fn mark_sensitive(&mut self) {
- unsafe {
- // Safety: guaranteed to have a parcel object, and this method never fails
- sys::AParcel_markSensitive(self.as_native())
- }
+ // Safety: guaranteed to have a parcel object, and this method never fails
+ unsafe { sys::AParcel_markSensitive(self.as_native()) }
}
/// Write a type that implements [`Serialize`] to the parcel.
@@ -265,11 +254,15 @@
f(&mut subparcel)?;
}
let end = self.get_data_position();
+ // Safety: start is less than the current size of the parcel data
+ // buffer, because we just got it with `get_data_position`.
unsafe {
self.set_data_position(start)?;
}
assert!(end >= start);
self.write(&(end - start))?;
+ // Safety: end is less than the current size of the parcel data
+ // buffer, because we just got it with `get_data_position`.
unsafe {
self.set_data_position(end)?;
}
@@ -278,20 +271,16 @@
/// Returns the current position in the parcel data.
pub fn get_data_position(&self) -> i32 {
- unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`, and this call is otherwise safe.
- sys::AParcel_getDataPosition(self.as_native())
- }
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`, and this call is otherwise safe.
+ unsafe { sys::AParcel_getDataPosition(self.as_native()) }
}
/// Returns the total size of the parcel.
pub fn get_data_size(&self) -> i32 {
- unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`, and this call is otherwise safe.
- sys::AParcel_getDataSize(self.as_native())
- }
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`, and this call is otherwise safe.
+ unsafe { sys::AParcel_getDataSize(self.as_native()) }
}
/// Move the current read/write position in the parcel.
@@ -304,7 +293,9 @@
/// accesses are bounds checked, this call is still safe, but we can't rely
/// on that.
pub unsafe fn set_data_position(&self, pos: i32) -> Result<()> {
- status_result(sys::AParcel_setDataPosition(self.as_native(), pos))
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`, and the caller guarantees that `pos` is within bounds.
+ status_result(unsafe { sys::AParcel_setDataPosition(self.as_native(), pos) })
}
/// Append a subset of another parcel.
@@ -317,10 +308,10 @@
start: i32,
size: i32,
) -> Result<()> {
+ // Safety: `Parcel::appendFrom` from C++ checks that `start`
+ // and `size` are in bounds, and returns an error otherwise.
+ // Both `self` and `other` always contain valid pointers.
let status = unsafe {
- // Safety: `Parcel::appendFrom` from C++ checks that `start`
- // and `size` are in bounds, and returns an error otherwise.
- // Both `self` and `other` always contain valid pointers.
sys::AParcel_appendFrom(other.as_native(), self.as_native_mut(), start, size)
};
status_result(status)
@@ -418,7 +409,9 @@
/// accesses are bounds checked, this call is still safe, but we can't rely
/// on that.
pub unsafe fn set_data_position(&self, pos: i32) -> Result<()> {
- self.borrowed_ref().set_data_position(pos)
+ // Safety: We have the same safety requirements as
+ // `BorrowedParcel::set_data_position`.
+ unsafe { self.borrowed_ref().set_data_position(pos) }
}
/// Append a subset of another parcel.
@@ -461,7 +454,7 @@
/// and call a closure with the sub-parcel as its parameter.
/// The closure can keep reading data from the sub-parcel
/// until it runs out of input data. The closure is responsible
- /// for calling [`ReadableSubParcel::has_more_data`] to check for
+ /// for calling `ReadableSubParcel::has_more_data` to check for
/// more data before every read, at least until Rust generators
/// are stabilized.
/// After the closure returns, skip to the end of the current
@@ -504,7 +497,10 @@
f(subparcel)?;
// Advance the data position to the actual end,
- // in case the closure read less data than was available
+ // in case the closure read less data than was available.
+ //
+ // Safety: end must be less than the current size of the parcel, because
+ // we checked above against `get_data_size`.
unsafe {
self.set_data_position(end)?;
}
@@ -595,7 +591,7 @@
/// and call a closure with the sub-parcel as its parameter.
/// The closure can keep reading data from the sub-parcel
/// until it runs out of input data. The closure is responsible
- /// for calling [`ReadableSubParcel::has_more_data`] to check for
+ /// for calling `ReadableSubParcel::has_more_data` to check for
/// more data before every read, at least until Rust generators
/// are stabilized.
/// After the closure returns, skip to the end of the current
@@ -649,17 +645,17 @@
// Internal APIs
impl<'a> BorrowedParcel<'a> {
pub(crate) fn write_binder(&mut self, binder: Option<&SpIBinder>) -> Result<()> {
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`. `AsNative` for `Option<SpIBinder`> will either return
+ // null or a valid pointer to an `AIBinder`, both of which are
+ // valid, safe inputs to `AParcel_writeStrongBinder`.
+ //
+ // This call does not take ownership of the binder. However, it does
+ // require a mutable pointer, which we cannot extract from an
+ // immutable reference, so we clone the binder, incrementing the
+ // refcount before the call. The refcount will be immediately
+ // decremented when this temporary is dropped.
unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`. `AsNative` for `Option<SpIBinder`> will either return
- // null or a valid pointer to an `AIBinder`, both of which are
- // valid, safe inputs to `AParcel_writeStrongBinder`.
- //
- // This call does not take ownership of the binder. However, it does
- // require a mutable pointer, which we cannot extract from an
- // immutable reference, so we clone the binder, incrementing the
- // refcount before the call. The refcount will be immediately
- // decremented when this temporary is dropped.
status_result(sys::AParcel_writeStrongBinder(
self.as_native_mut(),
binder.cloned().as_native_mut(),
@@ -669,33 +665,28 @@
pub(crate) fn read_binder(&self) -> Result<Option<SpIBinder>> {
let mut binder = ptr::null_mut();
- let status = unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`. We pass a valid, mutable out pointer to the `binder`
- // parameter. After this call, `binder` will be either null or a
- // valid pointer to an `AIBinder` owned by the caller.
- sys::AParcel_readStrongBinder(self.as_native(), &mut binder)
- };
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`. We pass a valid, mutable out pointer to the `binder`
+ // parameter. After this call, `binder` will be either null or a
+ // valid pointer to an `AIBinder` owned by the caller.
+ let status = unsafe { sys::AParcel_readStrongBinder(self.as_native(), &mut binder) };
status_result(status)?;
- Ok(unsafe {
- // Safety: `binder` is either null or a valid, owned pointer at this
- // point, so can be safely passed to `SpIBinder::from_raw`.
- SpIBinder::from_raw(binder)
- })
+ // Safety: `binder` is either null or a valid, owned pointer at this
+ // point, so can be safely passed to `SpIBinder::from_raw`.
+ Ok(unsafe { SpIBinder::from_raw(binder) })
}
}
impl Drop for Parcel {
fn drop(&mut self) {
// Run the C++ Parcel complete object destructor
- unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. Since we own the parcel, we can safely delete it
- // here.
- sys::AParcel_delete(self.ptr.as_ptr())
- }
+ //
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. Since we own the parcel, we can safely delete it
+ // here.
+ unsafe { sys::AParcel_delete(self.ptr.as_ptr()) }
}
}
@@ -732,6 +723,8 @@
parcel.write(&1i32).unwrap();
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
parcel.set_data_position(start).unwrap();
}
@@ -748,6 +741,8 @@
parcel.write(&b"Hello, Binder!\0"[..]).unwrap();
// Skip over string length
+ // SAFETY: str_start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(str_start).is_ok());
}
@@ -756,42 +751,56 @@
assert!(parcel.read::<bool>().unwrap());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<i8>().unwrap(), 72i8);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<u16>().unwrap(), 25928);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<i32>().unwrap(), 1819043144);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<u32>().unwrap(), 1819043144);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<i64>().unwrap(), 4764857262830019912);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<u64>().unwrap(), 4764857262830019912);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -799,6 +808,8 @@
assert_eq!(parcel.read::<f32>().unwrap(), 1143139100000000000000000000.0);
assert_eq!(parcel.read::<f32>().unwrap(), 40.043392);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -806,6 +817,8 @@
assert_eq!(parcel.read::<f64>().unwrap(), 34732488246.197815);
// Skip back to before the string length
+ // SAFETY: str_start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(str_start).is_ok());
}
@@ -819,15 +832,21 @@
let start = parcel.get_data_position();
assert!(parcel.write("Hello, Binder!").is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<Option<String>>().unwrap().unwrap(), "Hello, Binder!",);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(parcel.write("Embedded null \0 inside a string").is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -835,6 +854,8 @@
parcel.read::<Option<String>>().unwrap().unwrap(),
"Embedded null \0 inside a string",
);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -849,6 +870,8 @@
let s3 = "Some more text here.";
assert!(parcel.write(&[s1, s2, s3][..]).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -874,6 +897,8 @@
assert_eq!(parcel.get_data_position(), start + expected_len);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
parcel.set_data_position(start).unwrap();
}
@@ -893,6 +918,8 @@
assert_eq!(4, parcel2.get_data_size());
assert_eq!(Ok(()), parcel2.append_all_from(&parcel1));
assert_eq!(8, parcel2.get_data_size());
+ // SAFETY: 0 is less than the current size of the parcel data buffer, because the parcel is not
+ // empty.
unsafe {
parcel2.set_data_position(0).unwrap();
}
@@ -903,6 +930,8 @@
assert_eq!(Ok(()), parcel2.append_from(&parcel1, 0, 2));
assert_eq!(Ok(()), parcel2.append_from(&parcel1, 2, 2));
assert_eq!(4, parcel2.get_data_size());
+ // SAFETY: 0 is less than the current size of the parcel data buffer, because the parcel is not
+ // empty.
unsafe {
parcel2.set_data_position(0).unwrap();
}
@@ -911,6 +940,8 @@
let mut parcel2 = Parcel::new();
assert_eq!(Ok(()), parcel2.append_from(&parcel1, 0, 2));
assert_eq!(2, parcel2.get_data_size());
+ // SAFETY: 0 is less than the current size of the parcel data buffer, because the parcel is not
+ // empty.
unsafe {
parcel2.set_data_position(0).unwrap();
}
diff --git a/libs/binder/rust/src/parcel/file_descriptor.rs b/libs/binder/rust/src/parcel/file_descriptor.rs
index 7fe37f3..5c688fa 100644
--- a/libs/binder/rust/src/parcel/file_descriptor.rs
+++ b/libs/binder/rust/src/parcel/file_descriptor.rs
@@ -73,14 +73,12 @@
impl Serialize for ParcelFileDescriptor {
fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
let fd = self.0.as_raw_fd();
- let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. Likewise, `ParcelFileDescriptor` always contains a
- // valid file, so we can borrow a valid file
- // descriptor. `AParcel_writeParcelFileDescriptor` does NOT take
- // ownership of the fd, so we need not duplicate it first.
- sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), fd)
- };
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. Likewise, `ParcelFileDescriptor` always contains a
+ // valid file, so we can borrow a valid file
+ // descriptor. `AParcel_writeParcelFileDescriptor` does NOT take
+ // ownership of the fd, so we need not duplicate it first.
+ let status = unsafe { sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), fd) };
status_result(status)
}
}
@@ -92,13 +90,12 @@
if let Some(f) = this {
f.serialize(parcel)
} else {
- let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. `AParcel_writeParcelFileDescriptor` accepts the
- // value `-1` as the file descriptor to signify serializing a
- // null file descriptor.
- sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), -1i32)
- };
+ let status =
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. `AParcel_writeParcelFileDescriptor` accepts the
+ // value `-1` as the file descriptor to signify serializing a
+ // null file descriptor.
+ unsafe { sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), -1i32) };
status_result(status)
}
}
@@ -107,25 +104,23 @@
impl DeserializeOption for ParcelFileDescriptor {
fn deserialize_option(parcel: &BorrowedParcel<'_>) -> Result<Option<Self>> {
let mut fd = -1i32;
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. We pass a valid mutable pointer to an i32, which
+ // `AParcel_readParcelFileDescriptor` assigns the valid file
+ // descriptor into, or `-1` if deserializing a null file
+ // descriptor. The read function passes ownership of the file
+ // descriptor to its caller if it was non-null, so we must take
+ // ownership of the file and ensure that it is eventually closed.
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. We pass a valid mutable pointer to an i32, which
- // `AParcel_readParcelFileDescriptor` assigns the valid file
- // descriptor into, or `-1` if deserializing a null file
- // descriptor. The read function passes ownership of the file
- // descriptor to its caller if it was non-null, so we must take
- // ownership of the file and ensure that it is eventually closed.
status_result(sys::AParcel_readParcelFileDescriptor(parcel.as_native(), &mut fd))?;
}
if fd < 0 {
Ok(None)
} else {
- let file = unsafe {
- // Safety: At this point, we know that the file descriptor was
- // not -1, so must be a valid, owned file descriptor which we
- // can safely turn into a `File`.
- File::from_raw_fd(fd)
- };
+ // Safety: At this point, we know that the file descriptor was
+ // not -1, so must be a valid, owned file descriptor which we
+ // can safely turn into a `File`.
+ let file = unsafe { File::from_raw_fd(fd) };
Ok(Some(ParcelFileDescriptor::new(file)))
}
}
diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs
index 5d8c11c..9008a3c 100644
--- a/libs/binder/rust/src/parcel/parcelable.rs
+++ b/libs/binder/rust/src/parcel/parcelable.rs
@@ -50,14 +50,14 @@
fn read_from_parcel(&mut self, parcel: &BorrowedParcel<'_>) -> Result<()>;
}
-/// A struct whose instances can be written to a [`Parcel`].
+/// A struct whose instances can be written to a [`crate::parcel::Parcel`].
// Might be able to hook this up as a serde backend in the future?
pub trait Serialize {
- /// Serialize this instance into the given [`Parcel`].
+ /// Serialize this instance into the given [`crate::parcel::Parcel`].
fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()>;
}
-/// A struct whose instances can be restored from a [`Parcel`].
+/// A struct whose instances can be restored from a [`crate::parcel::Parcel`].
// Might be able to hook this up as a serde backend in the future?
pub trait Deserialize: Sized {
/// Type for the uninitialized value of this type. Will be either `Self`
@@ -80,10 +80,10 @@
/// Convert an initialized value of type `Self` into `Self::UninitType`.
fn from_init(value: Self) -> Self::UninitType;
- /// Deserialize an instance from the given [`Parcel`].
+ /// Deserialize an instance from the given [`crate::parcel::Parcel`].
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self>;
- /// Deserialize an instance from the given [`Parcel`] onto the
+ /// Deserialize an instance from the given [`crate::parcel::Parcel`] onto the
/// current object. This operation will overwrite the old value
/// partially or completely, depending on how much data is available.
fn deserialize_from(&mut self, parcel: &BorrowedParcel<'_>) -> Result<()> {
@@ -102,8 +102,8 @@
pub trait SerializeArray: Serialize + Sized {
/// Serialize an array of this type into the given parcel.
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: Safe FFI, slice will always be a safe pointer to pass.
let res = unsafe {
- // Safety: Safe FFI, slice will always be a safe pointer to pass.
sys::AParcel_writeParcelableArray(
parcel.as_native_mut(),
slice.as_ptr() as *const c_void,
@@ -117,7 +117,9 @@
/// Callback to serialize an element of a generic parcelable array.
///
-/// Safety: We are relying on binder_ndk to not overrun our slice. As long as it
+/// # Safety
+///
+/// We are relying on binder_ndk to not overrun our slice. As long as it
/// doesn't provide an index larger than the length of the original slice in
/// serialize_array, this operation is safe. The index provided is zero-based.
unsafe extern "C" fn serialize_element<T: Serialize>(
@@ -125,9 +127,14 @@
array: *const c_void,
index: usize,
) -> status_t {
- let slice: &[T] = slice::from_raw_parts(array.cast(), index + 1);
+ // Safety: The caller guarantees that `array` is a valid pointer of the
+ // appropriate type.
+ let slice: &[T] = unsafe { slice::from_raw_parts(array.cast(), index + 1) };
- let mut parcel = match BorrowedParcel::from_raw(parcel) {
+ // Safety: The caller must give us a parcel pointer which is either null or
+ // valid at least for the duration of this function call. We don't keep the
+ // resulting value beyond the function.
+ let mut parcel = match unsafe { BorrowedParcel::from_raw(parcel) } {
None => return StatusCode::UNEXPECTED_NULL as status_t,
Some(p) => p,
};
@@ -142,9 +149,9 @@
/// Deserialize an array of type from the given parcel.
fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> {
let mut vec: Option<Vec<Self::UninitType>> = None;
+ // Safety: Safe FFI, vec is the correct opaque type expected by
+ // allocate_vec and deserialize_element.
let res = unsafe {
- // Safety: Safe FFI, vec is the correct opaque type expected by
- // allocate_vec and deserialize_element.
sys::AParcel_readParcelableArray(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
@@ -153,21 +160,21 @@
)
};
status_result(res)?;
- let vec: Option<Vec<Self>> = unsafe {
- // Safety: We are assuming that the NDK correctly initialized every
- // element of the vector by now, so we know that all the
- // UninitTypes are now properly initialized. We can transmute from
- // Vec<T::UninitType> to Vec<T> because T::UninitType has the same
- // alignment and size as T, so the pointer to the vector allocation
- // will be compatible.
- mem::transmute(vec)
- };
+ // Safety: We are assuming that the NDK correctly initialized every
+ // element of the vector by now, so we know that all the
+ // UninitTypes are now properly initialized. We can transmute from
+ // Vec<T::UninitType> to Vec<T> because T::UninitType has the same
+ // alignment and size as T, so the pointer to the vector allocation
+ // will be compatible.
+ let vec: Option<Vec<Self>> = unsafe { mem::transmute(vec) };
Ok(vec)
}
}
/// Callback to deserialize a parcelable element.
///
+/// # Safety
+///
/// The opaque array data pointer must be a mutable pointer to an
/// `Option<Vec<T::UninitType>>` with at least enough elements for `index` to be valid
/// (zero-based).
@@ -176,13 +183,18 @@
array: *mut c_void,
index: usize,
) -> status_t {
- let vec = &mut *(array as *mut Option<Vec<T::UninitType>>);
+ // Safety: The caller guarantees that `array` is a valid pointer of the
+ // appropriate type.
+ let vec = unsafe { &mut *(array as *mut Option<Vec<T::UninitType>>) };
let vec = match vec {
Some(v) => v,
None => return StatusCode::BAD_INDEX as status_t,
};
- let parcel = match BorrowedParcel::from_raw(parcel as *mut _) {
+ // Safety: The caller must give us a parcel pointer which is either null or
+ // valid at least for the duration of this function call. We don't keep the
+ // resulting value beyond the function.
+ let parcel = match unsafe { BorrowedParcel::from_raw(parcel as *mut _) } {
None => return StatusCode::UNEXPECTED_NULL as status_t,
Some(p) => p,
};
@@ -254,16 +266,21 @@
///
/// The opaque data pointer passed to the array read function must be a mutable
/// pointer to an `Option<Vec<T::UninitType>>`. `buffer` will be assigned a mutable pointer
-/// to the allocated vector data if this function returns true.
+/// to the allocated vector data if this function returns true. `buffer` must be a valid pointer.
unsafe extern "C" fn allocate_vec_with_buffer<T: Deserialize>(
data: *mut c_void,
len: i32,
buffer: *mut *mut T,
) -> bool {
- let res = allocate_vec::<T>(data, len);
- let vec = &mut *(data as *mut Option<Vec<T::UninitType>>);
+ // Safety: We have the same safety requirements as `allocate_vec` for `data`.
+ let res = unsafe { allocate_vec::<T>(data, len) };
+ // Safety: The caller guarantees that `data` is a valid mutable pointer to the appropriate type.
+ let vec = unsafe { &mut *(data as *mut Option<Vec<T::UninitType>>) };
if let Some(new_vec) = vec {
- *buffer = new_vec.as_mut_ptr() as *mut T;
+ // Safety: The caller guarantees that `buffer` is a valid pointer.
+ unsafe {
+ *buffer = new_vec.as_mut_ptr() as *mut T;
+ }
}
res
}
@@ -275,7 +292,8 @@
/// The opaque data pointer passed to the array read function must be a mutable
/// pointer to an `Option<Vec<T::UninitType>>`.
unsafe extern "C" fn allocate_vec<T: Deserialize>(data: *mut c_void, len: i32) -> bool {
- let vec = &mut *(data as *mut Option<Vec<T::UninitType>>);
+ // Safety: The caller guarantees that `data` is a valid mutable pointer to the appropriate type.
+ let vec = unsafe { &mut *(data as *mut Option<Vec<T::UninitType>>) };
if len < 0 {
*vec = None;
return true;
@@ -286,7 +304,10 @@
let mut new_vec: Vec<T::UninitType> = Vec::with_capacity(len as usize);
new_vec.resize_with(len as usize, T::uninit);
- ptr::write(vec, Some(new_vec));
+ // Safety: The caller guarantees that vec is a valid mutable pointer to the appropriate type.
+ unsafe {
+ ptr::write(vec, Some(new_vec));
+ }
true
}
@@ -305,21 +326,21 @@
// Assert at compile time that `T` and `T::UninitType` have the same size and alignment.
let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT;
- // We can convert from Vec<T::UninitType> to Vec<T> because T::UninitType
- // has the same alignment and size as T, so the pointer to the vector
- // allocation will be compatible.
let mut vec = ManuallyDrop::new(vec);
- Vec::from_raw_parts(vec.as_mut_ptr().cast(), vec.len(), vec.capacity())
+ // Safety: We can convert from Vec<T::UninitType> to Vec<T> because
+ // T::UninitType has the same alignment and size as T, so the pointer to the
+ // vector allocation will be compatible.
+ unsafe { Vec::from_raw_parts(vec.as_mut_ptr().cast(), vec.len(), vec.capacity()) }
}
macro_rules! impl_parcelable {
{Serialize, $ty:ty, $write_fn:path} => {
impl Serialize for $ty {
fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`, and any `$ty` literal value is safe to pass to
+ // `$write_fn`.
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`, and any `$ty` literal value is safe to pass to
- // `$write_fn`.
status_result($write_fn(parcel.as_native_mut(), *self))
}
}
@@ -333,11 +354,11 @@
fn from_init(value: Self) -> Self::UninitType { value }
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut val = Self::default();
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. We pass a valid, mutable pointer to `val`, a
+ // literal of type `$ty`, and `$read_fn` will write the
+ // value read into `val` if successful
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. We pass a valid, mutable pointer to `val`, a
- // literal of type `$ty`, and `$read_fn` will write the
- // value read into `val` if successful
status_result($read_fn(parcel.as_native(), &mut val))?
};
Ok(val)
@@ -348,13 +369,13 @@
{SerializeArray, $ty:ty, $write_array_fn:path} => {
impl SerializeArray for $ty {
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the slice is > 0 length, `slice.as_ptr()`
+ // will be a valid pointer to an array of elements of type
+ // `$ty`. If the slice length is 0, `slice.as_ptr()` may be
+ // dangling, but this is safe since the pointer is not
+ // dereferenced if the length parameter is 0.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the slice is > 0 length, `slice.as_ptr()`
- // will be a valid pointer to an array of elements of type
- // `$ty`. If the slice length is 0, `slice.as_ptr()` may be
- // dangling, but this is safe since the pointer is not
- // dereferenced if the length parameter is 0.
$write_array_fn(
parcel.as_native_mut(),
slice.as_ptr(),
@@ -373,11 +394,11 @@
impl DeserializeArray for $ty {
fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> {
let mut vec: Option<Vec<Self::UninitType>> = None;
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. `allocate_vec<T>` expects the opaque pointer to
+ // be of type `*mut Option<Vec<T::UninitType>>`, so `&mut vec` is
+ // correct for it.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. `allocate_vec<T>` expects the opaque pointer to
- // be of type `*mut Option<Vec<T::UninitType>>`, so `&mut vec` is
- // correct for it.
$read_array_fn(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
@@ -385,11 +406,11 @@
)
};
status_result(status)?;
+ // Safety: We are assuming that the NDK correctly
+ // initialized every element of the vector by now, so we
+ // know that all the UninitTypes are now properly
+ // initialized.
let vec: Option<Vec<Self>> = unsafe {
- // Safety: We are assuming that the NDK correctly
- // initialized every element of the vector by now, so we
- // know that all the UninitTypes are now properly
- // initialized.
vec.map(|vec| vec_assume_init(vec))
};
Ok(vec)
@@ -479,13 +500,13 @@
impl SerializeArray for u8 {
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
+ // valid pointer to an array of elements of type `$ty`. If the slice
+ // length is 0, `slice.as_ptr()` may be dangling, but this is safe
+ // since the pointer is not dereferenced if the length parameter is
+ // 0.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
- // valid pointer to an array of elements of type `$ty`. If the slice
- // length is 0, `slice.as_ptr()` may be dangling, but this is safe
- // since the pointer is not dereferenced if the length parameter is
- // 0.
sys::AParcel_writeByteArray(
parcel.as_native_mut(),
slice.as_ptr() as *const i8,
@@ -518,13 +539,13 @@
impl SerializeArray for i16 {
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
+ // valid pointer to an array of elements of type `$ty`. If the slice
+ // length is 0, `slice.as_ptr()` may be dangling, but this is safe
+ // since the pointer is not dereferenced if the length parameter is
+ // 0.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
- // valid pointer to an array of elements of type `$ty`. If the slice
- // length is 0, `slice.as_ptr()` may be dangling, but this is safe
- // since the pointer is not dereferenced if the length parameter is
- // 0.
sys::AParcel_writeCharArray(
parcel.as_native_mut(),
slice.as_ptr() as *const u16,
@@ -538,22 +559,22 @@
impl SerializeOption for str {
fn serialize_option(this: Option<&Self>, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
match this {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the string pointer is null,
+ // `AParcel_writeString` requires that the length is -1 to
+ // indicate that we want to serialize a null string.
None => unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the string pointer is null,
- // `AParcel_writeString` requires that the length is -1 to
- // indicate that we want to serialize a null string.
status_result(sys::AParcel_writeString(parcel.as_native_mut(), ptr::null(), -1))
},
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `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.
Some(s) => unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `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() as *const c_char,
@@ -597,11 +618,11 @@
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut vec: Option<Vec<u8>> = None;
+ // Safety: `Parcel` always contains a valid pointer to an `AParcel`.
+ // `Option<Vec<u8>>` is equivalent to the expected `Option<Vec<i8>>`
+ // for `allocate_vec`, so `vec` is safe to pass as the opaque data
+ // pointer on platforms where char is signed.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an `AParcel`.
- // `Option<Vec<u8>>` is equivalent to the expected `Option<Vec<i8>>`
- // for `allocate_vec`, so `vec` is safe to pass as the opaque data
- // pointer on platforms where char is signed.
sys::AParcel_readString(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
@@ -751,11 +772,11 @@
impl Serialize for Status {
fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an `AParcel`
+ // and `Status` always contains a valid pointer to an `AStatus`, so
+ // both parameters are valid and safe. This call does not take
+ // ownership of either of its parameters.
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an `AParcel`
- // and `Status` always contains a valid pointer to an `AStatus`, so
- // both parameters are valid and safe. This call does not take
- // ownership of either of its parameters.
status_result(sys::AParcel_writeStatusHeader(parcel.as_native_mut(), self.as_native()))
}
}
@@ -772,21 +793,18 @@
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut status_ptr = ptr::null_mut();
- let ret_status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. We pass a mutable out pointer which will be
- // assigned a valid `AStatus` pointer if the function returns
- // status OK. This function passes ownership of the status
- // pointer to the caller, if it was assigned.
- sys::AParcel_readStatusHeader(parcel.as_native(), &mut status_ptr)
- };
+ let ret_status =
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. We pass a mutable out pointer which will be
+ // assigned a valid `AStatus` pointer if the function returns
+ // status OK. This function passes ownership of the status
+ // pointer to the caller, if it was assigned.
+ unsafe { sys::AParcel_readStatusHeader(parcel.as_native(), &mut status_ptr) };
status_result(ret_status)?;
- Ok(unsafe {
- // Safety: At this point, the return status of the read call was ok,
- // so we know that `status_ptr` is a valid, owned pointer to an
- // `AStatus`, from which we can safely construct a `Status` object.
- Status::from_ptr(status_ptr)
- })
+ // Safety: At this point, the return status of the read call was ok,
+ // so we know that `status_ptr` is a valid, owned pointer to an
+ // `AStatus`, from which we can safely construct a `Status` object.
+ Ok(unsafe { Status::from_ptr(status_ptr) })
}
}
@@ -880,7 +898,6 @@
/// `Serialize`, `SerializeArray` and `SerializeOption` for
/// structured parcelables. The target type must implement the
/// `Parcelable` trait.
-/// ```
#[macro_export]
macro_rules! impl_serialize_for_parcelable {
($parcelable:ident) => {
@@ -1070,6 +1087,8 @@
assert!(custom.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1092,6 +1111,8 @@
assert!(bools.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1101,6 +1122,8 @@
assert_eq!(parcel.read::<u32>().unwrap(), 0);
assert_eq!(parcel.read::<u32>().unwrap(), 0);
assert_eq!(parcel.read::<u32>().unwrap(), 1);
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1116,12 +1139,17 @@
assert!(parcel.write(&u8s[..]).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<u32>().unwrap(), 4); // 4 items
assert_eq!(parcel.read::<u32>().unwrap(), 0x752aff65); // bytes
+
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1131,18 +1159,25 @@
let i8s = [-128i8, 127, 42, -117];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(parcel.write(&i8s[..]).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert_eq!(parcel.read::<u32>().unwrap(), 4); // 4 items
assert_eq!(parcel.read::<u32>().unwrap(), 0x8b2a7f80); // bytes
+
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1152,10 +1187,14 @@
let u16s = [u16::max_value(), 12_345, 42, 117];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(u16s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1165,6 +1204,9 @@
assert_eq!(parcel.read::<u32>().unwrap(), 12345); // 12,345
assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42
assert_eq!(parcel.read::<u32>().unwrap(), 117); // 117
+
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1175,10 +1217,14 @@
let i16s = [i16::max_value(), i16::min_value(), 42, -117];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(i16s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1188,6 +1234,9 @@
assert_eq!(parcel.read::<u32>().unwrap(), 0x8000); // i16::min_value()
assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42
assert_eq!(parcel.read::<u32>().unwrap(), 0xff8b); // -117
+
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1198,10 +1247,14 @@
let u32s = [u32::max_value(), 12_345, 42, 117];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(u32s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1211,6 +1264,9 @@
assert_eq!(parcel.read::<u32>().unwrap(), 12345); // 12,345
assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42
assert_eq!(parcel.read::<u32>().unwrap(), 117); // 117
+
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1221,10 +1277,14 @@
let i32s = [i32::max_value(), i32::min_value(), 42, -117];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(i32s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1234,6 +1294,9 @@
assert_eq!(parcel.read::<u32>().unwrap(), 0x80000000); // i32::min_value()
assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42
assert_eq!(parcel.read::<u32>().unwrap(), 0xffffff8b); // -117
+
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1244,10 +1307,14 @@
let u64s = [u64::max_value(), 12_345, 42, 117];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(u64s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1258,10 +1325,14 @@
let i64s = [i64::max_value(), i64::min_value(), 42, -117];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(i64s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1272,10 +1343,14 @@
let f32s = [std::f32::NAN, std::f32::INFINITY, 1.23456789, std::f32::EPSILON];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(f32s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1288,10 +1363,14 @@
let f64s = [std::f64::NAN, std::f64::INFINITY, 1.234567890123456789, std::f64::EPSILON];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(f64s.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
@@ -1309,10 +1388,14 @@
let strs = [s1, s2, s3, s4];
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
assert!(strs.serialize(&mut parcel.borrowed()).is_ok());
+ // SAFETY: start is less than the current size of the parcel data buffer, because we haven't
+ // made it any shorter since we got the position.
unsafe {
assert!(parcel.set_data_position(start).is_ok());
}
diff --git a/libs/binder/rust/src/parcel/parcelable_holder.rs b/libs/binder/rust/src/parcel/parcelable_holder.rs
index eb82fb7..f906113 100644
--- a/libs/binder/rust/src/parcel/parcelable_holder.rs
+++ b/libs/binder/rust/src/parcel/parcelable_holder.rs
@@ -133,8 +133,8 @@
}
}
ParcelableHolderData::Parcel(ref mut parcel) => {
+ // Safety: 0 should always be a valid position.
unsafe {
- // Safety: 0 should always be a valid position.
parcel.set_data_position(0)?;
}
@@ -214,15 +214,15 @@
parcelable.write_to_parcel(parcel)?;
let end = parcel.get_data_position();
+ // Safety: we got the position from `get_data_position`.
unsafe {
- // Safety: we got the position from `get_data_position`.
parcel.set_data_position(length_start)?;
}
assert!(end >= data_start);
parcel.write(&(end - data_start))?;
+ // Safety: we got the position from `get_data_position`.
unsafe {
- // Safety: we got the position from `get_data_position`.
parcel.set_data_position(end)?;
}
@@ -260,11 +260,11 @@
new_parcel.append_from(parcel, data_start, data_size)?;
*self.data.get_mut().unwrap() = ParcelableHolderData::Parcel(new_parcel);
+ // Safety: `append_from` checks if `data_size` overflows
+ // `parcel` and returns `BAD_VALUE` if that happens. We also
+ // explicitly check for negative and zero `data_size` above,
+ // so `data_end` is guaranteed to be greater than `data_start`.
unsafe {
- // Safety: `append_from` checks if `data_size` overflows
- // `parcel` and returns `BAD_VALUE` if that happens. We also
- // explicitly check for negative and zero `data_size` above,
- // so `data_end` is guaranteed to be greater than `data_start`.
parcel.set_data_position(data_end)?;
}
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 036f6b4..dad3379 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -49,14 +49,12 @@
}
}
-/// # Safety
-///
-/// An `SpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe
+/// Safety: An `SpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Send for SpIBinder {}
-/// # Safety
-///
-/// An `SpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe
+/// Safety: An `SpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Sync for SpIBinder {}
impl SpIBinder {
@@ -97,11 +95,9 @@
/// Return true if this binder object is hosted in a different process than
/// the current one.
pub fn is_remote(&self) -> bool {
- unsafe {
- // Safety: `SpIBinder` guarantees that it always contains a valid
- // `AIBinder` pointer.
- sys::AIBinder_isRemote(self.as_native())
- }
+ // Safety: `SpIBinder` guarantees that it always contains a valid
+ // `AIBinder` pointer.
+ unsafe { sys::AIBinder_isRemote(self.as_native()) }
}
/// Try to convert this Binder object into a trait object for the given
@@ -116,12 +112,12 @@
/// Return the interface class of this binder object, if associated with
/// one.
pub fn get_class(&mut self) -> Option<InterfaceClass> {
+ // Safety: `SpIBinder` guarantees that it always contains a valid
+ // `AIBinder` pointer. `AIBinder_getClass` returns either a null
+ // pointer or a valid pointer to an `AIBinder_Class`. After mapping
+ // null to None, we can safely construct an `InterfaceClass` if the
+ // pointer was non-null.
unsafe {
- // Safety: `SpIBinder` guarantees that it always contains a valid
- // `AIBinder` pointer. `AIBinder_getClass` returns either a null
- // pointer or a valid pointer to an `AIBinder_Class`. After mapping
- // null to None, we can safely construct an `InterfaceClass` if the
- // pointer was non-null.
let class = sys::AIBinder_getClass(self.as_native_mut());
class.as_ref().map(|p| InterfaceClass::from_ptr(p))
}
@@ -152,7 +148,8 @@
///
/// See `SpIBinder::from_raw`.
pub unsafe fn new_spibinder(ptr: *mut sys::AIBinder) -> Option<SpIBinder> {
- SpIBinder::from_raw(ptr)
+ // Safety: The caller makes the same guarantees as this requires.
+ unsafe { SpIBinder::from_raw(ptr) }
}
}
@@ -171,30 +168,24 @@
impl AssociateClass for SpIBinder {
fn associate_class(&mut self, class: InterfaceClass) -> bool {
- unsafe {
- // Safety: `SpIBinder` guarantees that it always contains a valid
- // `AIBinder` pointer. An `InterfaceClass` can always be converted
- // into a valid `AIBinder_Class` pointer, so these parameters are
- // always safe.
- sys::AIBinder_associateClass(self.as_native_mut(), class.into())
- }
+ // Safety: `SpIBinder` guarantees that it always contains a valid
+ // `AIBinder` pointer. An `InterfaceClass` can always be converted
+ // into a valid `AIBinder_Class` pointer, so these parameters are
+ // always safe.
+ unsafe { sys::AIBinder_associateClass(self.as_native_mut(), class.into()) }
}
}
impl Ord for SpIBinder {
fn cmp(&self, other: &Self) -> Ordering {
- let less_than = unsafe {
- // Safety: SpIBinder always holds a valid `AIBinder` pointer, so
- // this pointer is always safe to pass to `AIBinder_lt` (null is
- // also safe to pass to this function, but we should never do that).
- sys::AIBinder_lt(self.0.as_ptr(), other.0.as_ptr())
- };
- let greater_than = unsafe {
- // Safety: SpIBinder always holds a valid `AIBinder` pointer, so
- // this pointer is always safe to pass to `AIBinder_lt` (null is
- // also safe to pass to this function, but we should never do that).
- sys::AIBinder_lt(other.0.as_ptr(), self.0.as_ptr())
- };
+ // Safety: SpIBinder always holds a valid `AIBinder` pointer, so this
+ // pointer is always safe to pass to `AIBinder_lt` (null is also safe to
+ // pass to this function, but we should never do that).
+ let less_than = unsafe { sys::AIBinder_lt(self.0.as_ptr(), other.0.as_ptr()) };
+ // Safety: SpIBinder always holds a valid `AIBinder` pointer, so this
+ // pointer is always safe to pass to `AIBinder_lt` (null is also safe to
+ // pass to this function, but we should never do that).
+ let greater_than = unsafe { sys::AIBinder_lt(other.0.as_ptr(), self.0.as_ptr()) };
if !less_than && !greater_than {
Ordering::Equal
} else if less_than {
@@ -221,10 +212,10 @@
impl Clone for SpIBinder {
fn clone(&self) -> Self {
+ // Safety: Cloning a strong reference must increment the reference
+ // count. We are guaranteed by the `SpIBinder` constructor
+ // invariants that `self.0` is always a valid `AIBinder` pointer.
unsafe {
- // Safety: Cloning a strong reference must increment the reference
- // count. We are guaranteed by the `SpIBinder` constructor
- // invariants that `self.0` is always a valid `AIBinder` pointer.
sys::AIBinder_incStrong(self.0.as_ptr());
}
Self(self.0)
@@ -235,9 +226,9 @@
// We hold a strong reference to the IBinder in SpIBinder and need to give up
// this reference on drop.
fn drop(&mut self) {
+ // Safety: SpIBinder always holds a valid `AIBinder` pointer, so we
+ // know this pointer is safe to pass to `AIBinder_decStrong` here.
unsafe {
- // Safety: SpIBinder always holds a valid `AIBinder` pointer, so we
- // know this pointer is safe to pass to `AIBinder_decStrong` here.
sys::AIBinder_decStrong(self.as_native_mut());
}
}
@@ -246,26 +237,24 @@
impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
fn prepare_transact(&self) -> Result<Parcel> {
let mut input = ptr::null_mut();
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. It is safe to cast from an
+ // immutable pointer to a mutable pointer here, because
+ // `AIBinder_prepareTransaction` only calls immutable `AIBinder`
+ // methods but the parameter is unfortunately not marked as const.
+ //
+ // After the call, input will be either a valid, owned `AParcel`
+ // pointer, or null.
let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. It is safe to cast from an
- // immutable pointer to a mutable pointer here, because
- // `AIBinder_prepareTransaction` only calls immutable `AIBinder`
- // methods but the parameter is unfortunately not marked as const.
- //
- // After the call, input will be either a valid, owned `AParcel`
- // pointer, or null.
sys::AIBinder_prepareTransaction(self.as_native() as *mut sys::AIBinder, &mut input)
};
status_result(status)?;
- unsafe {
- // Safety: At this point, `input` is either a valid, owned `AParcel`
- // pointer, or null. `OwnedParcel::from_raw` safely handles both cases,
- // taking ownership of the parcel.
- Parcel::from_raw(input).ok_or(StatusCode::UNEXPECTED_NULL)
- }
+ // Safety: At this point, `input` is either a valid, owned `AParcel`
+ // pointer, or null. `OwnedParcel::from_raw` safely handles both cases,
+ // taking ownership of the parcel.
+ unsafe { Parcel::from_raw(input).ok_or(StatusCode::UNEXPECTED_NULL) }
}
fn submit_transact(
@@ -275,23 +264,23 @@
flags: TransactionFlags,
) -> Result<Parcel> {
let mut reply = ptr::null_mut();
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. Although `IBinder::transact` is
+ // not a const method, it is still safe to cast our immutable
+ // pointer to mutable for the call. First, `IBinder::transact` is
+ // thread-safe, so concurrency is not an issue. The only way that
+ // `transact` can affect any visible, mutable state in the current
+ // process is by calling `onTransact` for a local service. However,
+ // in order for transactions to be thread-safe, this method must
+ // dynamically lock its data before modifying it. We enforce this
+ // property in Rust by requiring `Sync` for remotable objects and
+ // only providing `on_transact` with an immutable reference to
+ // `self`.
+ //
+ // This call takes ownership of the `data` parcel pointer, and
+ // passes ownership of the `reply` out parameter to its caller. It
+ // does not affect ownership of the `binder` parameter.
let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. Although `IBinder::transact` is
- // not a const method, it is still safe to cast our immutable
- // pointer to mutable for the call. First, `IBinder::transact` is
- // thread-safe, so concurrency is not an issue. The only way that
- // `transact` can affect any visible, mutable state in the current
- // process is by calling `onTransact` for a local service. However,
- // in order for transactions to be thread-safe, this method must
- // dynamically lock its data before modifying it. We enforce this
- // property in Rust by requiring `Sync` for remotable objects and
- // only providing `on_transact` with an immutable reference to
- // `self`.
- //
- // This call takes ownership of the `data` parcel pointer, and
- // passes ownership of the `reply` out parameter to its caller. It
- // does not affect ownership of the `binder` parameter.
sys::AIBinder_transact(
self.as_native() as *mut sys::AIBinder,
code,
@@ -302,45 +291,45 @@
};
status_result(status)?;
- unsafe {
- // Safety: `reply` is either a valid `AParcel` pointer or null
- // after the call to `AIBinder_transact` above, so we can
- // construct a `Parcel` out of it. `AIBinder_transact` passes
- // ownership of the `reply` parcel to Rust, so we need to
- // construct an owned variant.
- Parcel::from_raw(reply).ok_or(StatusCode::UNEXPECTED_NULL)
- }
+ // Safety: `reply` is either a valid `AParcel` pointer or null
+ // after the call to `AIBinder_transact` above, so we can
+ // construct a `Parcel` out of it. `AIBinder_transact` passes
+ // ownership of the `reply` parcel to Rust, so we need to
+ // construct an owned variant.
+ unsafe { Parcel::from_raw(reply).ok_or(StatusCode::UNEXPECTED_NULL) }
}
fn is_binder_alive(&self) -> bool {
- unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`.
- //
- // This call does not affect ownership of its pointer parameter.
- sys::AIBinder_isAlive(self.as_native())
- }
+ // Safety: `SpIBinder` guarantees that `self` always contains a valid
+ // pointer to an `AIBinder`.
+ //
+ // This call does not affect ownership of its pointer parameter.
+ unsafe { sys::AIBinder_isAlive(self.as_native()) }
}
#[cfg(not(android_vndk))]
fn set_requesting_sid(&mut self, enable: bool) {
+ // Safety: `SpIBinder` guarantees that `self` always contains a valid
+ // pointer to an `AIBinder`.
+ //
+ // This call does not affect ownership of its pointer parameter.
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();
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. `AsRawFd` guarantees that the
+ // file descriptor parameter is always be a valid open file. The
+ // `args` pointer parameter is a valid pointer to an array of C
+ // strings that will outlive the call since `args` lives for the
+ // whole function scope.
+ //
+ // This call does not affect ownership of its binder pointer
+ // parameter and does not take ownership of the file or args array
+ // parameters.
let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. `AsRawFd` guarantees that the
- // file descriptor parameter is always be a valid open file. The
- // `args` pointer parameter is a valid pointer to an array of C
- // strings that will outlive the call since `args` lives for the
- // whole function scope.
- //
- // This call does not affect ownership of its binder pointer
- // parameter and does not take ownership of the file or args array
- // parameters.
sys::AIBinder_dump(
self.as_native_mut(),
fp.as_raw_fd(),
@@ -353,22 +342,18 @@
fn get_extension(&mut self) -> Result<Option<SpIBinder>> {
let mut out = ptr::null_mut();
- let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. After this call, the `out`
- // parameter will be either null, or a valid pointer to an
- // `AIBinder`.
- //
- // This call passes ownership of the out pointer to its caller
- // (assuming it is set to a non-null value).
- sys::AIBinder_getExtension(self.as_native_mut(), &mut out)
- };
- let ibinder = unsafe {
- // Safety: The call above guarantees that `out` is either null or a
- // valid, owned pointer to an `AIBinder`, both of which are safe to
- // pass to `SpIBinder::from_raw`.
- SpIBinder::from_raw(out)
- };
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. After this call, the `out`
+ // parameter will be either null, or a valid pointer to an
+ // `AIBinder`.
+ //
+ // This call passes ownership of the out pointer to its caller
+ // (assuming it is set to a non-null value).
+ let status = unsafe { sys::AIBinder_getExtension(self.as_native_mut(), &mut out) };
+ // Safety: The call above guarantees that `out` is either null or a
+ // valid, owned pointer to an `AIBinder`, both of which are safe to
+ // pass to `SpIBinder::from_raw`.
+ let ibinder = unsafe { SpIBinder::from_raw(out) };
status_result(status)?;
Ok(ibinder)
@@ -377,17 +362,17 @@
impl<T: AsNative<sys::AIBinder>> IBinder for T {
fn link_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()> {
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. `recipient` can always be
+ // converted into a valid pointer to an
+ // `AIBinder_DeathRecipient`.
+ //
+ // The cookie is also the correct pointer, and by calling new_cookie,
+ // we have created a new ref-count to the cookie, which linkToDeath
+ // takes ownership of. Once the DeathRecipient is unlinked for any
+ // reason (including if this call fails), the onUnlinked callback
+ // will consume that ref-count.
status_result(unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. `recipient` can always be
- // converted into a valid pointer to an
- // `AIBinder_DeathRecipient`.
- //
- // The cookie is also the correct pointer, and by calling new_cookie,
- // we have created a new ref-count to the cookie, which linkToDeath
- // takes ownership of. Once the DeathRecipient is unlinked for any
- // reason (including if this call fails), the onUnlinked callback
- // will consume that ref-count.
sys::AIBinder_linkToDeath(
self.as_native_mut(),
recipient.as_native_mut(),
@@ -397,13 +382,13 @@
}
fn unlink_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()> {
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. `recipient` can always be
+ // converted into a valid pointer to an
+ // `AIBinder_DeathRecipient`. Any value is safe to pass as the
+ // cookie, although we depend on this value being set by
+ // `get_cookie` when the death recipient callback is called.
status_result(unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. `recipient` can always be
- // converted into a valid pointer to an
- // `AIBinder_DeathRecipient`. Any value is safe to pass as the
- // cookie, although we depend on this value being set by
- // `get_cookie` when the death recipient callback is called.
sys::AIBinder_unlinkToDeath(
self.as_native_mut(),
recipient.as_native_mut(),
@@ -413,13 +398,11 @@
}
fn ping_binder(&mut self) -> Result<()> {
- let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`.
- //
- // This call does not affect ownership of its pointer parameter.
- sys::AIBinder_ping(self.as_native_mut())
- };
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`.
+ //
+ // This call does not affect ownership of its pointer parameter.
+ let status = unsafe { sys::AIBinder_ping(self.as_native_mut()) };
status_result(status)
}
}
@@ -472,35 +455,31 @@
}
}
-/// # Safety
-///
-/// A `WpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe.
+/// Safety: A `WpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Send for WpIBinder {}
-/// # Safety
-///
-/// A `WpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe.
+/// Safety: A `WpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Sync for WpIBinder {}
impl WpIBinder {
/// Create a new weak reference from an object that can be converted into a
/// raw `AIBinder` pointer.
fn new<B: AsNative<sys::AIBinder>>(binder: &mut B) -> WpIBinder {
- let ptr = unsafe {
- // Safety: `SpIBinder` guarantees that `binder` always contains a
- // valid pointer to an `AIBinder`.
- sys::AIBinder_Weak_new(binder.as_native_mut())
- };
+ // Safety: `SpIBinder` guarantees that `binder` always contains a valid
+ // pointer to an `AIBinder`.
+ let ptr = unsafe { sys::AIBinder_Weak_new(binder.as_native_mut()) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null pointer from AIBinder_Weak_new"))
}
/// Promote this weak reference to a strong reference to the binder object.
pub fn promote(&self) -> Option<SpIBinder> {
+ // Safety: `WpIBinder` always contains a valid weak reference, so we can
+ // pass this pointer to `AIBinder_Weak_promote`. Returns either null or
+ // an AIBinder owned by the caller, both of which are valid to pass to
+ // `SpIBinder::from_raw`.
unsafe {
- // Safety: `WpIBinder` always contains a valid weak reference, so we
- // can pass this pointer to `AIBinder_Weak_promote`. Returns either
- // null or an AIBinder owned by the caller, both of which are valid
- // to pass to `SpIBinder::from_raw`.
let ptr = sys::AIBinder_Weak_promote(self.0.as_ptr());
SpIBinder::from_raw(ptr)
}
@@ -509,35 +488,27 @@
impl Clone for WpIBinder {
fn clone(&self) -> Self {
- let ptr = unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer,
- // so this pointer is always safe to pass to `AIBinder_Weak_clone`
- // (although null is also a safe value to pass to this API).
- //
- // We get ownership of the returned pointer, so can construct a new
- // WpIBinder object from it.
- sys::AIBinder_Weak_clone(self.0.as_ptr())
- };
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so
+ // this pointer is always safe to pass to `AIBinder_Weak_clone`
+ // (although null is also a safe value to pass to this API).
+ //
+ // We get ownership of the returned pointer, so can construct a new
+ // WpIBinder object from it.
+ let ptr = unsafe { sys::AIBinder_Weak_clone(self.0.as_ptr()) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null pointer from AIBinder_Weak_clone"))
}
}
impl Ord for WpIBinder {
fn cmp(&self, other: &Self) -> Ordering {
- let less_than = unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer,
- // so this pointer is always safe to pass to `AIBinder_Weak_lt`
- // (null is also safe to pass to this function, but we should never
- // do that).
- sys::AIBinder_Weak_lt(self.0.as_ptr(), other.0.as_ptr())
- };
- let greater_than = unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer,
- // so this pointer is always safe to pass to `AIBinder_Weak_lt`
- // (null is also safe to pass to this function, but we should never
- // do that).
- sys::AIBinder_Weak_lt(other.0.as_ptr(), self.0.as_ptr())
- };
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so
+ // this pointer is always safe to pass to `AIBinder_Weak_lt` (null is
+ // also safe to pass to this function, but we should never do that).
+ let less_than = unsafe { sys::AIBinder_Weak_lt(self.0.as_ptr(), other.0.as_ptr()) };
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so
+ // this pointer is always safe to pass to `AIBinder_Weak_lt` (null is
+ // also safe to pass to this function, but we should never do that).
+ let greater_than = unsafe { sys::AIBinder_Weak_lt(other.0.as_ptr(), self.0.as_ptr()) };
if !less_than && !greater_than {
Ordering::Equal
} else if less_than {
@@ -564,9 +535,9 @@
impl Drop for WpIBinder {
fn drop(&mut self) {
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so we
+ // know this pointer is safe to pass to `AIBinder_Weak_delete` here.
unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so we
- // know this pointer is safe to pass to `AIBinder_Weak_delete` here.
sys::AIBinder_Weak_delete(self.0.as_ptr());
}
}
@@ -574,7 +545,7 @@
/// Rust wrapper around DeathRecipient objects.
///
-/// The cookie in this struct represents an Arc<F> for the owned callback.
+/// The cookie in this struct represents an `Arc<F>` for the owned callback.
/// This struct owns a ref-count of it, and so does every binder that we
/// have been linked with.
///
@@ -592,17 +563,13 @@
cookie_decr_refcount: unsafe extern "C" fn(*mut c_void),
}
-/// # Safety
-///
-/// A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and a pointer
-/// to a `Fn` which is `Sync` and `Send` (the cookie field). As
+/// Safety: A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and
+/// a pointer to a `Fn` which is `Sync` and `Send` (the cookie field). As
/// `AIBinder_DeathRecipient` is threadsafe, this structure is too.
unsafe impl Send for DeathRecipient {}
-/// # Safety
-///
-/// A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and a pointer
-/// to a `Fn` which is `Sync` and `Send` (the cookie field). As
+/// Safety: A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and
+/// a pointer to a `Fn` which is `Sync` and `Send` (the cookie field). As
/// `AIBinder_DeathRecipient` is threadsafe, this structure is too.
unsafe impl Sync for DeathRecipient {}
@@ -614,19 +581,17 @@
F: Fn() + Send + Sync + 'static,
{
let callback: *const F = Arc::into_raw(Arc::new(callback));
- let recipient = unsafe {
- // Safety: The function pointer is a valid death recipient callback.
- //
- // This call returns an owned `AIBinder_DeathRecipient` pointer
- // which must be destroyed via `AIBinder_DeathRecipient_delete` when
- // no longer needed.
- sys::AIBinder_DeathRecipient_new(Some(Self::binder_died::<F>))
- };
+ // Safety: The function pointer is a valid death recipient callback.
+ //
+ // This call returns an owned `AIBinder_DeathRecipient` pointer which
+ // must be destroyed via `AIBinder_DeathRecipient_delete` when no longer
+ // needed.
+ let recipient = unsafe { sys::AIBinder_DeathRecipient_new(Some(Self::binder_died::<F>)) };
+ // Safety: The function pointer is a valid onUnlinked callback.
+ //
+ // All uses of linkToDeath in this file correctly increment the
+ // ref-count that this onUnlinked callback will decrement.
unsafe {
- // Safety: The function pointer is a valid onUnlinked callback.
- //
- // All uses of linkToDeath in this file correctly increment the
- // ref-count that this onUnlinked callback will decrement.
sys::AIBinder_DeathRecipient_setOnUnlinked(
recipient,
Some(Self::cookie_decr_refcount::<F>),
@@ -648,7 +613,12 @@
///
/// The caller must handle the returned ref-count correctly.
unsafe fn new_cookie(&self) -> *mut c_void {
- (self.vtable.cookie_incr_refcount)(self.cookie);
+ // Safety: `cookie_incr_refcount` points to
+ // `Self::cookie_incr_refcount`, and `self.cookie` is the cookie for an
+ // Arc<F>.
+ unsafe {
+ (self.vtable.cookie_incr_refcount)(self.cookie);
+ }
// Return a raw pointer with ownership of a ref-count
self.cookie
@@ -667,13 +637,14 @@
///
/// # Safety
///
- /// The `cookie` parameter must be the cookie for an Arc<F> and
+ /// The `cookie` parameter must be the cookie for an `Arc<F>` and
/// the caller must hold a ref-count to it.
unsafe extern "C" fn binder_died<F>(cookie: *mut c_void)
where
F: Fn() + Send + Sync + 'static,
{
- let callback = (cookie as *const F).as_ref().unwrap();
+ // Safety: The caller promises that `cookie` is for an Arc<F>.
+ let callback = unsafe { (cookie as *const F).as_ref().unwrap() };
callback();
}
@@ -682,34 +653,34 @@
///
/// # Safety
///
- /// The `cookie` parameter must be the cookie for an Arc<F> and
+ /// The `cookie` parameter must be the cookie for an `Arc<F>` and
/// the owner must give up a ref-count to it.
unsafe extern "C" fn cookie_decr_refcount<F>(cookie: *mut c_void)
where
F: Fn() + Send + Sync + 'static,
{
- drop(Arc::from_raw(cookie as *const F));
+ // Safety: The caller promises that `cookie` is for an Arc<F>.
+ drop(unsafe { Arc::from_raw(cookie as *const F) });
}
/// Callback that increments the ref-count.
///
/// # Safety
///
- /// The `cookie` parameter must be the cookie for an Arc<F> and
+ /// The `cookie` parameter must be the cookie for an `Arc<F>` and
/// the owner must handle the created ref-count properly.
unsafe extern "C" fn cookie_incr_refcount<F>(cookie: *mut c_void)
where
F: Fn() + Send + Sync + 'static,
{
- let arc = mem::ManuallyDrop::new(Arc::from_raw(cookie as *const F));
+ // Safety: The caller promises that `cookie` is for an Arc<F>.
+ let arc = mem::ManuallyDrop::new(unsafe { Arc::from_raw(cookie as *const F) });
mem::forget(Arc::clone(&arc));
}
}
-/// # Safety
-///
-/// A `DeathRecipient` is always constructed with a valid raw pointer to an
-/// `AIBinder_DeathRecipient`, so it is always type-safe to extract this
+/// Safety: A `DeathRecipient` is always constructed with a valid raw pointer to
+/// an `AIBinder_DeathRecipient`, so it is always type-safe to extract this
/// pointer.
unsafe impl AsNative<sys::AIBinder_DeathRecipient> for DeathRecipient {
fn as_native(&self) -> *const sys::AIBinder_DeathRecipient {
@@ -723,18 +694,19 @@
impl Drop for DeathRecipient {
fn drop(&mut self) {
+ // Safety: `self.recipient` is always a valid, owned
+ // `AIBinder_DeathRecipient` pointer returned by
+ // `AIBinder_DeathRecipient_new` when `self` was created. This delete
+ // method can only be called once when `self` is dropped.
unsafe {
- // Safety: `self.recipient` is always a valid, owned
- // `AIBinder_DeathRecipient` pointer returned by
- // `AIBinder_DeathRecipient_new` when `self` was created. This
- // delete method can only be called once when `self` is dropped.
sys::AIBinder_DeathRecipient_delete(self.recipient);
+ }
- // Safety: We own a ref-count to the cookie, and so does every
- // linked binder. This call gives up our ref-count. The linked
- // binders should already have given up their ref-count, or should
- // do so shortly.
- (self.vtable.cookie_decr_refcount)(self.cookie)
+ // Safety: We own a ref-count to the cookie, and so does every linked
+ // binder. This call gives up our ref-count. The linked binders should
+ // already have given up their ref-count, or should do so shortly.
+ unsafe {
+ (self.vtable.cookie_decr_refcount)(self.cookie);
}
}
}
@@ -754,11 +726,9 @@
fn from_binder(binder: SpIBinder) -> Result<Self>;
}
-/// # Safety
-///
-/// This is a convenience method that wraps `AsNative` for `SpIBinder` to allow
-/// invocation of `IBinder` methods directly from `Interface` objects. It shares
-/// the same safety as the implementation for `SpIBinder`.
+/// Safety: This is a convenience method that wraps `AsNative` for `SpIBinder`
+/// to allow invocation of `IBinder` methods directly from `Interface` objects.
+/// It shares the same safety as the implementation for `SpIBinder`.
unsafe impl<T: Proxy> AsNative<sys::AIBinder> for T {
fn as_native(&self) -> *const sys::AIBinder {
self.as_binder().as_native()
@@ -773,24 +743,20 @@
/// exist.
pub fn get_service(name: &str) -> Option<SpIBinder> {
let name = CString::new(name).ok()?;
- unsafe {
- // Safety: `AServiceManager_getService` returns either a null pointer or
- // a valid pointer to an owned `AIBinder`. Either of these values is
- // safe to pass to `SpIBinder::from_raw`.
- SpIBinder::from_raw(sys::AServiceManager_getService(name.as_ptr()))
- }
+ // Safety: `AServiceManager_getService` returns either a null pointer or a
+ // valid pointer to an owned `AIBinder`. Either of these values is safe to
+ // pass to `SpIBinder::from_raw`.
+ unsafe { SpIBinder::from_raw(sys::AServiceManager_getService(name.as_ptr())) }
}
/// Retrieve an existing service, or start it if it is configured as a dynamic
/// service and isn't yet started.
pub fn wait_for_service(name: &str) -> Option<SpIBinder> {
let name = CString::new(name).ok()?;
- unsafe {
- // Safety: `AServiceManager_waitforService` returns either a null
- // pointer or a valid pointer to an owned `AIBinder`. Either of these
- // values is safe to pass to `SpIBinder::from_raw`.
- SpIBinder::from_raw(sys::AServiceManager_waitForService(name.as_ptr()))
- }
+ // Safety: `AServiceManager_waitforService` returns either a null pointer or
+ // a valid pointer to an owned `AIBinder`. Either of these values is safe to
+ // pass to `SpIBinder::from_raw`.
+ unsafe { SpIBinder::from_raw(sys::AServiceManager_waitForService(name.as_ptr())) }
}
/// Retrieve an existing service for a particular interface, blocking for a few
@@ -809,12 +775,10 @@
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()))
- }
+ // 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.
+ unsafe { Ok(sys::AServiceManager_isDeclared(interface.as_ptr())) }
}
/// Retrieve all declared instances for a particular interface
@@ -827,11 +791,13 @@
// 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() {
+ if let Some(instances) = unsafe { 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());
+ unsafe {
+ instances.push(CStr::from_ptr(instance).to_owned());
+ }
} else {
eprintln!("Opaque pointer was null in get_declared_instances callback!");
}
@@ -839,10 +805,10 @@
let interface = CString::new(interface).or(Err(StatusCode::UNEXPECTED_NULL))?;
let mut instances: Vec<CString> = vec![];
+ // 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.
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,
@@ -860,10 +826,8 @@
})
}
-/// # Safety
-///
-/// `SpIBinder` guarantees that `binder` always contains a valid pointer to an
-/// `AIBinder`, so we can trivially extract this pointer here.
+/// Safety: `SpIBinder` guarantees that `binder` always contains a valid pointer
+/// to an `AIBinder`, so we can trivially extract this pointer here.
unsafe impl AsNative<sys::AIBinder> for SpIBinder {
fn as_native(&self) -> *const sys::AIBinder {
self.0.as_ptr()
diff --git a/libs/binder/rust/src/state.rs b/libs/binder/rust/src/state.rs
index 4886c5f..a3a2562 100644
--- a/libs/binder/rust/src/state.rs
+++ b/libs/binder/rust/src/state.rs
@@ -35,8 +35,8 @@
/// not work: the callbacks will be queued but never called as there is no
/// thread to call them on.
pub fn start_thread_pool() {
+ // Safety: Safe FFI
unsafe {
- // Safety: Safe FFI
sys::ABinderProcess_startThreadPool();
}
}
@@ -48,8 +48,8 @@
/// called, this is 15. If it is called additional times, the thread pool
/// size can only be increased.
pub fn set_thread_pool_max_thread_count(num_threads: u32) {
+ // Safety: Safe FFI
unsafe {
- // Safety: Safe FFI
sys::ABinderProcess_setThreadPoolMaxThreadCount(num_threads);
}
}
@@ -62,8 +62,8 @@
/// [`set_thread_pool_max_thread_count`](Self::set_thread_pool_max_thread_count)
/// and [`start_thread_pool`](Self::start_thread_pool).
pub fn join_thread_pool() {
+ // Safety: Safe FFI
unsafe {
- // Safety: Safe FFI
sys::ABinderProcess_joinThreadPool();
}
}
@@ -86,10 +86,8 @@
/// \return calling uid or the current process's UID if this thread isn't
/// processing a transaction.
pub fn get_calling_uid() -> uid_t {
- unsafe {
- // Safety: Safe FFI
- sys::AIBinder_getCallingUid()
- }
+ // Safety: Safe FFI
+ unsafe { sys::AIBinder_getCallingUid() }
}
/// This returns the calling PID assuming that this thread is called from a
@@ -111,10 +109,8 @@
/// If the transaction being processed is a oneway transaction, then this
/// method will return 0.
pub fn get_calling_pid() -> pid_t {
- unsafe {
- // Safety: Safe FFI
- sys::AIBinder_getCallingPid()
- }
+ // Safety: Safe FFI
+ unsafe { sys::AIBinder_getCallingPid() }
}
/// Determine whether the current thread is currently executing an incoming transaction.
@@ -122,10 +118,8 @@
/// \return true if the current thread is currently executing an incoming transaction, and false
/// otherwise.
pub fn is_handling_transaction() -> bool {
- unsafe {
- // Safety: Safe FFI
- sys::AIBinder_isHandlingTransaction()
- }
+ // Safety: Safe FFI
+ unsafe { sys::AIBinder_isHandlingTransaction() }
}
/// This function makes the client's security context available to the
diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs
index ca2cedc..c049b80 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -545,6 +545,11 @@
}
fn get_expected_selinux_context() -> &'static str {
+ // SAFETY: The pointer we pass to `getcon` is valid because it comes from a reference, and
+ // `getcon` doesn't retain it after it returns. If `getcon` succeeds then `out_ptr` will
+ // point to a valid C string, otherwise it will remain null. We check for null, so the
+ // pointer we pass to `CStr::from_ptr` must be a valid pointer to a C string. There is a
+ // memory leak as we don't call `freecon`, but that's fine because this is just a test.
unsafe {
let mut out_ptr = ptr::null_mut();
assert_eq!(selinux_sys::getcon(&mut out_ptr), 0);
diff --git a/libs/binder/rust/tests/ndk_rust_interop.rs b/libs/binder/rust/tests/ndk_rust_interop.rs
index 415ede1..37f182e 100644
--- a/libs/binder/rust/tests/ndk_rust_interop.rs
+++ b/libs/binder/rust/tests/ndk_rust_interop.rs
@@ -28,10 +28,11 @@
///
/// # Safety
///
-/// service_name must be a valid, non-null C-style string (null-terminated).
+/// service_name must be a valid, non-null C-style string (nul-terminated).
#[no_mangle]
pub unsafe extern "C" fn rust_call_ndk(service_name: *const c_char) -> c_int {
- let service_name = CStr::from_ptr(service_name).to_str().unwrap();
+ // SAFETY: Our caller promises that service_name is a valid C string.
+ let service_name = unsafe { CStr::from_ptr(service_name) }.to_str().unwrap();
// The Rust class descriptor pointer will not match the NDK one, but the
// descriptor strings match so this needs to still associate.
@@ -85,10 +86,11 @@
///
/// # Safety
///
-/// service_name must be a valid, non-null C-style string (null-terminated).
+/// service_name must be a valid, non-null C-style string (nul-terminated).
#[no_mangle]
pub unsafe extern "C" fn rust_start_service(service_name: *const c_char) -> c_int {
- let service_name = CStr::from_ptr(service_name).to_str().unwrap();
+ // SAFETY: Our caller promises that service_name is a valid C string.
+ let service_name = unsafe { CStr::from_ptr(service_name) }.to_str().unwrap();
let service = BnBinderRustNdkInteropTest::new_binder(Service, BinderFeatures::default());
match binder::add_service(service_name, service.as_binder()) {
Ok(_) => StatusCode::OK as c_int,
diff --git a/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs b/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs
index 29bf92c..ce0f742 100644
--- a/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs
+++ b/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs
@@ -105,9 +105,9 @@
for operation in read_operations {
match operation {
ReadOperation::SetDataPosition { pos } => {
+ // Safety: Safe if pos is less than current size of the parcel.
+ // It relies on C++ code for bound checks
unsafe {
- // Safety: Safe if pos is less than current size of the parcel.
- // It relies on C++ code for bound checks
match parcel.set_data_position(pos) {
Ok(result) => result,
Err(e) => println!("error occurred while setting data position: {:?}", e),
diff --git a/libs/binder/rust/tests/serialization.rs b/libs/binder/rust/tests/serialization.rs
index 6220db4..2b6c282 100644
--- a/libs/binder/rust/tests/serialization.rs
+++ b/libs/binder/rust/tests/serialization.rs
@@ -26,7 +26,7 @@
use binder::binder_impl::{Binder, BorrowedParcel, TransactionCode};
use std::ffi::{c_void, CStr, CString};
-use std::sync::Once;
+use std::sync::OnceLock;
#[allow(
non_camel_case_types,
@@ -70,20 +70,18 @@
};
}
-static SERVICE_ONCE: Once = Once::new();
-static mut SERVICE: Option<SpIBinder> = None;
+static SERVICE: OnceLock<SpIBinder> = OnceLock::new();
/// Start binder service and return a raw AIBinder pointer to it.
///
/// Safe to call multiple times, only creates the service once.
#[no_mangle]
pub extern "C" fn rust_service() -> *mut c_void {
- unsafe {
- SERVICE_ONCE.call_once(|| {
- SERVICE = Some(BnReadParcelTest::new_binder((), BinderFeatures::default()).as_binder());
- });
- SERVICE.as_ref().unwrap().as_raw().cast()
- }
+ let service = SERVICE
+ .get_or_init(|| BnReadParcelTest::new_binder((), BinderFeatures::default()).as_binder());
+ // SAFETY: The SpIBinder will remain alive as long as the program is running because it is in
+ // the static SERVICE, so the pointer is valid forever.
+ unsafe { service.as_raw().cast() }
}
/// Empty interface just to use the declare_binder_interface macro
@@ -113,11 +111,13 @@
bindings::Transaction_TEST_BOOL => {
assert!(parcel.read::<bool>()?);
assert!(!parcel.read::<bool>()?);
+ // SAFETY: Just reading an extern constant.
assert_eq!(parcel.read::<Vec<bool>>()?, unsafe { bindings::TESTDATA_BOOL });
assert_eq!(parcel.read::<Option<Vec<bool>>>()?, None);
reply.write(&true)?;
reply.write(&false)?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_BOOL }[..])?;
reply.write(&(None as Option<Vec<bool>>))?;
}
@@ -125,14 +125,18 @@
assert_eq!(parcel.read::<i8>()?, 0);
assert_eq!(parcel.read::<i8>()?, 1);
assert_eq!(parcel.read::<i8>()?, i8::max_value());
+ // SAFETY: Just reading an extern constant.
assert_eq!(parcel.read::<Vec<i8>>()?, unsafe { bindings::TESTDATA_I8 });
+ // SAFETY: Just reading an extern constant.
assert_eq!(parcel.read::<Vec<u8>>()?, unsafe { bindings::TESTDATA_U8 });
assert_eq!(parcel.read::<Option<Vec<i8>>>()?, None);
reply.write(&0i8)?;
reply.write(&1i8)?;
reply.write(&i8::max_value())?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_I8 }[..])?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_U8 }[..])?;
reply.write(&(None as Option<Vec<i8>>))?;
}
@@ -140,12 +144,14 @@
assert_eq!(parcel.read::<u16>()?, 0);
assert_eq!(parcel.read::<u16>()?, 1);
assert_eq!(parcel.read::<u16>()?, u16::max_value());
+ // SAFETY: Just reading an extern constant.
assert_eq!(parcel.read::<Vec<u16>>()?, unsafe { bindings::TESTDATA_CHARS });
assert_eq!(parcel.read::<Option<Vec<u16>>>()?, None);
reply.write(&0u16)?;
reply.write(&1u16)?;
reply.write(&u16::max_value())?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_CHARS }[..])?;
reply.write(&(None as Option<Vec<u16>>))?;
}
@@ -153,12 +159,14 @@
assert_eq!(parcel.read::<i32>()?, 0);
assert_eq!(parcel.read::<i32>()?, 1);
assert_eq!(parcel.read::<i32>()?, i32::max_value());
+ // SAFETY: Just reading an extern constant.
assert_eq!(parcel.read::<Vec<i32>>()?, unsafe { bindings::TESTDATA_I32 });
assert_eq!(parcel.read::<Option<Vec<i32>>>()?, None);
reply.write(&0i32)?;
reply.write(&1i32)?;
reply.write(&i32::max_value())?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_I32 }[..])?;
reply.write(&(None as Option<Vec<i32>>))?;
}
@@ -166,12 +174,14 @@
assert_eq!(parcel.read::<i64>()?, 0);
assert_eq!(parcel.read::<i64>()?, 1);
assert_eq!(parcel.read::<i64>()?, i64::max_value());
+ // SAFETY: Just reading an extern constant.
assert_eq!(parcel.read::<Vec<i64>>()?, unsafe { bindings::TESTDATA_I64 });
assert_eq!(parcel.read::<Option<Vec<i64>>>()?, None);
reply.write(&0i64)?;
reply.write(&1i64)?;
reply.write(&i64::max_value())?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_I64 }[..])?;
reply.write(&(None as Option<Vec<i64>>))?;
}
@@ -179,12 +189,14 @@
assert_eq!(parcel.read::<u64>()?, 0);
assert_eq!(parcel.read::<u64>()?, 1);
assert_eq!(parcel.read::<u64>()?, u64::max_value());
+ // SAFETY: Just reading an extern constant.
assert_eq!(parcel.read::<Vec<u64>>()?, unsafe { bindings::TESTDATA_U64 });
assert_eq!(parcel.read::<Option<Vec<u64>>>()?, None);
reply.write(&0u64)?;
reply.write(&1u64)?;
reply.write(&u64::max_value())?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_U64 }[..])?;
reply.write(&(None as Option<Vec<u64>>))?;
}
@@ -192,10 +204,12 @@
assert_eq!(parcel.read::<f32>()?, 0f32);
let floats = parcel.read::<Vec<f32>>()?;
assert!(floats[0].is_nan());
+ // SAFETY: Just reading an extern constant.
assert_eq!(floats[1..], unsafe { bindings::TESTDATA_FLOAT }[1..]);
assert_eq!(parcel.read::<Option<Vec<f32>>>()?, None);
reply.write(&0f32)?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_FLOAT }[..])?;
reply.write(&(None as Option<Vec<f32>>))?;
}
@@ -203,10 +217,12 @@
assert_eq!(parcel.read::<f64>()?, 0f64);
let doubles = parcel.read::<Vec<f64>>()?;
assert!(doubles[0].is_nan());
+ // SAFETY: Just reading an extern constant.
assert_eq!(doubles[1..], unsafe { bindings::TESTDATA_DOUBLE }[1..]);
assert_eq!(parcel.read::<Option<Vec<f64>>>()?, None);
reply.write(&0f64)?;
+ // SAFETY: Just reading an extern constant.
reply.write(&unsafe { bindings::TESTDATA_DOUBLE }[..])?;
reply.write(&(None as Option<Vec<f64>>))?;
}
@@ -216,14 +232,17 @@
let s: Option<String> = parcel.read()?;
assert_eq!(s, None);
let s: Option<Vec<Option<String>>> = parcel.read()?;
+ // SAFETY: Just reading an extern constant.
for (s, expected) in s.unwrap().iter().zip(unsafe { bindings::TESTDATA_STRS }.iter()) {
let expected =
+ // SAFETY: Just reading an extern constant.
unsafe { expected.as_ref().and_then(|e| CStr::from_ptr(e).to_str().ok()) };
assert_eq!(s.as_deref(), expected);
}
let s: Option<Vec<Option<String>>> = parcel.read()?;
assert_eq!(s, None);
+ // SAFETY: Just reading an extern constant.
let strings: Vec<Option<String>> = unsafe {
bindings::TESTDATA_STRS
.iter()
@@ -258,8 +277,7 @@
assert!(ibinders[1].is_none());
assert!(parcel.read::<Option<Vec<Option<SpIBinder>>>>()?.is_none());
- let service =
- unsafe { SERVICE.as_ref().expect("Global binder service not initialized").clone() };
+ let service = SERVICE.get().expect("Global binder service not initialized").clone();
reply.write(&service)?;
reply.write(&(None as Option<&SpIBinder>))?;
reply.write(&[Some(&service), None][..])?;
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index 41856f9..cd3e7c0 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -77,6 +77,8 @@
static_libs: [
"binderRecordReplayTestIface-cpp",
"binderReadParcelIface-cpp",
+ "libbinder_random_parcel_seeds",
+ "libbinder_random_parcel",
],
test_suites: ["general-tests"],
require_root: true,
diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp
index bc40864..6712c9c 100644
--- a/libs/binder/tests/binderAllocationLimits.cpp
+++ b/libs/binder/tests/binderAllocationLimits.cpp
@@ -216,16 +216,16 @@
auto server = RpcServer::make();
server->setRootObject(sp<BBinder>::make());
- CHECK_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
+ ASSERT_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
std::thread([server]() { server->join(); }).detach();
- status_t status;
auto session = RpcSession::make();
- status = session->setupUnixDomainClient(addr.c_str());
- CHECK_EQ(status, OK) << "Could not connect: " << addr << ": " << statusToString(status).c_str();
+ status_t status = session->setupUnixDomainClient(addr.c_str());
+ ASSERT_EQ(status, OK) << "Could not connect: " << addr << ": " << statusToString(status).c_str();
auto remoteBinder = session->getRootObject();
+ ASSERT_NE(remoteBinder, nullptr);
size_t mallocs = 0, totalBytes = 0;
{
@@ -233,7 +233,7 @@
mallocs++;
totalBytes += bytes;
});
- CHECK_EQ(OK, remoteBinder->pingBinder());
+ ASSERT_EQ(OK, remoteBinder->pingBinder());
}
EXPECT_EQ(mallocs, 1);
EXPECT_EQ(totalBytes, 40);
diff --git a/libs/binder/tests/binderRecordReplayTest.cpp b/libs/binder/tests/binderRecordReplayTest.cpp
index 17d5c8a..6773c95 100644
--- a/libs/binder/tests/binderRecordReplayTest.cpp
+++ b/libs/binder/tests/binderRecordReplayTest.cpp
@@ -15,6 +15,7 @@
*/
#include <BnBinderRecordReplayTest.h>
+#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/unique_fd.h>
#include <binder/Binder.h>
@@ -23,6 +24,11 @@
#include <binder/IPCThreadState.h>
#include <binder/IServiceManager.h>
#include <binder/RecordedTransaction.h>
+
+#include <fuzzbinder/libbinder_driver.h>
+#include <fuzzer/FuzzedDataProvider.h>
+#include <fuzzseeds/random_parcel_seeds.h>
+
#include <gtest/gtest.h>
#include <sys/prctl.h>
@@ -30,6 +36,7 @@
#include "parcelables/SingleDataParcelable.h"
using namespace android;
+using android::generateSeedsFromRecording;
using android::binder::Status;
using android::binder::debug::RecordedTransaction;
using parcelables::SingleDataParcelable;
@@ -84,6 +91,44 @@
GENERATE_GETTER_SETTER(SingleDataParcelableArray, std::vector<SingleDataParcelable>);
};
+std::vector<uint8_t> retrieveData(base::borrowed_fd fd) {
+ struct stat fdStat;
+ EXPECT_TRUE(fstat(fd.get(), &fdStat) != -1);
+ EXPECT_TRUE(fdStat.st_size != 0);
+
+ std::vector<uint8_t> buffer(fdStat.st_size);
+ auto readResult = android::base::ReadFully(fd, buffer.data(), fdStat.st_size);
+ EXPECT_TRUE(readResult != 0);
+ return std::move(buffer);
+}
+
+void replayFuzzService(const sp<BpBinder>& binder, const RecordedTransaction& transaction) {
+ base::unique_fd seedFd(open("/data/local/tmp/replayFuzzService",
+ O_RDWR | O_CREAT | O_CLOEXEC | O_TRUNC, 0666));
+ ASSERT_TRUE(seedFd.ok());
+
+ // generate corpus from this transaction.
+ generateSeedsFromRecording(seedFd, transaction);
+
+ // Read the data which has been written to seed corpus
+ ASSERT_EQ(0, lseek(seedFd.get(), 0, SEEK_SET));
+ std::vector<uint8_t> seedData = retrieveData(seedFd);
+
+ // use fuzzService to replay the corpus
+ FuzzedDataProvider provider(seedData.data(), seedData.size());
+ fuzzService(binder, std::move(provider));
+}
+
+void replayBinder(const sp<BpBinder>& binder, const RecordedTransaction& transaction) {
+ // TODO: move logic to replay RecordedTransaction into RecordedTransaction
+ Parcel data;
+ data.setData(transaction.getDataParcel().data(), transaction.getDataParcel().dataSize());
+ auto result = binder->transact(transaction.getCode(), data, nullptr, transaction.getFlags());
+
+ // make sure recording does the thing we expect it to do
+ EXPECT_EQ(OK, result);
+}
+
class BinderRecordReplayTest : public ::testing::Test {
public:
void SetUp() override {
@@ -98,48 +143,46 @@
template <typename T, typename U>
void recordReplay(Status (IBinderRecordReplayTest::*set)(T), U recordedValue,
Status (IBinderRecordReplayTest::*get)(U*), U changedValue) {
- base::unique_fd fd(open("/data/local/tmp/binderRecordReplayTest.rec",
- O_RDWR | O_CREAT | O_CLOEXEC, 0666));
- ASSERT_TRUE(fd.ok());
+ auto replayFunctions = {&replayBinder, &replayFuzzService};
+ for (auto replayFunc : replayFunctions) {
+ base::unique_fd fd(open("/data/local/tmp/binderRecordReplayTest.rec",
+ O_RDWR | O_CREAT | O_CLOEXEC, 0666));
+ ASSERT_TRUE(fd.ok());
- // record a transaction
- mBpBinder->startRecordingBinder(fd);
- auto status = (*mInterface.*set)(recordedValue);
- EXPECT_TRUE(status.isOk());
- mBpBinder->stopRecordingBinder();
+ // record a transaction
+ mBpBinder->startRecordingBinder(fd);
+ auto status = (*mInterface.*set)(recordedValue);
+ EXPECT_TRUE(status.isOk());
+ mBpBinder->stopRecordingBinder();
- // test transaction does the thing we expect it to do
- U output;
- status = (*mInterface.*get)(&output);
- EXPECT_TRUE(status.isOk());
- EXPECT_EQ(output, recordedValue);
+ // test transaction does the thing we expect it to do
+ U output;
+ status = (*mInterface.*get)(&output);
+ EXPECT_TRUE(status.isOk());
+ EXPECT_EQ(output, recordedValue);
- // write over the existing state
- status = (*mInterface.*set)(changedValue);
- EXPECT_TRUE(status.isOk());
+ // write over the existing state
+ status = (*mInterface.*set)(changedValue);
+ EXPECT_TRUE(status.isOk());
- status = (*mInterface.*get)(&output);
- EXPECT_TRUE(status.isOk());
+ status = (*mInterface.*get)(&output);
+ EXPECT_TRUE(status.isOk());
- EXPECT_EQ(output, changedValue);
+ EXPECT_EQ(output, changedValue);
- // replay transaction
- ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET));
- std::optional<RecordedTransaction> transaction = RecordedTransaction::fromFile(fd);
- ASSERT_NE(transaction, std::nullopt);
+ // replay transaction
+ ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET));
+ std::optional<RecordedTransaction> transaction = RecordedTransaction::fromFile(fd);
+ ASSERT_NE(transaction, std::nullopt);
- // TODO: move logic to replay RecordedTransaction into RecordedTransaction
- Parcel data;
- data.setData(transaction->getDataParcel().data(), transaction->getDataParcel().dataSize());
- auto result =
- mBpBinder->transact(transaction->getCode(), data, nullptr, transaction->getFlags());
+ const RecordedTransaction& recordedTransaction = *transaction;
+ // call replay function with recorded transaction
+ (*replayFunc)(mBpBinder, recordedTransaction);
- // make sure recording does the thing we expect it to do
- EXPECT_EQ(OK, result);
-
- status = (*mInterface.*get)(&output);
- EXPECT_TRUE(status.isOk());
- EXPECT_EQ(output, recordedValue);
+ status = (*mInterface.*get)(&output);
+ EXPECT_TRUE(status.isOk());
+ EXPECT_EQ(output, recordedValue);
+ }
}
private:
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index d352ce5..4c3c68e 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -249,12 +249,12 @@
CHECK_EQ(options.numIncomingConnectionsBySession.size(), options.numSessions);
}
- SocketType socketType = std::get<0>(GetParam());
- RpcSecurity rpcSecurity = std::get<1>(GetParam());
- uint32_t clientVersion = std::get<2>(GetParam());
- uint32_t serverVersion = std::get<3>(GetParam());
- bool singleThreaded = std::get<4>(GetParam());
- bool noKernel = std::get<5>(GetParam());
+ SocketType socketType = GetParam().type;
+ RpcSecurity rpcSecurity = GetParam().security;
+ uint32_t clientVersion = GetParam().clientVersion;
+ uint32_t serverVersion = GetParam().serverVersion;
+ bool singleThreaded = GetParam().singleThreaded;
+ bool noKernel = GetParam().noKernel;
std::string path = android::base::GetExecutableDirectory();
auto servicePath = android::base::StringPrintf("%s/binder_rpc_test_service%s%s", path.c_str(),
@@ -674,7 +674,7 @@
// session 0 - will check for leaks in destrutor of proc
// session 1 - we want to make sure it gets deleted when we drop all references to it
auto proc = createRpcTestSocketServerProcess(
- {.numThreads = 1, .numIncomingConnectionsBySession = {0, 1}, .numSessions = 2});
+ {.numThreads = 1, .numSessions = 2, .numIncomingConnectionsBySession = {0, 1}});
wp<RpcSession> session = proc.proc->sessions.at(1).session;
@@ -1121,12 +1121,27 @@
}
#ifdef BINDER_RPC_TO_TRUSTY_TEST
-INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc,
- ::testing::Combine(::testing::Values(SocketType::TIPC),
- ::testing::Values(RpcSecurity::RAW),
- ::testing::ValuesIn(testVersions()),
- ::testing::ValuesIn(testVersions()),
- ::testing::Values(true), ::testing::Values(true)),
+
+static std::vector<BinderRpc::ParamType> getTrustyBinderRpcParams() {
+ std::vector<BinderRpc::ParamType> ret;
+
+ for (const auto& clientVersion : testVersions()) {
+ for (const auto& serverVersion : testVersions()) {
+ ret.push_back(BinderRpc::ParamType{
+ .type = SocketType::TIPC,
+ .security = RpcSecurity::RAW,
+ .clientVersion = clientVersion,
+ .serverVersion = serverVersion,
+ .singleThreaded = true,
+ .noKernel = true,
+ });
+ }
+ }
+
+ return ret;
+}
+
+INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()),
BinderRpc::PrintParamInfo);
#else // BINDER_RPC_TO_TRUSTY_TEST
bool testSupportVsockLoopback() {
@@ -1246,13 +1261,47 @@
return ret;
}
-INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc,
- ::testing::Combine(::testing::ValuesIn(testSocketTypes()),
- ::testing::ValuesIn(RpcSecurityValues()),
- ::testing::ValuesIn(testVersions()),
- ::testing::ValuesIn(testVersions()),
- ::testing::Values(false, true),
- ::testing::Values(false, true)),
+static std::vector<BinderRpc::ParamType> getBinderRpcParams() {
+ std::vector<BinderRpc::ParamType> ret;
+
+ constexpr bool full = false;
+
+ for (const auto& type : testSocketTypes()) {
+ if (full || type == SocketType::UNIX) {
+ for (const auto& security : RpcSecurityValues()) {
+ for (const auto& clientVersion : testVersions()) {
+ for (const auto& serverVersion : testVersions()) {
+ for (bool singleThreaded : {false, true}) {
+ for (bool noKernel : {false, true}) {
+ ret.push_back(BinderRpc::ParamType{
+ .type = type,
+ .security = security,
+ .clientVersion = clientVersion,
+ .serverVersion = serverVersion,
+ .singleThreaded = singleThreaded,
+ .noKernel = noKernel,
+ });
+ }
+ }
+ }
+ }
+ }
+ } else {
+ ret.push_back(BinderRpc::ParamType{
+ .type = type,
+ .security = RpcSecurity::RAW,
+ .clientVersion = RPC_WIRE_PROTOCOL_VERSION,
+ .serverVersion = RPC_WIRE_PROTOCOL_VERSION,
+ .singleThreaded = false,
+ .noKernel = false,
+ });
+ }
+ }
+
+ return ret;
+}
+
+INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, ::testing::ValuesIn(getBinderRpcParams()),
BinderRpc::PrintParamInfo);
class BinderRpcServerRootObject
diff --git a/libs/binder/tests/binderRpcTestFixture.h b/libs/binder/tests/binderRpcTestFixture.h
index 0b8920b..2c9646b 100644
--- a/libs/binder/tests/binderRpcTestFixture.h
+++ b/libs/binder/tests/binderRpcTestFixture.h
@@ -106,15 +106,23 @@
}
};
-class BinderRpc : public ::testing::TestWithParam<
- std::tuple<SocketType, RpcSecurity, uint32_t, uint32_t, bool, bool>> {
+struct BinderRpcParam {
+ SocketType type;
+ RpcSecurity security;
+ uint32_t clientVersion;
+ uint32_t serverVersion;
+ bool singleThreaded;
+ bool noKernel;
+};
+class BinderRpc : public ::testing::TestWithParam<BinderRpcParam> {
public:
- SocketType socketType() const { return std::get<0>(GetParam()); }
- RpcSecurity rpcSecurity() const { return std::get<1>(GetParam()); }
- uint32_t clientVersion() const { return std::get<2>(GetParam()); }
- uint32_t serverVersion() const { return std::get<3>(GetParam()); }
- bool serverSingleThreaded() const { return std::get<4>(GetParam()); }
- bool noKernel() const { return std::get<5>(GetParam()); }
+ // TODO: avoid unnecessary layer of indirection
+ SocketType socketType() const { return GetParam().type; }
+ RpcSecurity rpcSecurity() const { return GetParam().security; }
+ uint32_t clientVersion() const { return GetParam().clientVersion; }
+ uint32_t serverVersion() const { return GetParam().serverVersion; }
+ bool serverSingleThreaded() const { return GetParam().singleThreaded; }
+ bool noKernel() const { return GetParam().noKernel; }
bool clientOrServerSingleThreaded() const {
return !kEnableRpcThreads || serverSingleThreaded();
@@ -148,15 +156,16 @@
}
static std::string PrintParamInfo(const testing::TestParamInfo<ParamType>& info) {
- auto [type, security, clientVersion, serverVersion, singleThreaded, noKernel] = info.param;
- auto ret = PrintToString(type) + "_" + newFactory(security)->toCString() + "_clientV" +
- std::to_string(clientVersion) + "_serverV" + std::to_string(serverVersion);
- if (singleThreaded) {
+ auto ret = PrintToString(info.param.type) + "_" +
+ newFactory(info.param.security)->toCString() + "_clientV" +
+ std::to_string(info.param.clientVersion) + "_serverV" +
+ std::to_string(info.param.serverVersion);
+ if (info.param.singleThreaded) {
ret += "_single_threaded";
} else {
ret += "_multi_threaded";
}
- if (noKernel) {
+ if (info.param.noKernel) {
ret += "_no_kernel";
} else {
ret += "_with_kernel";
diff --git a/libs/binder/tests/binderRpcTestTrusty.cpp b/libs/binder/tests/binderRpcTestTrusty.cpp
index 28be10d..fcb83bd 100644
--- a/libs/binder/tests/binderRpcTestTrusty.cpp
+++ b/libs/binder/tests/binderRpcTestTrusty.cpp
@@ -57,9 +57,9 @@
[](size_t n) { return n != 0; }),
"Non-zero incoming connections on Trusty");
- RpcSecurity rpcSecurity = std::get<1>(GetParam());
- uint32_t clientVersion = std::get<2>(GetParam());
- uint32_t serverVersion = std::get<3>(GetParam());
+ RpcSecurity rpcSecurity = GetParam().security;
+ uint32_t clientVersion = GetParam().clientVersion;
+ uint32_t serverVersion = GetParam().serverVersion;
auto ret = std::make_unique<TrustyProcessSession>();
@@ -89,12 +89,27 @@
return ret;
}
-INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc,
- ::testing::Combine(::testing::Values(SocketType::TIPC),
- ::testing::Values(RpcSecurity::RAW),
- ::testing::ValuesIn(testVersions()),
- ::testing::ValuesIn(testVersions()),
- ::testing::Values(false), ::testing::Values(true)),
+static std::vector<BinderRpc::ParamType> getTrustyBinderRpcParams() {
+ std::vector<BinderRpc::ParamType> ret;
+
+ for (const auto& clientVersion : testVersions()) {
+ for (const auto& serverVersion : testVersions()) {
+ ret.push_back(BinderRpc::ParamType{
+ .type = SocketType::TIPC,
+ .security = RpcSecurity::RAW,
+ .clientVersion = clientVersion,
+ .serverVersion = serverVersion,
+ // TODO: should we test both versions here?
+ .singleThreaded = false,
+ .noKernel = true,
+ });
+ }
+ }
+
+ return ret;
+}
+
+INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()),
BinderRpc::PrintParamInfo);
} // namespace android
diff --git a/libs/binder/tests/binderRpcUniversalTests.cpp b/libs/binder/tests/binderRpcUniversalTests.cpp
index 1f46010..e43508e 100644
--- a/libs/binder/tests/binderRpcUniversalTests.cpp
+++ b/libs/binder/tests/binderRpcUniversalTests.cpp
@@ -84,7 +84,7 @@
GTEST_SKIP() << "This test requires a multi-threaded service";
}
- SocketType type = std::get<0>(GetParam());
+ SocketType type = GetParam().type;
if (type == SocketType::PRECONNECTED || type == SocketType::UNIX ||
type == SocketType::UNIX_BOOTSTRAP || type == SocketType::UNIX_RAW) {
// we can't get port numbers for unix sockets
diff --git a/libs/binder/tests/parcel_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/Android.bp
index 35866ad..0d1503e 100644
--- a/libs/binder/tests/parcel_fuzzer/Android.bp
+++ b/libs/binder/tests/parcel_fuzzer/Android.bp
@@ -104,3 +104,28 @@
local_include_dirs: ["include_random_parcel"],
export_include_dirs: ["include_random_parcel"],
}
+
+cc_library {
+ name: "libbinder_random_parcel_seeds",
+ host_supported: true,
+ vendor_available: true,
+ target: {
+ darwin: {
+ enabled: false,
+ },
+ },
+ srcs: [
+ "random_parcel_seeds.cpp",
+ ],
+ shared_libs: [
+ "libbase",
+ "libbinder",
+ "libbinder_ndk",
+ "libcutils",
+ "libutils",
+ ],
+ local_include_dirs: [
+ "include_random_parcel_seeds",
+ ],
+ export_include_dirs: ["include_random_parcel_seeds"],
+}
diff --git a/libs/binder/tests/parcel_fuzzer/include_random_parcel_seeds/fuzzseeds/random_parcel_seeds.h b/libs/binder/tests/parcel_fuzzer/include_random_parcel_seeds/fuzzseeds/random_parcel_seeds.h
new file mode 100644
index 0000000..5755239
--- /dev/null
+++ b/libs/binder/tests/parcel_fuzzer/include_random_parcel_seeds/fuzzseeds/random_parcel_seeds.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2023 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 <android-base/file.h>
+#include <android-base/hex.h>
+#include <android-base/logging.h>
+
+#include <binder/Binder.h>
+#include <binder/Parcel.h>
+#include <binder/RecordedTransaction.h>
+
+#include <private/android_filesystem_config.h>
+
+#include <vector>
+
+using android::Parcel;
+using android::base::HexString;
+using std::vector;
+
+namespace android {
+namespace impl {
+// computes the bytes so that if they are passed to FuzzedDataProvider and
+// provider.ConsumeIntegralInRange<T>(min, max) is called, it will return val
+template <typename T>
+void writeReversedBuffer(std::vector<std::byte>& integralBuffer, T min, T max, T val);
+
+// Calls writeInBuffer method with min and max numeric limits of type T. This method
+// is reversal of ConsumeIntegral<T>() in FuzzedDataProvider
+template <typename T>
+void writeReversedBuffer(std::vector<std::byte>& integralBuffer, T val);
+} // namespace impl
+void generateSeedsFromRecording(base::borrowed_fd fd,
+ const binder::debug::RecordedTransaction& transaction);
+} // namespace android
diff --git a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp
index 45c3a90..47d2a0a 100644
--- a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp
+++ b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp
@@ -21,6 +21,8 @@
#include <binder/IPCThreadState.h>
#include <binder/ProcessState.h>
+#include <private/android_filesystem_config.h>
+
namespace android {
void fuzzService(const sp<IBinder>& binder, FuzzedDataProvider&& provider) {
@@ -33,6 +35,11 @@
.extraFds = {},
};
+ // Reserved bytes so that we don't have to change fuzzers and seed corpus if
+ // we introduce anything new in fuzzService.
+ std::vector<uint8_t> reservedBytes = provider.ConsumeBytes<uint8_t>(8);
+ (void)reservedBytes;
+
// always refresh the calling identity, because we sometimes set it below, but also,
// the code we're fuzzing might reset it
IPCThreadState::self()->clearCallingIdentity();
@@ -40,7 +47,12 @@
// Always take so that a perturbation of just the one ConsumeBool byte will always
// take the same path, but with a different UID. Without this, the fuzzer needs to
// guess both the change in value and the shift at the same time.
- int64_t maybeSetUid = provider.ConsumeIntegral<int64_t>();
+ int64_t maybeSetUid = provider.PickValueInArray<int64_t>(
+ {static_cast<int64_t>(AID_ROOT) << 32, static_cast<int64_t>(AID_SYSTEM) << 32,
+ provider.ConsumeIntegralInRange<int64_t>(static_cast<int64_t>(AID_ROOT) << 32,
+ static_cast<int64_t>(AID_USER) << 32),
+ provider.ConsumeIntegral<int64_t>()});
+
if (provider.ConsumeBool()) {
// set calling uid
IPCThreadState::self()->restoreCallingIdentity(maybeSetUid);
diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp
new file mode 100644
index 0000000..9e3e2ab
--- /dev/null
+++ b/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp
@@ -0,0 +1,146 @@
+/*
+ * Copyright (C) 2023 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 <android-base/file.h>
+#include <android-base/logging.h>
+
+#include <binder/RecordedTransaction.h>
+
+#include <fuzzseeds/random_parcel_seeds.h>
+
+using android::base::WriteFully;
+
+namespace android {
+namespace impl {
+template <typename T>
+std::vector<uint8_t> reverseBytes(T min, T max, T val) {
+ uint64_t range = static_cast<uint64_t>(max) - min;
+ uint64_t result = val - min;
+ size_t offset = 0;
+
+ std::vector<uint8_t> reverseData;
+ uint8_t reversed = 0;
+ reversed |= result;
+
+ while (offset < sizeof(T) * CHAR_BIT && (range >> offset) > 0) {
+ reverseData.push_back(reversed);
+ reversed = 0;
+ reversed |= (result >> CHAR_BIT);
+ result = result >> CHAR_BIT;
+ offset += CHAR_BIT;
+ }
+
+ return std::move(reverseData);
+}
+
+template <typename T>
+void writeReversedBuffer(std::vector<uint8_t>& integralBuffer, T min, T max, T val) {
+ std::vector<uint8_t> reversedData = reverseBytes(min, max, val);
+ // ConsumeIntegral Calls read buffer from the end. Keep inserting at the front of the buffer
+ // so that we can align fuzzService operations with seed generation for readability.
+ integralBuffer.insert(integralBuffer.begin(), reversedData.begin(), reversedData.end());
+}
+
+template <typename T>
+void writeReversedBuffer(std::vector<uint8_t>& integralBuffer, T val) {
+ // For ConsumeIntegral<T>() calls, FuzzedDataProvider uses numeric limits min and max
+ // as range
+ writeReversedBuffer(integralBuffer, std::numeric_limits<T>::min(),
+ std::numeric_limits<T>::max(), val);
+}
+
+} // namespace impl
+
+void generateSeedsFromRecording(base::borrowed_fd fd,
+ const binder::debug::RecordedTransaction& transaction) {
+ // Write Reserved bytes for future use
+ std::vector<uint8_t> reservedBytes(8);
+ CHECK(WriteFully(fd, reservedBytes.data(), reservedBytes.size())) << fd.get();
+
+ std::vector<uint8_t> integralBuffer;
+
+ // Write UID array : Array elements are initialized in the order that they are declared
+ // UID array index 2 element
+ // int64_t aidRoot = 0;
+ impl::writeReversedBuffer(integralBuffer, static_cast<int64_t>(AID_ROOT) << 32,
+ static_cast<int64_t>(AID_USER) << 32,
+ static_cast<int64_t>(AID_ROOT) << 32);
+
+ // UID array index 3 element
+ impl::writeReversedBuffer(integralBuffer, static_cast<int64_t>(AID_ROOT) << 32);
+
+ // always pick AID_ROOT -> index 0
+ size_t uidIndex = 0;
+ impl::writeReversedBuffer(integralBuffer, static_cast<size_t>(0), static_cast<size_t>(3),
+ uidIndex);
+
+ // Never set uid in seed corpus
+ uint8_t writeUid = 0;
+ impl::writeReversedBuffer(integralBuffer, writeUid);
+
+ // Read random code. this will be from recorded transaction
+ uint8_t selectCode = 1;
+ impl::writeReversedBuffer(integralBuffer, selectCode);
+
+ // Get from recorded transaction
+ uint32_t code = transaction.getCode();
+ impl::writeReversedBuffer(integralBuffer, code);
+
+ // Get from recorded transaction
+ uint32_t flags = transaction.getFlags();
+ impl::writeReversedBuffer(integralBuffer, flags);
+
+ // always fuzz primary binder
+ size_t extraBindersIndex = 0;
+ impl::writeReversedBuffer(integralBuffer, static_cast<size_t>(0), static_cast<size_t>(0),
+ extraBindersIndex);
+
+ const Parcel& dataParcel = transaction.getDataParcel();
+
+ // This buffer holds the bytes which will be used for fillRandomParcel API
+ std::vector<uint8_t> fillParcelBuffer;
+
+ // Don't take rpc path
+ uint8_t rpcBranch = 0;
+ impl::writeReversedBuffer(fillParcelBuffer, rpcBranch);
+
+ // Implicit branch on this path -> options->writeHeader(p, provider)
+ uint8_t writeHeaderInternal = 0;
+ impl::writeReversedBuffer(fillParcelBuffer, writeHeaderInternal);
+
+ // Choose to write data in parcel
+ size_t fillFuncIndex = 0;
+ impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), static_cast<size_t>(2),
+ fillFuncIndex);
+
+ // Write parcel data size from recorded transaction
+ size_t toWrite = transaction.getDataParcel().dataBufferSize();
+ impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), toWrite, toWrite);
+
+ // Write parcel data with size towrite from recorded transaction
+ CHECK(WriteFully(fd, dataParcel.data(), toWrite)) << fd.get();
+
+ // Write Fill Parcel buffer size in integralBuffer so that fuzzService knows size of data
+ size_t subDataSize = toWrite + fillParcelBuffer.size();
+ impl::writeReversedBuffer(integralBuffer, static_cast<size_t>(0), subDataSize, subDataSize);
+
+ // Write fill parcel buffer
+ CHECK(WriteFully(fd, fillParcelBuffer.data(), fillParcelBuffer.size())) << fd.get();
+
+ // Write the integralBuffer to data
+ CHECK(WriteFully(fd, integralBuffer.data(), integralBuffer.size())) << fd.get();
+}
+} // namespace android
diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/test_fuzzer/Android.bp
index 690c39a..96092b1 100644
--- a/libs/binder/tests/parcel_fuzzer/test_fuzzer/Android.bp
+++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/Android.bp
@@ -36,8 +36,8 @@
triage_assignee: "waghpawan@google.com",
// This fuzzer should be used only test fuzzService locally
- fuzz_on_haiku_host: false,
- fuzz_on_haiku_device: false,
+ fuzz_on_haiku_host: true,
+ fuzz_on_haiku_device: true,
},
}
diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/TestServiceFuzzer.cpp b/libs/binder/tests/parcel_fuzzer/test_fuzzer/TestServiceFuzzer.cpp
index 7fbf2d0..46205d7 100644
--- a/libs/binder/tests/parcel_fuzzer/test_fuzzer/TestServiceFuzzer.cpp
+++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/TestServiceFuzzer.cpp
@@ -20,6 +20,8 @@
#include <binder/IPCThreadState.h>
#include <log/log.h>
+#include <private/android_filesystem_config.h>
+
using android::binder::Status;
namespace android {
@@ -29,6 +31,8 @@
ON_PLAIN,
ON_BINDER,
ON_KNOWN_UID,
+ ON_SYSTEM_AID,
+ ON_ROOT_AID,
};
// This service is to verify that fuzzService is functioning properly
@@ -48,6 +52,18 @@
}
break;
}
+ case CrashType::ON_SYSTEM_AID: {
+ if (IPCThreadState::self()->getCallingUid() == AID_SYSTEM) {
+ LOG_ALWAYS_FATAL("Expected crash, AID_SYSTEM.");
+ }
+ break;
+ }
+ case CrashType::ON_ROOT_AID: {
+ if (IPCThreadState::self()->getCallingUid() == AID_ROOT) {
+ LOG_ALWAYS_FATAL("Expected crash, AID_ROOT.");
+ }
+ break;
+ }
default:
break;
}
@@ -99,6 +115,10 @@
gCrashType = CrashType::ON_PLAIN;
} else if (arg == "KNOWN_UID") {
gCrashType = CrashType::ON_KNOWN_UID;
+ } else if (arg == "AID_SYSTEM") {
+ gCrashType = CrashType::ON_SYSTEM_AID;
+ } else if (arg == "AID_ROOT") {
+ gCrashType = CrashType::ON_ROOT_AID;
} else if (arg == "BINDER") {
gCrashType = CrashType::ON_BINDER;
} else {
diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/run_fuzz_service_test.sh b/libs/binder/tests/parcel_fuzzer/test_fuzzer/run_fuzz_service_test.sh
index e568035..25906d8 100755
--- a/libs/binder/tests/parcel_fuzzer/test_fuzzer/run_fuzz_service_test.sh
+++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/run_fuzz_service_test.sh
@@ -27,7 +27,7 @@
exit 1
fi
-for CRASH_TYPE in PLAIN KNOWN_UID BINDER; do
+for CRASH_TYPE in PLAIN KNOWN_UID AID_SYSTEM AID_ROOT BINDER; do
echo "INFO: Running fuzzer : test_service_fuzzer_should_crash $CRASH_TYPE"
./test_service_fuzzer_should_crash "$CRASH_TYPE" -max_total_time=30 &>"$FUZZER_OUT"
diff --git a/libs/binder/tests/rpc_fuzzer/main.cpp b/libs/binder/tests/rpc_fuzzer/main.cpp
index b8ae84d..dcc8b8e 100644
--- a/libs/binder/tests/rpc_fuzzer/main.cpp
+++ b/libs/binder/tests/rpc_fuzzer/main.cpp
@@ -135,7 +135,7 @@
// b/260736889 - limit arbitrarily, due to thread resource exhaustion, which currently
// aborts. Servers should consider RpcServer::setConnectionFilter instead.
- constexpr size_t kMaxConnections = 1000;
+ constexpr size_t kMaxConnections = 10;
while (provider.remaining_bytes() > 0) {
if (connections.empty() ||
diff --git a/libs/binder/trusty/fuzzer/Android.bp b/libs/binder/trusty/fuzzer/Android.bp
new file mode 100644
index 0000000..2f1f54b
--- /dev/null
+++ b/libs/binder/trusty/fuzzer/Android.bp
@@ -0,0 +1,39 @@
+// Copyright (C) 2023 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.
+
+package {
+ default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+cc_fuzz {
+ name: "trusty_binder_fuzzer",
+ defaults: ["trusty_fuzzer_defaults"],
+ srcs: [":trusty_tipc_fuzzer"],
+ cflags: [
+ "-DTRUSTY_APP_PORT=\"com.android.trusty.binder.test.service\"",
+ "-DTRUSTY_APP_UUID=\"d42f06c5-9dc5-455b-9914-cf094116cfa8\"",
+ "-DTRUSTY_APP_FILENAME=\"binder-test-service.syms.elf\"",
+ ],
+}
+
+cc_fuzz {
+ name: "trusty_binder_rpc_fuzzer",
+ defaults: ["trusty_fuzzer_defaults"],
+ srcs: [":trusty_tipc_fuzzer"],
+ cflags: [
+ "-DTRUSTY_APP_PORT=\"com.android.trusty.binderRpcTestService.V0\"",
+ "-DTRUSTY_APP_UUID=\"87e424e5-69d7-4bbd-8b7c-7e24812cbc94\"",
+ "-DTRUSTY_APP_FILENAME=\"binderRpcTestService.syms.elf\"",
+ ],
+}
diff --git a/libs/fakeservicemanager/FakeServiceManager.cpp b/libs/fakeservicemanager/FakeServiceManager.cpp
index 3272bbc..80661c1 100644
--- a/libs/fakeservicemanager/FakeServiceManager.cpp
+++ b/libs/fakeservicemanager/FakeServiceManager.cpp
@@ -26,6 +26,8 @@
}
sp<IBinder> FakeServiceManager::checkService( const String16& name) const {
+ std::lock_guard<std::mutex> l(mMutex);
+
auto it = mNameToService.find(name);
if (it == mNameToService.end()) {
return nullptr;
@@ -36,6 +38,8 @@
status_t FakeServiceManager::addService(const String16& name, const sp<IBinder>& service,
bool /*allowIsolated*/,
int /*dumpsysFlags*/) {
+ std::lock_guard<std::mutex> l(mMutex);
+
if (service == nullptr) {
return UNEXPECTED_NULL;
}
@@ -44,6 +48,8 @@
}
Vector<String16> FakeServiceManager::listServices(int /*dumpsysFlags*/) {
+ std::lock_guard<std::mutex> l(mMutex);
+
Vector<String16> services;
for (auto const& [name, service] : mNameToService) {
(void) service;
@@ -61,10 +67,14 @@
}
bool FakeServiceManager::isDeclared(const String16& name) {
+ std::lock_guard<std::mutex> l(mMutex);
+
return mNameToService.find(name) != mNameToService.end();
}
Vector<String16> FakeServiceManager::getDeclaredInstances(const String16& name) {
+ std::lock_guard<std::mutex> l(mMutex);
+
Vector<String16> out;
const String16 prefix = name + String16("/");
for (const auto& [registeredName, service] : mNameToService) {
@@ -108,6 +118,8 @@
}
void FakeServiceManager::clear() {
+ std::lock_guard<std::mutex> l(mMutex);
+
mNameToService.clear();
}
} // namespace android
diff --git a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
index 97add24..f62241d 100644
--- a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
+++ b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
@@ -19,6 +19,7 @@
#include <binder/IServiceManager.h>
#include <map>
+#include <mutex>
#include <optional>
#include <vector>
@@ -68,6 +69,7 @@
void clear();
private:
+ mutable std::mutex mMutex;
std::map<String16, sp<IBinder>> mNameToService;
};
diff --git a/libs/gui/OWNERS b/libs/gui/OWNERS
index 826a418..070f6bf 100644
--- a/libs/gui/OWNERS
+++ b/libs/gui/OWNERS
@@ -1,3 +1,5 @@
+# Bug component: 1075131
+
chrisforbes@google.com
jreck@google.com
diff --git a/libs/renderengine/OWNERS b/libs/renderengine/OWNERS
index 5d23a5e..66e1aa1 100644
--- a/libs/renderengine/OWNERS
+++ b/libs/renderengine/OWNERS
@@ -1,3 +1,5 @@
+# Bug component: 1075131
+
adyabr@google.com
alecmouri@google.com
djsollen@google.com
diff --git a/libs/sensor/SensorManager.cpp b/libs/sensor/SensorManager.cpp
index 40061cd..9f814f1 100644
--- a/libs/sensor/SensorManager.cpp
+++ b/libs/sensor/SensorManager.cpp
@@ -176,11 +176,8 @@
mSensors = mSensorServer->getSensorList(mOpPackageName);
size_t count = mSensors.size();
- if (count == 0) {
- ALOGE("Failed to get Sensor list");
- mSensorServer.clear();
- return UNKNOWN_ERROR;
- }
+ // If count is 0, mSensorList will be non-null. This is old
+ // existing behavior and callers expect this.
mSensorList =
static_cast<Sensor const**>(malloc(count * sizeof(Sensor*)));
LOG_ALWAYS_FATAL_IF(mSensorList == nullptr, "mSensorList NULL");
diff --git a/libs/ui/DebugUtils.cpp b/libs/ui/DebugUtils.cpp
index 073da89..8675f14 100644
--- a/libs/ui/DebugUtils.cpp
+++ b/libs/ui/DebugUtils.cpp
@@ -304,6 +304,12 @@
return std::string("BGRA_8888");
case android::PIXEL_FORMAT_R_8:
return std::string("R_8");
+ case android::PIXEL_FORMAT_R_16_UINT:
+ return std::string("R_16_UINT");
+ case android::PIXEL_FORMAT_RG_1616_UINT:
+ return std::string("RG_1616_UINT");
+ case android::PIXEL_FORMAT_RGBA_10101010:
+ return std::string("RGBA_10101010");
default:
return StringPrintf("Unknown %#08x", format);
}
diff --git a/libs/ui/include/ui/FatVector.h b/libs/ui/include/ui/FatVector.h
index cb61e6a..494272b 100644
--- a/libs/ui/include/ui/FatVector.h
+++ b/libs/ui/include/ui/FatVector.h
@@ -65,6 +65,17 @@
free(p);
}
}
+
+ // The STL checks that this member type is present so that
+ // std::allocator_traits<InlineStdAllocator<T, SIZE>>::rebind_alloc<Other>
+ // works. std::vector won't be able to construct an
+ // InlineStdAllocator<Other, SIZE>, because InlineStdAllocator has no
+ // default constructor, but vector presumably doesn't rebind the allocator
+ // because it doesn't allocate internal node types.
+ template <class Other>
+ struct rebind {
+ typedef InlineStdAllocator<Other, SIZE> other;
+ };
Allocation& mAllocation;
};
diff --git a/services/inputflinger/OWNERS b/services/inputflinger/OWNERS
index c88bfe9..21d208f 100644
--- a/services/inputflinger/OWNERS
+++ b/services/inputflinger/OWNERS
@@ -1 +1,2 @@
+# Bug component: 136048
include platform/frameworks/base:/INPUT_OWNERS
diff --git a/services/sensorservice/SensorFusion.cpp b/services/sensorservice/SensorFusion.cpp
index e27b52b..5c00260 100644
--- a/services/sensorservice/SensorFusion.cpp
+++ b/services/sensorservice/SensorFusion.cpp
@@ -19,6 +19,7 @@
#include "SensorService.h"
#include <android/util/ProtoOutputStream.h>
+#include <cutils/properties.h>
#include <frameworks/base/core/proto/android/service/sensor_service.proto.h>
namespace android {
@@ -60,10 +61,12 @@
mGyro = uncalibratedGyro;
}
- // 200 Hz for gyro events is a good compromise between precision
- // and power/cpu usage.
- mEstimatedGyroRate = 200;
- mTargetDelayNs = 1000000000LL/mEstimatedGyroRate;
+ // Wearable devices will want to tune this parameter
+ // to 100 (Hz) in device.mk to save some power.
+ int32_t value = property_get_int32(
+ "sensors.aosp_low_power_sensor_fusion.maximum_rate", 200);
+ mEstimatedGyroRate = static_cast<float>(value);
+ mTargetDelayNs = 1000000000LL / mEstimatedGyroRate;
for (int i = 0; i<NUM_FUSION_MODE; ++i) {
mFusions[i].init(i);
diff --git a/services/sensorservice/tests/sensorservicetest.cpp b/services/sensorservice/tests/sensorservicetest.cpp
index caf7f03..1406bb3 100644
--- a/services/sensorservice/tests/sensorservicetest.cpp
+++ b/services/sensorservice/tests/sensorservicetest.cpp
@@ -130,7 +130,7 @@
printf("ALOOPER_POLL_TIMEOUT\n");
break;
case ALOOPER_POLL_ERROR:
- printf("ALOOPER_POLL_TIMEOUT\n");
+ printf("ALOOPER_POLL_ERROR\n");
break;
default:
printf("ugh? poll returned %d\n", ret);
diff --git a/services/surfaceflinger/OWNERS b/services/surfaceflinger/OWNERS
index 4e7da82..3270e4c 100644
--- a/services/surfaceflinger/OWNERS
+++ b/services/surfaceflinger/OWNERS
@@ -1,6 +1,9 @@
+# Bug component: 1075131
+
adyabr@google.com
alecmouri@google.com
chaviw@google.com
+domlaskowski@google.com
lpy@google.com
pdwilliams@google.com
racarr@google.com
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 8c46515..64348a5 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -6755,7 +6755,7 @@
if (captureListener) {
// TODO: The future returned by std::async blocks the main thread. Return a chain of
// futures to the Binder thread instead.
- std::async([=]() mutable {
+ (void)std::async([=]() mutable {
ATRACE_NAME("captureListener is nonnull!");
auto fenceResult = renderFuture.get();
// TODO(b/232535621): Change ScreenCaptureResults to store a FenceResult.
diff --git a/services/vibratorservice/OWNERS b/services/vibratorservice/OWNERS
index d073e2b..031b333 100644
--- a/services/vibratorservice/OWNERS
+++ b/services/vibratorservice/OWNERS
@@ -1 +1,3 @@
+# Bug component: 345036
+
include platform/frameworks/base:/services/core/java/com/android/server/vibrator/OWNERS
diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp
index c46036a..64393be 100644
--- a/vulkan/libvulkan/swapchain.cpp
+++ b/vulkan/libvulkan/swapchain.cpp
@@ -16,6 +16,7 @@
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
+#include <aidl/android/hardware/graphics/common/PixelFormat.h>
#include <android/hardware/graphics/common/1.0/types.h>
#include <grallocusage/GrallocUsageConversion.h>
#include <graphicsenv/GraphicsEnv.h>
@@ -25,8 +26,6 @@
#include <sync/sync.h>
#include <system/window.h>
#include <ui/BufferQueueDefs.h>
-#include <ui/DebugUtils.h>
-#include <ui/PixelFormat.h>
#include <utils/StrongPointer.h>
#include <utils/Timers.h>
#include <utils/Trace.h>
@@ -37,6 +36,7 @@
#include "driver.h"
+using PixelFormat = aidl::android::hardware::graphics::common::PixelFormat;
using android::hardware::graphics::common::V1_0::BufferUsage;
namespace vulkan {
@@ -489,27 +489,27 @@
*count = num_copied;
}
-android::PixelFormat GetNativePixelFormat(VkFormat format) {
- android::PixelFormat native_format = android::PIXEL_FORMAT_RGBA_8888;
+PixelFormat GetNativePixelFormat(VkFormat format) {
+ PixelFormat native_format = PixelFormat::RGBA_8888;
switch (format) {
case VK_FORMAT_R8G8B8A8_UNORM:
case VK_FORMAT_R8G8B8A8_SRGB:
- native_format = android::PIXEL_FORMAT_RGBA_8888;
+ native_format = PixelFormat::RGBA_8888;
break;
case VK_FORMAT_R5G6B5_UNORM_PACK16:
- native_format = android::PIXEL_FORMAT_RGB_565;
+ native_format = PixelFormat::RGB_565;
break;
case VK_FORMAT_R16G16B16A16_SFLOAT:
- native_format = android::PIXEL_FORMAT_RGBA_FP16;
+ native_format = PixelFormat::RGBA_FP16;
break;
case VK_FORMAT_A2B10G10R10_UNORM_PACK32:
- native_format = android::PIXEL_FORMAT_RGBA_1010102;
+ native_format = PixelFormat::RGBA_1010102;
break;
case VK_FORMAT_R8_UNORM:
- native_format = android::PIXEL_FORMAT_R_8;
+ native_format = PixelFormat::R_8;
break;
case VK_FORMAT_R10X6G10X6B10X6A10X6_UNORM_4PACK16:
- native_format = android::PIXEL_FORMAT_RGBA_10101010;
+ native_format = PixelFormat::RGBA_10101010;
break;
default:
ALOGV("unsupported swapchain format %d", format);
@@ -1256,7 +1256,7 @@
if (!allocator)
allocator = &GetData(device).allocator;
- android::PixelFormat native_pixel_format =
+ PixelFormat native_pixel_format =
GetNativePixelFormat(create_info->imageFormat);
android_dataspace native_dataspace =
GetNativeDataspace(create_info->imageColorSpace);
@@ -1351,10 +1351,11 @@
const auto& dispatch = GetData(device).driver;
- err = native_window_set_buffers_format(window, native_pixel_format);
+ err = native_window_set_buffers_format(
+ window, static_cast<int>(native_pixel_format));
if (err != android::OK) {
ALOGE("native_window_set_buffers_format(%s) failed: %s (%d)",
- decodePixelFormat(native_pixel_format).c_str(), strerror(-err), err);
+ toString(native_pixel_format).c_str(), strerror(-err), err);
return VK_ERROR_SURFACE_LOST_KHR;
}