Merge "binder_rpc_fuzzer: improve speed by reducing the no of connections" 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 e84428e..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();
@@ -2374,11 +2407,15 @@
p->fts_number = p->fts_parent->fts_number;
switch (p->fts_info) {
case FTS_D:
- if (p->fts_level == 4
+ if (p->fts_level == 3
+ && !strcmp(p->fts_parent->fts_name, "obb")
+ && !strcmp(p->fts_parent->fts_parent->fts_name, "Android")) {
+ p->fts_number = 1;
+ } else if (p->fts_level == 4
&& !strcmp(p->fts_name, "cache")
&& !strcmp(p->fts_parent->fts_parent->fts_name, "data")
&& !strcmp(p->fts_parent->fts_parent->fts_parent->fts_name, "Android")) {
- p->fts_number = 1;
+ p->fts_number = 2;
}
[[fallthrough]]; // to count the directory
case FTS_DEFAULT:
@@ -2387,9 +2424,13 @@
case FTS_SLNONE:
int64_t size = (p->fts_statp->st_blocks * 512);
if (p->fts_number == 1) {
- stats->cacheSize += size;
+ stats->codeSize += size;
+ } else {
+ if (p->fts_number == 2) {
+ stats->cacheSize += size;
+ }
+ stats->dataSize += size;
}
- stats->dataSize += size;
break;
}
}
@@ -2663,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.
@@ -2735,11 +2777,6 @@
extStats.dataSize = dataSize;
atrace_pm_end();
} else {
- atrace_pm_begin("obb");
- auto obbPath = create_data_path(uuid_) + "/media/obb";
- calculate_tree_size(obbPath, &extStats.codeSize);
- atrace_pm_end();
-
atrace_pm_begin("code");
calculate_tree_size(create_data_app_path(uuid_), &stats.codeSize);
atrace_pm_end();
@@ -2770,9 +2807,10 @@
atrace_pm_begin("external");
auto dataMediaPath = create_data_media_path(uuid_, userId);
collectManualExternalStatsForUser(dataMediaPath, &extStats);
+
#if MEASURE_DEBUG
LOG(DEBUG) << "Measured external data " << extStats.dataSize << " cache "
- << extStats.cacheSize;
+ << extStats.cacheSize << " code " << extStats.codeSize;
#endif
atrace_pm_end();
@@ -2802,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.
@@ -2922,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);
@@ -2971,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();
@@ -3014,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);
@@ -3257,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();
@@ -3267,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);
@@ -3298,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);
@@ -3749,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();
@@ -3771,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/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/data/etc/Android.bp b/data/etc/Android.bp
index bdd5172..92dc46e 100644
--- a/data/etc/Android.bp
+++ b/data/etc/Android.bp
@@ -167,6 +167,18 @@
}
prebuilt_etc {
+ name: "android.hardware.telephony.satellite.prebuilt.xml",
+ src: "android.hardware.telephony.satellite.xml",
+ defaults: ["frameworks_native_data_etc_defaults"],
+}
+
+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.telephony.satellite.xml b/data/etc/android.hardware.telephony.satellite.xml
new file mode 100644
index 0000000..5966cba
--- /dev/null
+++ b/data/etc/android.hardware.telephony.satellite.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2022 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.
+-->
+
+<!-- Feature for devices that support Satellite communication via Satellite HAL APIs. -->
+<permissions>
+ <feature name="android.hardware.telephony.satellite" />
+</permissions>
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/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/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 b90b40b..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 {
@@ -260,7 +260,14 @@
/// Trying to use this function on a local binder will result in an
/// INVALID_OPERATION code being returned and nothing happening.
///
- /// This link always holds a weak reference to its recipient.
+ /// This link only holds a weak reference to its recipient. If the
+ /// `DeathRecipient` is dropped then it will be unlinked.
+ ///
+ /// Note that the notifications won't work if you don't first start at least
+ /// one Binder thread by calling
+ /// [`ProcessState::start_thread_pool`](crate::ProcessState::start_thread_pool)
+ /// or
+ /// [`ProcessState::join_thread_pool`](crate::ProcessState::join_thread_pool).
fn link_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()>;
/// Remove a previously registered death notification.
@@ -290,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),
@@ -331,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()
@@ -535,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() }
}
};
}
@@ -657,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())
@@ -917,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()
}
}
@@ -1018,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..8d9ce0e 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,9 @@
}
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() }
}
}
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 b35511d..a3a2562 100644
--- a/libs/binder/rust/src/state.rs
+++ b/libs/binder/rust/src/state.rs
@@ -22,35 +22,48 @@
pub struct ProcessState;
impl ProcessState {
- /// Start the Binder IPC thread pool
+ /// Starts the Binder IPC thread pool.
///
- /// Starts 1 thread, plus allows the kernel to lazily start up to 'num_threads'
- /// additional threads as specified by set_thread_pool_max_thread_count.
+ /// Starts 1 thread, plus allows the kernel to lazily start up to
+ /// `num_threads` additional threads as specified by
+ /// [`set_thread_pool_max_thread_count`](Self::set_thread_pool_max_thread_count).
+ ///
+ /// This should be done before creating any Binder client or server. If
+ /// neither this nor [`join_thread_pool`](Self::join_thread_pool) are
+ /// called, then some things (such as callbacks and
+ /// [`IBinder::link_to_death`](crate::IBinder::link_to_death)) will silently
+ /// 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();
}
}
- /// Set the maximum number of threads that can be started in the threadpool.
+ /// Sets the maximum number of threads that can be started in the
+ /// threadpool.
///
- /// By default, after startThreadPool is called, this is 15. If it is called
- /// additional times, the thread pool size can only be increased.
+ /// By default, after [`start_thread_pool`](Self::start_thread_pool) is
+ /// 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);
}
}
- /// Block on the Binder IPC thread pool
+ /// Blocks on the Binder IPC thread pool by adding the current thread to the
+ /// pool.
///
- /// This adds additional threads in addition to what is created by
- /// set_thread_pool_max_thread_count and start_thread_pool.
+ /// Note that this adds the current thread in addition to those that are
+ /// created by
+ /// [`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();
}
}
@@ -73,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
@@ -98,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.
@@ -109,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/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/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index d352ce5..38c7f7c 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -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;
diff --git a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp
index 45c3a90..b268c5d 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) {
@@ -40,7 +42,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/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/gui/tests/OWNERS b/libs/gui/tests/OWNERS
new file mode 100644
index 0000000..156efdb
--- /dev/null
+++ b/libs/gui/tests/OWNERS
@@ -0,0 +1,3 @@
+# Android > Android OS & Apps > Framework (Java + Native) > Window Manager > Surfaces
+# Bug component: 316245 = per-file BLASTBufferQueue_test.cpp, DisplayInfo_test.cpp, EndToEndNativeInputTest.cpp, WindowInfos_test.cpp
+# Buganizer template url: https://b.corp.google.com/issues/new?component=316245&template=1018194 = per-file BLASTBufferQueue_test.cpp, DisplayInfo_test.cpp, EndToEndNativeInputTest.cpp, WindowInfos_test.cpp
diff --git a/libs/input/OWNERS b/libs/input/OWNERS
new file mode 100644
index 0000000..c88bfe9
--- /dev/null
+++ b/libs/input/OWNERS
@@ -0,0 +1 @@
+include platform/frameworks/base:/INPUT_OWNERS
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..4734097 100644
--- a/services/surfaceflinger/OWNERS
+++ b/services/surfaceflinger/OWNERS
@@ -1,6 +1,7 @@
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;
}