Merge "Add profile for VM compilation"
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index dcf92be..9b96f36 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -1,8 +1,10 @@
[Builtin Hooks]
clang_format = true
+rustfmt = true
[Builtin Hooks Options]
clang_format = --commit ${PREUPLOAD_COMMIT} --style file --extensions c,h,cc,cpp
+rustfmt = --config-path=rustfmt.toml
[Hook Scripts]
aosp_hook = ${REPO_ROOT}/frameworks/base/tools/aosp/aosp_sha.sh ${PREUPLOAD_COMMIT} "."
diff --git a/debuggerd/TEST_MAPPING b/debuggerd/TEST_MAPPING
index efc13df..394447b 100644
--- a/debuggerd/TEST_MAPPING
+++ b/debuggerd/TEST_MAPPING
@@ -6,5 +6,10 @@
{
"name": "libtombstoned_client_rust_test"
}
+ ],
+ "hwasan-presubmit": [
+ {
+ "name": "debuggerd_test"
+ }
]
}
diff --git a/debuggerd/crasher/Android.bp b/debuggerd/crasher/Android.bp
index 23b106e..799163e 100644
--- a/debuggerd/crasher/Android.bp
+++ b/debuggerd/crasher/Android.bp
@@ -45,6 +45,8 @@
shared_libs: [
"libbase",
"liblog",
+ ],
+ static_libs: [
"libseccomp_policy",
],
multilib: {
diff --git a/debuggerd/crasher/crasher.cpp b/debuggerd/crasher/crasher.cpp
index db30b8f..55490b5 100644
--- a/debuggerd/crasher/crasher.cpp
+++ b/debuggerd/crasher/crasher.cpp
@@ -39,6 +39,8 @@
#include "debuggerd/handler.h"
#endif
+extern "C" void android_set_abort_message(const char* msg);
+
#if defined(__arm__)
// See https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt for details.
#define __kuser_helper_version (*(int32_t*) 0xffff0ffc)
@@ -182,6 +184,8 @@
fprintf(stderr, " leak leak memory until we get OOM-killed\n");
fprintf(stderr, "\n");
fprintf(stderr, " abort call abort()\n");
+ fprintf(stderr, " abort_with_msg call abort() setting an abort message\n");
+ fprintf(stderr, " abort_with_null_msg call abort() setting a null abort message\n");
fprintf(stderr, " assert call assert() without a function\n");
fprintf(stderr, " assert2 call assert() with a function\n");
fprintf(stderr, " exit call exit(1)\n");
@@ -259,6 +263,12 @@
return crash(42);
} else if (!strcasecmp(arg, "abort")) {
maybe_abort();
+ } else if (!strcasecmp(arg, "abort_with_msg")) {
+ android_set_abort_message("Aborting due to crasher");
+ maybe_abort();
+ } else if (!strcasecmp(arg, "abort_with_null")) {
+ android_set_abort_message(nullptr);
+ maybe_abort();
} else if (!strcasecmp(arg, "assert")) {
__assert("some_file.c", 123, "false");
} else if (!strcasecmp(arg, "assert2")) {
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index a5e2413..5a9d4f2 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -377,6 +377,8 @@
#if !defined(__aarch64__)
GTEST_SKIP() << "Requires aarch64";
#endif
+ // HWASan crashes with SIGABRT on tag mismatch.
+ SKIP_WITH_HWASAN;
int intercept_result;
unique_fd output_fd;
StartProcess([]() {
@@ -408,6 +410,10 @@
#if defined(__i386__)
GTEST_SKIP() << "architecture does not pass arguments in registers";
#endif
+ // The memory dump in HWASan crashes sadly shows the memory near the registers
+ // in the HWASan dump function, rather the faulting context. This is a known
+ // issue.
+ SKIP_WITH_HWASAN;
int intercept_result;
unique_fd output_fd;
StartProcess([]() {
@@ -486,6 +492,8 @@
// instead of GWP-ASan.
GTEST_SKIP() << "Skipped on MTE.";
}
+ // Skip this test on HWASan, which will reliably catch test errors as well.
+ SKIP_WITH_HWASAN;
GwpAsanTestParameters params = GetParam();
LogcatCollector logcat_collector;
@@ -2021,6 +2029,9 @@
// Verify that a fault address after the last map is properly handled.
TEST_F(CrasherTest, fault_address_after_last_map) {
+ // This makes assumptions about the memory layout that are not true in HWASan
+ // processes.
+ SKIP_WITH_HWASAN;
uintptr_t crash_uptr = untag_address(UINTPTR_MAX - 15);
StartProcess([crash_uptr]() {
ASSERT_EQ(0, crash_call(crash_uptr));
diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp
index b8b9262..8c719c8 100644
--- a/fs_mgr/fs_mgr_fstab.cpp
+++ b/fs_mgr/fs_mgr_fstab.cpp
@@ -653,6 +653,7 @@
entry->blk_device = partition;
// AVB keys for DSU should always be under kDsuKeysDir.
entry->avb_keys = kDsuKeysDir;
+ entry->fs_mgr_flags.logical = true;
}
// Make sure the ext4 is included to support GSI.
auto partition_ext4 =
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index 38b47d5..11da568 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -538,6 +538,9 @@
// Unmap a COW and remove it from a MetadataBuilder.
void UnmapAndDeleteCowPartition(MetadataBuilder* current_metadata);
+ // Remove invalid snapshots if any
+ void RemoveInvalidSnapshots(LockedFile* lock);
+
// Unmap and remove all known snapshots.
bool RemoveAllSnapshots(LockedFile* lock);
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index d086f29..a83f535 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -218,7 +218,10 @@
if (!file) return false;
UpdateState state = ReadUpdateState(file.get());
- if (state == UpdateState::None) return true;
+ if (state == UpdateState::None) {
+ RemoveInvalidSnapshots(file.get());
+ return true;
+ }
if (state == UpdateState::Initiated) {
LOG(INFO) << "Update has been initiated, now canceling";
@@ -1903,6 +1906,33 @@
return true;
}
+void SnapshotManager::RemoveInvalidSnapshots(LockedFile* lock) {
+ std::vector<std::string> snapshots;
+
+ // Remove the stale snapshot metadata
+ //
+ // We make sure that all the three cases
+ // are valid before removing the snapshot metadata:
+ //
+ // 1: dm state is active
+ // 2: Root fs is not mounted off as a snapshot device
+ // 3: Snapshot slot suffix should match current device slot
+ if (!ListSnapshots(lock, &snapshots, device_->GetSlotSuffix()) || snapshots.empty()) {
+ return;
+ }
+
+ // We indeed have some invalid snapshots
+ for (const auto& name : snapshots) {
+ if (dm_.GetState(name) == DmDeviceState::ACTIVE && !IsSnapshotDevice(name)) {
+ if (!DeleteSnapshot(lock, name)) {
+ LOG(ERROR) << "Failed to delete invalid snapshot: " << name;
+ } else {
+ LOG(INFO) << "Invalid snapshot: " << name << " deleted";
+ }
+ }
+ }
+}
+
bool SnapshotManager::RemoveAllSnapshots(LockedFile* lock) {
std::vector<std::string> snapshots;
if (!ListSnapshots(lock, &snapshots)) {
diff --git a/init/epoll.cpp b/init/epoll.cpp
index 0580f86..74d8aac 100644
--- a/init/epoll.cpp
+++ b/init/epoll.cpp
@@ -23,8 +23,6 @@
#include <functional>
#include <map>
-#include <android-base/logging.h>
-
namespace android {
namespace init {
@@ -44,11 +42,8 @@
if (!events) {
return Error() << "Must specify events";
}
-
- Info info;
- info.events = events;
- info.handler = std::make_shared<decltype(handler)>(std::move(handler));
- auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(info));
+ auto sp = std::make_shared<decltype(handler)>(std::move(handler));
+ auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(sp));
if (!inserted) {
return Error() << "Cannot specify two epoll handlers for a given FD";
}
@@ -89,14 +84,8 @@
}
std::vector<std::shared_ptr<Handler>> pending_functions;
for (int i = 0; i < num_events; ++i) {
- auto& info = *reinterpret_cast<Info*>(ev[i].data.ptr);
- if ((info.events & (EPOLLIN | EPOLLPRI)) == (EPOLLIN | EPOLLPRI) &&
- (ev[i].events & EPOLLIN) != ev[i].events) {
- // This handler wants to know about exception events, and just got one.
- // Log something informational.
- LOG(ERROR) << "Received unexpected epoll event set: " << ev[i].events;
- }
- pending_functions.emplace_back(info.handler);
+ auto sp = *reinterpret_cast<std::shared_ptr<Handler>*>(ev[i].data.ptr);
+ pending_functions.emplace_back(std::move(sp));
}
return pending_functions;
diff --git a/init/epoll.h b/init/epoll.h
index f58ae8d..0df5289 100644
--- a/init/epoll.h
+++ b/init/epoll.h
@@ -46,13 +46,8 @@
std::optional<std::chrono::milliseconds> timeout);
private:
- struct Info {
- std::shared_ptr<Handler> handler;
- uint32_t events;
- };
-
android::base::unique_fd epoll_fd_;
- std::map<int, Info> epoll_handlers_;
+ std::map<int, std::shared_ptr<Handler>> epoll_handlers_;
};
} // namespace init
diff --git a/init/init.cpp b/init/init.cpp
index 5a0b3a6..038f172 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -661,8 +661,7 @@
PLOG(FATAL) << "failed to create signalfd";
}
- constexpr int flags = EPOLLIN | EPOLLPRI;
- if (auto result = epoll->RegisterHandler(signal_fd, HandleSignalFd, flags); !result.ok()) {
+ if (auto result = epoll->RegisterHandler(signal_fd, HandleSignalFd); !result.ok()) {
LOG(FATAL) << result.error();
}
}
@@ -796,6 +795,10 @@
InstallRebootSignalHandlers();
}
+ // No threads should be spin up until signalfd
+ // is registered. If the threads are indeed required,
+ // each of these threads _should_ make sure SIGCHLD signal
+ // is blocked. See b/223076262
boot_clock::time_point start_time = boot_clock::now();
trigger_shutdown = [](const std::string& command) { shutdown_state.TriggerShutdown(command); };
diff --git a/init/sigchld_handler.cpp b/init/sigchld_handler.cpp
index 6fc64df..9b2c7d9 100644
--- a/init/sigchld_handler.cpp
+++ b/init/sigchld_handler.cpp
@@ -95,10 +95,7 @@
LOG(INFO) << name << " received signal " << siginfo.si_status << wait_string;
}
- if (!service) {
- LOG(INFO) << name << " did not have an associated service entry and will not be reaped";
- return pid;
- }
+ if (!service) return pid;
service->Reap(siginfo);
diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index 54772b6..edda414 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -159,6 +159,20 @@
return TaskProfiles::GetInstance().SetTaskProfiles(tid, profiles, use_fd_cache);
}
+// C wrapper for SetProcessProfiles.
+// No need to have this in the header file because this function is specifically for crosvm. Crosvm
+// which is written in Rust has its own declaration of this foreign function and doesn't rely on the
+// header. See
+// https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3574427/5/src/linux/android.rs#12
+extern "C" bool android_set_process_profiles(uid_t uid, pid_t pid, size_t num_profiles,
+ const char* profiles[]) {
+ std::vector<std::string> profiles_(num_profiles);
+ for (size_t i = 0; i < num_profiles; i++) {
+ profiles_.emplace_back(profiles[i]);
+ }
+ return SetProcessProfiles(uid, pid, profiles_);
+}
+
static std::string ConvertUidToPath(const char* cgroup, uid_t uid) {
return StringPrintf("%s/uid_%d", cgroup, uid);
}
diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index 27060ae..e1c5934 100644
--- a/libprocessgroup/task_profiles.cpp
+++ b/libprocessgroup/task_profiles.cpp
@@ -806,6 +806,7 @@
bool TaskProfiles::SetProcessProfiles(uid_t uid, pid_t pid,
const std::vector<std::string>& profiles, bool use_fd_cache) {
+ bool success = true;
for (const auto& name : profiles) {
TaskProfile* profile = GetProfile(name);
if (profile != nullptr) {
@@ -814,16 +815,19 @@
}
if (!profile->ExecuteForProcess(uid, pid)) {
PLOG(WARNING) << "Failed to apply " << name << " process profile";
+ success = false;
}
} else {
- PLOG(WARNING) << "Failed to find " << name << "process profile";
+ PLOG(WARNING) << "Failed to find " << name << " process profile";
+ success = false;
}
}
- return true;
+ return success;
}
bool TaskProfiles::SetTaskProfiles(int tid, const std::vector<std::string>& profiles,
bool use_fd_cache) {
+ bool success = true;
for (const auto& name : profiles) {
TaskProfile* profile = GetProfile(name);
if (profile != nullptr) {
@@ -832,10 +836,12 @@
}
if (!profile->ExecuteForTask(tid)) {
PLOG(WARNING) << "Failed to apply " << name << " task profile";
+ success = false;
}
} else {
- PLOG(WARNING) << "Failed to find " << name << "task profile";
+ PLOG(WARNING) << "Failed to find " << name << " task profile";
+ success = false;
}
}
- return true;
+ return success;
}
diff --git a/libutils/Android.bp b/libutils/Android.bp
index a9bd7d9..1b29285 100644
--- a/libutils/Android.bp
+++ b/libutils/Android.bp
@@ -188,6 +188,7 @@
defaults: ["libutils_defaults"],
// TODO(b/153609531): remove when no longer needed.
native_bridge_supported: true,
+ min_sdk_version: "29",
srcs: [
"CallStack.cpp",
diff --git a/rootdir/init.rc b/rootdir/init.rc
index 135acce..d39a21c 100644
--- a/rootdir/init.rc
+++ b/rootdir/init.rc
@@ -829,7 +829,7 @@
mkdir /data/misc/odsign/metrics 0770 root system
# Directory for VirtualizationService temporary image files.
- mkdir /data/misc/virtualizationservice 0700 virtualizationservice virtualizationservice
+ mkdir /data/misc/virtualizationservice 0700 system system
mkdir /data/preloads 0775 system system encryption=None
@@ -1306,3 +1306,15 @@
on property:sys.boot_completed=1 && property:sys.init.userspace_reboot.in_progress=1
setprop sys.init.userspace_reboot.in_progress ""
+
+# Multi-Gen LRU Experiment
+on property:persist.device_config.mglru_native.lru_gen_config=none
+ write /sys/kernel/mm/lru_gen/enabled 0
+on property:persist.device_config.mglru_native.lru_gen_config=core
+ write /sys/kernel/mm/lru_gen/enabled 1
+on property:persist.device_config.mglru_native.lru_gen_config=core_and_mm_walk
+ write /sys/kernel/mm/lru_gen/enabled 3
+on property:persist.device_config.mglru_native.lru_gen_config=core_and_nonleaf_young
+ write /sys/kernel/mm/lru_gen/enabled 5
+on property:persist.device_config.mglru_native.lru_gen_config=all
+ write /sys/kernel/mm/lru_gen/enabled 7
diff --git a/rootdir/ueventd.rc b/rootdir/ueventd.rc
index 3101974..a140c8c 100644
--- a/rootdir/ueventd.rc
+++ b/rootdir/ueventd.rc
@@ -67,9 +67,8 @@
# CDMA radio interface MUX
/dev/ppp 0660 radio vpn
-# Virtualization is managed by VirtualizationService.
-/dev/kvm 0600 virtualizationservice root
-/dev/vhost-vsock 0600 virtualizationservice root
+/dev/kvm 0600 system system
+/dev/vhost-vsock 0600 system system
# sysfs properties
/sys/devices/platform/trusty.* trusty_version 0440 root log
diff --git a/rustfmt.toml b/rustfmt.toml
new file mode 120000
index 0000000..ee92d9e
--- /dev/null
+++ b/rustfmt.toml
@@ -0,0 +1 @@
+../../build/soong/scripts/rustfmt.toml
\ No newline at end of file
diff --git a/trusty/apploader/apploader.cpp b/trusty/apploader/apploader.cpp
index c72af40..278499f 100644
--- a/trusty/apploader/apploader.cpp
+++ b/trusty/apploader/apploader.cpp
@@ -223,6 +223,9 @@
case APPLOADER_ERR_INVALID_VERSION:
LOG(ERROR) << "Error: invalid application version";
break;
+ case APPLOADER_ERR_POLICY_VIOLATION:
+ LOG(ERROR) << "Error: loading denied by policy engine";
+ break;
default:
LOG(ERROR) << "Unrecognized error: " << resp.error;
break;
diff --git a/trusty/apploader/apploader_ipc.h b/trusty/apploader/apploader_ipc.h
index 6cda7c1..306596e 100644
--- a/trusty/apploader/apploader_ipc.h
+++ b/trusty/apploader/apploader_ipc.h
@@ -56,6 +56,7 @@
APPLOADER_ERR_ALREADY_EXISTS,
APPLOADER_ERR_INTERNAL,
APPLOADER_ERR_INVALID_VERSION,
+ APPLOADER_ERR_POLICY_VIOLATION,
};
/**
diff --git a/trusty/apploader/fuzz/Android.bp b/trusty/apploader/fuzz/Android.bp
index e37dab1..c961b36 100644
--- a/trusty/apploader/fuzz/Android.bp
+++ b/trusty/apploader/fuzz/Android.bp
@@ -25,7 +25,10 @@
"-DTRUSTY_APP_PORT=\"com.android.trusty.apploader\"",
"-DTRUSTY_APP_UUID=\"081ba88f-f1ee-452e-b5e8-a7e9ef173a97\"",
"-DTRUSTY_APP_FILENAME=\"apploader.syms.elf\"",
- ]
+ ],
+ fuzz_config: {
+ cc: ["trong@google.com"],
+ },
}
// Fuzz app package sent to apploader.
@@ -37,4 +40,7 @@
shared_libs: [
"libdmabufheap",
],
+ fuzz_config: {
+ cc: ["trong@google.com"],
+ },
}
diff --git a/trusty/confirmationui/fuzz/Android.bp b/trusty/confirmationui/fuzz/Android.bp
index ba57191..4780943 100644
--- a/trusty/confirmationui/fuzz/Android.bp
+++ b/trusty/confirmationui/fuzz/Android.bp
@@ -25,6 +25,9 @@
"-DTRUSTY_APP_UUID=\"7dee2364-c036-425b-b086-df0f6c233c1b\"",
"-DTRUSTY_APP_FILENAME=\"confirmationui.syms.elf\"",
],
+ fuzz_config: {
+ cc: ["trong@google.com"],
+ },
}
@@ -36,6 +39,9 @@
shared_libs: [
"libdmabufheap",
],
+ fuzz_config: {
+ cc: ["trong@google.com"],
+ },
// The initial corpus for this fuzzer was derived by dumping messages from/to
// HAL to/from TA triggered by VtsHalConfirmationUIV1_0TargetTest.
diff --git a/trusty/gatekeeper/fuzz/Android.bp b/trusty/gatekeeper/fuzz/Android.bp
index d084cb6..67d0c0f 100644
--- a/trusty/gatekeeper/fuzz/Android.bp
+++ b/trusty/gatekeeper/fuzz/Android.bp
@@ -25,6 +25,9 @@
"-DTRUSTY_APP_UUID=\"38ba0cdc-df0e-11e4-9869-233fb6ae4795\"",
"-DTRUSTY_APP_FILENAME=\"gatekeeper.syms.elf\"",
],
+ fuzz_config: {
+ cc: ["trong@google.com"],
+ },
// The initial corpus for this fuzzer was derived by dumping messages from
// the `secure_env` emulator interface for cuttlefish while enrolling a new
diff --git a/trusty/keymaster/fuzz/Android.bp b/trusty/keymaster/fuzz/Android.bp
index 8d7ee00..5f24bc6 100644
--- a/trusty/keymaster/fuzz/Android.bp
+++ b/trusty/keymaster/fuzz/Android.bp
@@ -25,6 +25,9 @@
"-DTRUSTY_APP_UUID=\"5f902ace-5e5c-4cd8-ae54-87b88c22ddaf\"",
"-DTRUSTY_APP_FILENAME=\"keymaster.syms.elf\"",
],
+ fuzz_config: {
+ cc: ["trong@google.com"],
+ },
// The initial corpus for this fuzzer was derived by dumping messages from
// the `secure_env` emulator interface for cuttlefish while running the
diff --git a/trusty/libtrusty-rs/Android.bp b/trusty/libtrusty-rs/Android.bp
index 47c64b7..bc1dcf6 100644
--- a/trusty/libtrusty-rs/Android.bp
+++ b/trusty/libtrusty-rs/Android.bp
@@ -24,6 +24,7 @@
],
rustlibs: [
"libnix",
+ "liblibc",
],
}
@@ -33,5 +34,6 @@
srcs: ["tests/test.rs"],
rustlibs: [
"libtrusty-rs",
+ "liblibc",
]
}
diff --git a/trusty/libtrusty-rs/src/lib.rs b/trusty/libtrusty-rs/src/lib.rs
index b3102f8..28ea075 100644
--- a/trusty/libtrusty-rs/src/lib.rs
+++ b/trusty/libtrusty-rs/src/lib.rs
@@ -51,8 +51,8 @@
//!
//! chann.send("Hello, world!".as_bytes()).unwrap();
//!
-//! let mut read_buf = [0u8; 1024];
-//! let read_len = stream.read(&mut read_buf[..]).unwrap();
+//! let mut read_buf = Vec::new();
+//! let read_len = stream.recv(&mut read_buf).unwrap();
//!
//! let response = std::str::from_utf8(&read_buf[..read_len]).unwrap();
//! assert_eq!("Hello, world!", response);
@@ -63,8 +63,8 @@
use crate::sys::tipc_connect;
use std::ffi::CString;
use std::fs::File;
-use std::io;
use std::io::prelude::*;
+use std::io::{ErrorKind, Result};
use std::os::unix::prelude::AsRawFd;
use std::path::Path;
@@ -73,6 +73,12 @@
/// The default filesystem path for the Trusty IPC device.
pub const DEFAULT_DEVICE: &str = "/dev/trusty-ipc-dev0";
+/// The maximum size an incoming TIPC message can be.
+///
+/// This can be used to pre-allocate buffer space in order to ensure that your
+/// read buffer can always hold an incoming message.
+pub const MAX_MESSAGE_SIZE: usize = 4096;
+
/// A channel for communicating with a Trusty service.
///
/// See the [crate-level documentation][crate] for usage details and examples.
@@ -92,7 +98,7 @@
/// bytes. This is handled with a panic because the service names are all
/// hard-coded constants, and so such an error should always be indicative of a
/// bug in the calling code.
- pub fn connect(device: impl AsRef<Path>, service: &str) -> io::Result<Self> {
+ pub fn connect(device: impl AsRef<Path>, service: &str) -> Result<Self> {
let file = File::options().read(true).write(true).open(device)?;
let srv_name = CString::new(service).expect("Service name contained null bytes");
@@ -107,7 +113,7 @@
///
/// The entire contents of `buf` will be sent as a single message to the
/// connected service.
- pub fn send(&mut self, buf: &[u8]) -> io::Result<()> {
+ pub fn send(&mut self, buf: &[u8]) -> Result<()> {
let write_len = self.0.write(buf)?;
// Verify that the expected number of bytes were written. The entire message
@@ -125,19 +131,91 @@
Ok(())
}
- /// Receives a message from the connected service.
+ /// Reads the next incoming message.
///
- /// Returns the number of bytes in the received message, or any error that
- /// occurred when reading the message. Blocks until there is a message to
- /// receive if none is already ready to read.
+ /// Attempts to read the next incoming message from the connected service if any
+ /// exist. If the initial capacity of `buf` is not enough to hold the incoming
+ /// message the function repeatedly attempts to reserve additional space until
+ /// it is able to fully read the message.
+ ///
+ /// Blocks until there is an incoming message if there is not already a message
+ /// ready to be received.
///
/// # Errors
///
- /// Returns an error with native error code 90 (`EMSGSIZE`) if `buf` isn't large
+ /// If this function encounters an error of the kind [`ErrorKind::Interrupted`]
+ /// then the error is ignored and the operation will be tried again.
+ ///
+ /// If this function encounters an error with the error code `EMSGSIZE` then
+ /// additional space will be reserved in `buf` and the operation will be tried
+ /// again.
+ ///
+ /// If any other read error is encountered then this function immediately
+ /// returns the error to the caller, and the length of `buf` is set to 0.
+ pub fn recv(&mut self, buf: &mut Vec<u8>) -> Result<()> {
+ // If no space has been allocated in the buffer reserve enough space to hold any
+ // incoming message.
+ if buf.capacity() == 0 {
+ buf.reserve(MAX_MESSAGE_SIZE);
+ }
+
+ loop {
+ // Resize the vec to make its full capacity available to write into.
+ buf.resize(buf.capacity(), 0);
+
+ match self.0.read(buf.as_mut_slice()) {
+ Ok(len) => {
+ buf.truncate(len);
+ return Ok(());
+ }
+
+ Err(err) => {
+ if let Some(libc::EMSGSIZE) = err.raw_os_error() {
+ // Ensure that we didn't get `EMSGSIZE` when we already had enough capacity
+ // to contain the maximum message size. This should never happen, but if it
+ // does we don't want to hang by looping infinitely.
+ assert!(
+ buf.capacity() < MAX_MESSAGE_SIZE,
+ "Received `EMSGSIZE` error when buffer capacity was already at maximum",
+ );
+
+ // If we didn't have enough space to hold the incoming message, reserve
+ // enough space to fit the maximum message size regardless of how much
+ // capacity the buffer already had.
+ buf.reserve(MAX_MESSAGE_SIZE - buf.capacity());
+ } else if err.kind() == ErrorKind::Interrupted {
+ // If we get an interrupted error the operation can be retried as-is, i.e.
+ // we don't need to allocate additional space.
+ continue;
+ } else {
+ buf.truncate(0);
+ return Err(err);
+ }
+ }
+ }
+ }
+ }
+
+ /// Reads the next incoming message without allocating.
+ ///
+ /// Returns the number of bytes in the received message, or any error that
+ /// occurred when reading the message.
+ ///
+ /// Blocks until there is an incoming message if there is not already a message
+ /// ready to be received.
+ ///
+ /// # Errors
+ ///
+ /// Returns an error with native error code `EMSGSIZE` if `buf` isn't large
/// enough to contain the incoming message. Use
/// [`raw_os_error`][std::io::Error::raw_os_error] to check the error code to
- /// determine if you need to increase the size of `buf`.
- pub fn recv(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+ /// determine if you need to increase the size of `buf`. If error code
+ /// `EMSGSIZE` is returned the incoming message will not be dropped, and a
+ /// subsequent call to `recv_no_alloc` can still read it.
+ ///
+ /// An error of the [`ErrorKind::Interrupted`] kind is non-fatal and the read
+ /// operation should be retried if there is nothing else to do.
+ pub fn recv_no_alloc(&mut self, buf: &mut [u8]) -> Result<usize> {
self.0.read(buf)
}
diff --git a/trusty/libtrusty-rs/tests/test.rs b/trusty/libtrusty-rs/tests/test.rs
index a6f1370..6bff479 100644
--- a/trusty/libtrusty-rs/tests/test.rs
+++ b/trusty/libtrusty-rs/tests/test.rs
@@ -3,7 +3,7 @@
const ECHO_NAME: &str = "com.android.ipc-unittest.srv.echo";
#[test]
-fn echo() {
+fn recv_no_alloc() {
let mut connection = TipcChannel::connect(DEFAULT_DEVICE, ECHO_NAME)
.expect("Failed to connect to Trusty service");
@@ -11,9 +11,10 @@
let send_buf = [7u8; 32];
connection.send(send_buf.as_slice()).unwrap();
- // Receive the response message from the TA.
+ // Receive the response message from the TA. The response message will be the
+ // same as the message we just sent.
let mut recv_buf = [0u8; 32];
- let read_len = connection.recv(&mut recv_buf).expect("Failed to read from connection");
+ let read_len = connection.recv_no_alloc(recv_buf.as_mut_slice()).unwrap();
assert_eq!(
send_buf.len(),
@@ -24,3 +25,62 @@
);
assert_eq!(send_buf, recv_buf, "Received data does not match sent data");
}
+
+#[test]
+fn recv_small_buf() {
+ let mut connection = TipcChannel::connect(DEFAULT_DEVICE, ECHO_NAME)
+ .expect("Failed to connect to Trusty service");
+
+ // Send a long message to the echo service so that we can test receiving a long
+ // message.
+ let send_buf = [7u8; 2048];
+ connection.send(send_buf.as_slice()).unwrap();
+
+ // Attempt to receive the response message with a buffer that is too small to
+ // contain the message.
+ let mut recv_buf = [0u8; 32];
+ let err = connection.recv_no_alloc(recv_buf.as_mut_slice()).unwrap_err();
+
+ assert_eq!(
+ Some(libc::EMSGSIZE),
+ err.raw_os_error(),
+ "Unexpected error err when receiving incoming message: {:?}",
+ err,
+ );
+}
+
+#[test]
+fn recv_empty_vec() {
+ let mut connection = TipcChannel::connect(DEFAULT_DEVICE, ECHO_NAME)
+ .expect("Failed to connect to Trusty service");
+
+ // Send a message to the echo TA.
+ let send_buf = [7u8; 2048];
+ connection.send(send_buf.as_slice()).unwrap();
+
+ // Receive the response message. `recv_buf` is initially empty, and `recv` is
+ // responsible for allocating enough space to hold the message.
+ let mut recv_buf = Vec::new();
+ connection.recv(&mut recv_buf).unwrap();
+
+ assert_eq!(send_buf.as_slice(), recv_buf, "Received data does not match sent data");
+}
+
+#[test]
+fn recv_vec_existing_capacity() {
+ let mut connection = TipcChannel::connect(DEFAULT_DEVICE, ECHO_NAME)
+ .expect("Failed to connect to Trusty service");
+
+ // Send a message to the echo TA.
+ let send_buf = [7u8; 2048];
+ connection.send(send_buf.as_slice()).unwrap();
+
+ // Receive the response message into a buffer that already has enough capacity
+ // to hold the message. No additional capacity should be allocated when
+ // receiving the message.
+ let mut recv_buf = Vec::with_capacity(2048);
+ connection.recv(&mut recv_buf).unwrap();
+
+ assert_eq!(send_buf.as_slice(), recv_buf, "Received data does not match sent data");
+ assert_eq!(2048, recv_buf.capacity(), "Additional capacity was allocated when not needed");
+}