diff options
81 files changed, 1063 insertions, 564 deletions
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 2b94b71a7f..0d051161c7 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -829,7 +829,8 @@ status_t Dumpstate::AddZipEntryFromFd(const std::string& entry_name, int fd, // Logging statement below is useful to time how long each entry takes, but it's too verbose. // MYLOGD("Adding zip entry %s\n", entry_name.c_str()); - int32_t err = zip_writer_->StartEntryWithTime(valid_name.c_str(), ZipWriter::kCompress, + size_t flags = ZipWriter::kCompress | ZipWriter::kDefaultCompression; + int32_t err = zip_writer_->StartEntryWithTime(valid_name.c_str(), flags, get_mtime(fd, ds.now_)); if (err != 0) { MYLOGE("zip_writer_->StartEntryWithTime(%s): %s\n", valid_name.c_str(), @@ -921,7 +922,8 @@ void Dumpstate::AddDir(const std::string& dir, bool recursive) { bool Dumpstate::AddTextZipEntry(const std::string& entry_name, const std::string& content) { MYLOGD("Adding zip text entry %s\n", entry_name.c_str()); - int32_t err = zip_writer_->StartEntryWithTime(entry_name.c_str(), ZipWriter::kCompress, ds.now_); + size_t flags = ZipWriter::kCompress | ZipWriter::kDefaultCompression; + int32_t err = zip_writer_->StartEntryWithTime(entry_name.c_str(), flags, ds.now_); if (err != 0) { MYLOGE("zip_writer_->StartEntryWithTime(%s): %s\n", entry_name.c_str(), ZipWriter::ErrorCodeString(err)); diff --git a/cmds/installd/Android.bp b/cmds/installd/Android.bp index c9f680b266..0f7c48964f 100644 --- a/cmds/installd/Android.bp +++ b/cmds/installd/Android.bp @@ -72,8 +72,6 @@ cc_defaults { }, }, - clang: true, - tidy: true, tidy_checks: [ "-*", @@ -81,8 +79,9 @@ cc_defaults { "cert-*", "-cert-err58-cpp", ], - tidy_flags: [ - "-warnings-as-errors=clang-analyzer-security*,cert-*", + tidy_checks_as_errors: [ + "clang-analyzer-security*", + "cert-*", ], } @@ -127,7 +126,6 @@ cc_library_headers { cc_test_host { name: "run_dex2oat_test", test_suites: ["general-tests"], - clang: true, srcs: [ "run_dex2oat_test.cpp", "run_dex2oat.cpp", @@ -187,7 +185,6 @@ cc_binary { "-Wall", "-Werror", ], - clang: true, srcs: [ "otapreopt_chroot.cpp", diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index a49f563060..1d7dd5f51e 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -1869,8 +1869,9 @@ binder::Status InstalldNativeService::destroyUserData(const std::optional<std::s binder::Status res = ok(); if (flags & FLAG_STORAGE_DE) { auto path = create_data_user_de_path(uuid_, userId); - if (delete_dir_contents_and_dir(path, true) != 0) { - res = error("Failed to delete " + path); + // Contents only, as vold is responsible for the user_de dir itself. + if (delete_dir_contents(path, true) != 0) { + res = error("Failed to delete contents of " + path); } auto sdk_sandbox_de_path = create_data_misc_sdk_sandbox_path(uuid_, /*isCeData=*/false, userId); @@ -1890,8 +1891,9 @@ binder::Status InstalldNativeService::destroyUserData(const std::optional<std::s } if (flags & FLAG_STORAGE_CE) { auto path = create_data_user_ce_path(uuid_, userId); - if (delete_dir_contents_and_dir(path, true) != 0) { - res = error("Failed to delete " + path); + // Contents only, as vold is responsible for the user_ce dir itself. + if (delete_dir_contents(path, true) != 0) { + res = error("Failed to delete contents of " + path); } auto sdk_sandbox_ce_path = create_data_misc_sdk_sandbox_path(uuid_, /*isCeData=*/true, userId); @@ -1899,8 +1901,9 @@ binder::Status InstalldNativeService::destroyUserData(const std::optional<std::s res = error("Failed to delete " + sdk_sandbox_ce_path); } path = findDataMediaPath(uuid, userId); - if (delete_dir_contents_and_dir(path, true) != 0) { - res = error("Failed to delete " + path); + // Contents only, as vold is responsible for the media dir itself. + if (delete_dir_contents(path, true) != 0) { + res = error("Failed to delete contents of " + path); } } return res; @@ -2876,6 +2879,9 @@ binder::Status InstalldNativeService::getExternalSize(const std::optional<std::s auto obbPath = StringPrintf("%s/Android/obb", create_data_media_path(uuid_, userId).c_str()); calculate_tree_size(obbPath, &obbSize); + if (!(flags & FLAG_USE_QUOTA)) { + totalSize -= obbSize; + } ATRACE_END(); } @@ -3565,10 +3571,10 @@ binder::Status InstalldNativeService::tryMountDataMirror( return error("Failed to stat " + mirrorVolCePath); } - if (mirrorCeStat.st_ino == ceStat.st_ino) { + if (mirrorCeStat.st_ino == ceStat.st_ino && mirrorCeStat.st_dev == ceStat.st_dev) { // As it's being called by prepareUserStorage, it can be called multiple times. // Hence, we if we mount it already, we should skip it. - LOG(WARNING) << "CE dir is mounted already: " + cePath; + LOG(INFO) << "CE dir is mounted already: " + cePath; return ok(); } diff --git a/cmds/installd/tests/Android.bp b/cmds/installd/tests/Android.bp index b3baca5c41..07f73b9029 100644 --- a/cmds/installd/tests/Android.bp +++ b/cmds/installd/tests/Android.bp @@ -11,7 +11,6 @@ package { cc_test { name: "installd_utils_test", test_suites: ["device-tests"], - clang: true, srcs: ["installd_utils_test.cpp"], cflags: [ "-Wall", @@ -36,7 +35,6 @@ cc_test { cc_test { name: "installd_cache_test", test_suites: ["device-tests"], - clang: true, srcs: ["installd_cache_test.cpp"], cflags: [ "-Wall", @@ -82,7 +80,6 @@ cc_test { cc_test { name: "installd_service_test", test_suites: ["device-tests"], - clang: true, srcs: ["installd_service_test.cpp"], cflags: [ "-Wall", @@ -130,7 +127,6 @@ cc_test { cc_test { name: "installd_dexopt_test", test_suites: ["device-tests"], - clang: true, srcs: ["installd_dexopt_test.cpp"], cflags: [ "-Wall", @@ -177,7 +173,6 @@ cc_test { cc_test { name: "installd_otapreopt_test", test_suites: ["device-tests"], - clang: true, srcs: ["installd_otapreopt_test.cpp"], cflags: [ "-Wall", @@ -198,7 +193,6 @@ cc_test { cc_test { name: "installd_file_test", test_suites: ["device-tests"], - clang: true, srcs: ["installd_file_test.cpp"], cflags: [ "-Wall", diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp index 4eb30e2542..3849c40ac2 100644 --- a/cmds/installd/tests/installd_dexopt_test.cpp +++ b/cmds/installd/tests/installd_dexopt_test.cpp @@ -920,6 +920,11 @@ TEST_F(DexoptTest, ResolveStartupConstStrings) { TEST_F(DexoptTest, DexoptDex2oat64Enabled) { LOG(INFO) << "DexoptDex2oat64Enabled"; + std::string zygote_prop = android::base::GetProperty("ro.zygote", ""); + ASSERT_GT(zygote_prop.size(), 0); + if (zygote_prop != "zygote32_64" && zygote_prop != "zygote64_32") { + GTEST_SKIP() << "DexoptDex2oat64Enabled skipped for single-bitness Zygote."; + } const std::string property = "dalvik.vm.dex2oat64.enabled"; const std::string previous_value = android::base::GetProperty(property, ""); auto restore_property = android::base::make_scope_guard([=]() { diff --git a/libs/attestation/Android.bp b/libs/attestation/Android.bp index ea3c341fe4..2bf15d45eb 100644 --- a/libs/attestation/Android.bp +++ b/libs/attestation/Android.bp @@ -28,11 +28,9 @@ cc_library_static { "-Werror", ], srcs: [ - "HmacKeyManager.cpp" + "HmacKeyManager.cpp", ], - clang: true, - shared_libs: [ "liblog", "libcrypto", diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index d8d2cf2652..1403e418d0 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -199,7 +199,6 @@ cc_library { "libbinder_headers", ], - clang: true, sanitize: { misc_undefined: ["integer"], }, diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index 39befbe8e0..6a12e65fc0 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -49,10 +49,11 @@ static_assert(sizeof(IBinder) == 12); static_assert(sizeof(BBinder) == 20); #endif +// global b/c b/230079120 - consistent symbol table #ifdef BINDER_RPC_DEV_SERVERS -constexpr const bool kEnableRpcDevServers = true; +bool kEnableRpcDevServers = true; #else -constexpr const bool kEnableRpcDevServers = false; +bool kEnableRpcDevServers = false; #endif // Log any reply transactions for which the data exceeds this size @@ -156,7 +157,7 @@ status_t IBinder::getDebugPid(pid_t* out) { status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, const sp<IBinder>& keepAliveBinder) { - if constexpr (!kEnableRpcDevServers) { + if (!kEnableRpcDevServers) { ALOGW("setRpcClientDebug disallowed because RPC is not enabled"); return INVALID_OPERATION; } @@ -201,6 +202,7 @@ public: RpcServerLink(const sp<RpcServer>& rpcServer, const sp<IBinder>& keepAliveBinder, const wp<BBinder>& binder) : mRpcServer(rpcServer), mKeepAliveBinder(keepAliveBinder), mBinder(binder) {} + virtual ~RpcServerLink(); void binderDied(const wp<IBinder>&) override { LOG_RPC_DETAIL("RpcServerLink: binder died, shutting down RpcServer"); if (mRpcServer == nullptr) { @@ -226,6 +228,7 @@ private: sp<IBinder> mKeepAliveBinder; // hold to avoid automatically unlinking wp<BBinder> mBinder; }; +BBinder::RpcServerLink::~RpcServerLink() {} class BBinder::Extras { @@ -496,7 +499,7 @@ void BBinder::setParceled() { } status_t BBinder::setRpcClientDebug(const Parcel& data) { - if constexpr (!kEnableRpcDevServers) { + if (!kEnableRpcDevServers) { ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__); return INVALID_OPERATION; } @@ -521,7 +524,7 @@ status_t BBinder::setRpcClientDebug(const Parcel& data) { status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, const sp<IBinder>& keepAliveBinder) { - if constexpr (!kEnableRpcDevServers) { + if (!kEnableRpcDevServers) { ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__); return INVALID_OPERATION; } @@ -539,7 +542,7 @@ status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, return UNEXPECTED_NULL; } - size_t binderThreadPoolMaxCount = ProcessState::self()->getThreadPoolMaxThreadCount(); + size_t binderThreadPoolMaxCount = ProcessState::self()->getThreadPoolMaxTotalThreadCount(); if (binderThreadPoolMaxCount <= 1) { ALOGE("%s: ProcessState thread pool max count is %zu. RPC is disabled for this service " "because RPC requires the service to support multithreading.", diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp index 921e57c7bf..1eb2ffd22d 100644 --- a/libs/binder/BpBinder.cpp +++ b/libs/binder/BpBinder.cpp @@ -279,7 +279,7 @@ status_t BpBinder::transact( if (mAlive) { bool privateVendor = flags & FLAG_PRIVATE_VENDOR; // don't send userspace flags to the kernel - flags = flags & ~FLAG_PRIVATE_VENDOR; + flags = flags & ~static_cast<uint32_t>(FLAG_PRIVATE_VENDOR); // user transactions require a given stability level if (code >= FIRST_CALL_TRANSACTION && code <= LAST_CALL_TRANSACTION) { diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 3c97dcab93..d53621946a 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -638,7 +638,9 @@ void IPCThreadState::processPostWriteDerefs() void IPCThreadState::joinThreadPool(bool isMain) { LOG_THREADPOOL("**** THREAD %p (PID %d) IS JOINING THE THREAD POOL\n", (void*)pthread_self(), getpid()); - + pthread_mutex_lock(&mProcess->mThreadCountLock); + mProcess->mCurrentThreads++; + pthread_mutex_unlock(&mProcess->mThreadCountLock); mOut.writeInt32(isMain ? BC_ENTER_LOOPER : BC_REGISTER_LOOPER); mIsLooper = true; @@ -666,6 +668,13 @@ void IPCThreadState::joinThreadPool(bool isMain) mOut.writeInt32(BC_EXIT_LOOPER); mIsLooper = false; talkWithDriver(false); + pthread_mutex_lock(&mProcess->mThreadCountLock); + LOG_ALWAYS_FATAL_IF(mProcess->mCurrentThreads == 0, + "Threadpool thread count = 0. Thread cannot exist and exit in empty " + "threadpool\n" + "Misconfiguration. Increase threadpool max threads configuration\n"); + mProcess->mCurrentThreads--; + pthread_mutex_unlock(&mProcess->mThreadCountLock); } status_t IPCThreadState::setupPolling(int* fd) @@ -677,6 +686,9 @@ status_t IPCThreadState::setupPolling(int* fd) mOut.writeInt32(BC_ENTER_LOOPER); flushCommands(); *fd = mProcess->mDriverFD; + pthread_mutex_lock(&mProcess->mThreadCountLock); + mProcess->mCurrentThreads++; + pthread_mutex_unlock(&mProcess->mThreadCountLock); return 0; } @@ -989,6 +1001,7 @@ finish: if (acquireResult) *acquireResult = err; if (reply) reply->setError(err); mLastError = err; + logExtendedError(); } return err; @@ -1443,6 +1456,23 @@ status_t IPCThreadState::freeze(pid_t pid, bool enable, uint32_t timeout_ms) { return ret; } +void IPCThreadState::logExtendedError() { + struct binder_extended_error ee = {.command = BR_OK}; + + if (!ProcessState::isDriverFeatureEnabled(ProcessState::DriverFeature::EXTENDED_ERROR)) + return; + +#if defined(__ANDROID__) + if (ioctl(self()->mProcess->mDriverFD, BINDER_GET_EXTENDED_ERROR, &ee) < 0) { + ALOGE("Failed to get extended error: %s", strerror(errno)); + return; + } +#endif + + ALOGE_IF(ee.command != BR_OK, "Binder transaction failure: %d/%d/%d", + ee.id, ee.command, ee.param); +} + void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data, size_t /*dataSize*/, const binder_size_t* /*objects*/, diff --git a/libs/binder/MemoryHeapBase.cpp b/libs/binder/MemoryHeapBase.cpp index 8132d46940..8fe1d2bb3d 100644 --- a/libs/binder/MemoryHeapBase.cpp +++ b/libs/binder/MemoryHeapBase.cpp @@ -74,7 +74,7 @@ MemoryHeapBase::MemoryHeapBase(size_t size, uint32_t flags, char const * name) fd = memfd_create_region(name ? name : "MemoryHeapBase", size); if (fd < 0 || (mapfd(fd, true, size) != NO_ERROR)) return; const int SEAL_FLAGS = ((mFlags & READ_ONLY) ? F_SEAL_FUTURE_WRITE : 0) | - ((mFlags & MEMFD_ALLOW_SEALING) ? 0 : F_SEAL_SEAL); + ((mFlags & MEMFD_ALLOW_SEALING_FLAG) ? 0 : F_SEAL_SEAL); if (SEAL_FLAGS && (fcntl(fd, F_ADD_SEALS, SEAL_FLAGS) == -1)) { ALOGE("MemoryHeapBase: MemFD %s sealing with flags %x failed with error %s", name, SEAL_FLAGS, strerror(errno)); @@ -85,12 +85,9 @@ MemoryHeapBase::MemoryHeapBase(size_t size, uint32_t flags, char const * name) } return; #else - mFlags &= ~(FORCE_MEMFD | MEMFD_ALLOW_SEALING); + mFlags &= ~(FORCE_MEMFD | MEMFD_ALLOW_SEALING_FLAG); #endif } - if (mFlags & MEMFD_ALLOW_SEALING) { - LOG_ALWAYS_FATAL("Invalid Flags. MEMFD_ALLOW_SEALING only valid with FORCE_MEMFD."); - } fd = ashmem_create_region(name ? name : "MemoryHeapBase", size); ALOGE_IF(fd < 0, "MemoryHeapBase: error creating ashmem region: %s", strerror(errno)); if (fd < 0 || (mapfd(fd, true, size) != NO_ERROR)) return; @@ -103,7 +100,7 @@ MemoryHeapBase::MemoryHeapBase(const char* device, size_t size, uint32_t flags) : mFD(-1), mSize(0), mBase(MAP_FAILED), mFlags(flags), mDevice(nullptr), mNeedUnmap(false), mOffset(0) { - if (flags & (FORCE_MEMFD | MEMFD_ALLOW_SEALING)) { + if (flags & (FORCE_MEMFD | MEMFD_ALLOW_SEALING_FLAG)) { LOG_ALWAYS_FATAL("FORCE_MEMFD, MEMFD_ALLOW_SEALING only valid with creating constructor"); } int open_flags = O_RDWR; @@ -125,7 +122,7 @@ MemoryHeapBase::MemoryHeapBase(int fd, size_t size, uint32_t flags, off_t offset : mFD(-1), mSize(0), mBase(MAP_FAILED), mFlags(flags), mDevice(nullptr), mNeedUnmap(false), mOffset(0) { - if (flags & (FORCE_MEMFD | MEMFD_ALLOW_SEALING)) { + if (flags & (FORCE_MEMFD | MEMFD_ALLOW_SEALING_FLAG)) { LOG_ALWAYS_FATAL("FORCE_MEMFD, MEMFD_ALLOW_SEALING only valid with creating constructor"); } const size_t pagesize = getpagesize(); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 58b0b35323..038ce381e2 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -151,6 +151,10 @@ static void release_object(const sp<ProcessState>& proc, const flat_binder_objec ALOGE("Invalid object type 0x%08x", obj.hdr.type); } +Parcel::RpcFields::RpcFields(const sp<RpcSession>& session) : mSession(session) { + LOG_ALWAYS_FATAL_IF(mSession == nullptr); +} + status_t Parcel::finishFlattenBinder(const sp<IBinder>& binder) { internal::Stability::tryMarkCompilationUnit(binder.get()); @@ -182,13 +186,14 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder) { if (binder) local = binder->localBinder(); if (local) local->setParceled(); - if (isForRpc()) { + if (const auto* rpcFields = maybeRpcFields()) { if (binder) { status_t status = writeInt32(1); // non-null if (status != OK) return status; uint64_t address; // TODO(b/167966510): need to undo this if the Parcel is not sent - status = mSession->state()->onBinderLeaving(mSession, binder, &address); + status = rpcFields->mSession->state()->onBinderLeaving(rpcFields->mSession, binder, + &address); if (status != OK) return status; status = writeUint64(address); if (status != OK) return status; @@ -259,9 +264,7 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder) { status_t Parcel::unflattenBinder(sp<IBinder>* out) const { - if (isForRpc()) { - LOG_ALWAYS_FATAL_IF(mSession == nullptr, "RpcSession required to read from remote parcel"); - + if (const auto* rpcFields = maybeRpcFields()) { int32_t isPresent; status_t status = readInt32(&isPresent); if (status != OK) return status; @@ -271,10 +274,14 @@ status_t Parcel::unflattenBinder(sp<IBinder>* out) const if (isPresent & 1) { uint64_t addr; if (status_t status = readUint64(&addr); status != OK) return status; - if (status_t status = mSession->state()->onBinderEntering(mSession, addr, &binder); + if (status_t status = + rpcFields->mSession->state()->onBinderEntering(rpcFields->mSession, addr, + &binder); status != OK) return status; - if (status_t status = mSession->state()->flushExcessBinderRefs(mSession, addr, binder); + if (status_t status = + rpcFields->mSession->state()->flushExcessBinderRefs(rpcFields->mSession, + addr, binder); status != OK) return status; } @@ -378,8 +385,10 @@ void Parcel::setDataPosition(size_t pos) const } mDataPos = pos; - mNextObjectHint = 0; - mObjectsSorted = false; + if (const auto* kernelFields = maybeKernelFields()) { + kernelFields->mNextObjectHint = 0; + kernelFields->mObjectsSorted = false; + } } status_t Parcel::setDataCapacity(size_t size) @@ -406,25 +415,27 @@ status_t Parcel::setData(const uint8_t* buffer, size_t len) if (err == NO_ERROR) { memcpy(const_cast<uint8_t*>(data()), buffer, len); mDataSize = len; - mFdsKnown = false; + if (auto* kernelFields = maybeKernelFields()) { + kernelFields->mFdsKnown = false; + } } return err; } -status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) -{ - if (mSession != parcel->mSession) { +status_t Parcel::appendFrom(const Parcel* parcel, size_t offset, size_t len) { + if (isForRpc() != parcel->isForRpc()) { ALOGE("Cannot append Parcel from one context to another. They may be different formats, " "and objects are specific to a context."); return BAD_TYPE; } + if (isForRpc() && maybeRpcFields()->mSession != parcel->maybeRpcFields()->mSession) { + ALOGE("Cannot append Parcels from different sessions"); + return BAD_TYPE; + } status_t err; - const uint8_t *data = parcel->mData; - const binder_size_t *objects = parcel->mObjects; - size_t size = parcel->mObjectsSize; + const uint8_t* data = parcel->mData; int startPos = mDataPos; - int firstIndex = -1, lastIndex = -2; if (len == 0) { return NO_ERROR; @@ -443,18 +454,6 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) return BAD_VALUE; } - // Count objects in range - for (int i = 0; i < (int) size; i++) { - size_t off = objects[i]; - if ((off >= offset) && (off + sizeof(flat_binder_object) <= offset + len)) { - if (firstIndex == -1) { - firstIndex = i; - } - lastIndex = i; - } - } - int numObjects = lastIndex - firstIndex + 1; - if ((mDataSize+len) > mDataCapacity) { // grow data err = growData(len); @@ -470,43 +469,63 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) err = NO_ERROR; - if (numObjects > 0) { - const sp<ProcessState> proc(ProcessState::self()); - // grow objects - if (mObjectsCapacity < mObjectsSize + numObjects) { - if ((size_t) numObjects > SIZE_MAX - mObjectsSize) return NO_MEMORY; // overflow - if (mObjectsSize + numObjects > SIZE_MAX / 3) return NO_MEMORY; // overflow - size_t newSize = ((mObjectsSize + numObjects)*3)/2; - if (newSize > SIZE_MAX / sizeof(binder_size_t)) return NO_MEMORY; // overflow - binder_size_t *objects = - (binder_size_t*)realloc(mObjects, newSize*sizeof(binder_size_t)); - if (objects == (binder_size_t*)nullptr) { - return NO_MEMORY; + if (auto* kernelFields = maybeKernelFields()) { + auto* otherKernelFields = parcel->maybeKernelFields(); + LOG_ALWAYS_FATAL_IF(otherKernelFields == nullptr); + + const binder_size_t* objects = otherKernelFields->mObjects; + size_t size = otherKernelFields->mObjectsSize; + // Count objects in range + int firstIndex = -1, lastIndex = -2; + for (int i = 0; i < (int)size; i++) { + size_t off = objects[i]; + if ((off >= offset) && (off + sizeof(flat_binder_object) <= offset + len)) { + if (firstIndex == -1) { + firstIndex = i; + } + lastIndex = i; } - mObjects = objects; - mObjectsCapacity = newSize; } + int numObjects = lastIndex - firstIndex + 1; + if (numObjects > 0) { + const sp<ProcessState> proc(ProcessState::self()); + // grow objects + if (kernelFields->mObjectsCapacity < kernelFields->mObjectsSize + numObjects) { + if ((size_t)numObjects > SIZE_MAX - kernelFields->mObjectsSize) + return NO_MEMORY; // overflow + if (kernelFields->mObjectsSize + numObjects > SIZE_MAX / 3) + return NO_MEMORY; // overflow + size_t newSize = ((kernelFields->mObjectsSize + numObjects) * 3) / 2; + if (newSize > SIZE_MAX / sizeof(binder_size_t)) return NO_MEMORY; // overflow + binder_size_t* objects = (binder_size_t*)realloc(kernelFields->mObjects, + newSize * sizeof(binder_size_t)); + if (objects == (binder_size_t*)nullptr) { + return NO_MEMORY; + } + kernelFields->mObjects = objects; + kernelFields->mObjectsCapacity = newSize; + } + + // append and acquire objects + int idx = kernelFields->mObjectsSize; + for (int i = firstIndex; i <= lastIndex; i++) { + size_t off = objects[i] - offset + startPos; + kernelFields->mObjects[idx++] = off; + kernelFields->mObjectsSize++; + + flat_binder_object* flat = reinterpret_cast<flat_binder_object*>(mData + off); + acquire_object(proc, *flat, this); - // append and acquire objects - int idx = mObjectsSize; - for (int i = firstIndex; i <= lastIndex; i++) { - size_t off = objects[i] - offset + startPos; - mObjects[idx++] = off; - mObjectsSize++; - - flat_binder_object* flat - = reinterpret_cast<flat_binder_object*>(mData + off); - acquire_object(proc, *flat, this); - - if (flat->hdr.type == BINDER_TYPE_FD) { - // If this is a file descriptor, we need to dup it so the - // new Parcel now owns its own fd, and can declare that we - // officially know we have fds. - flat->handle = fcntl(flat->handle, F_DUPFD_CLOEXEC, 0); - flat->cookie = 1; - mHasFds = mFdsKnown = true; - if (!mAllowFds) { - err = FDS_NOT_ALLOWED; + if (flat->hdr.type == BINDER_TYPE_FD) { + // If this is a file descriptor, we need to dup it so the + // new Parcel now owns its own fd, and can declare that we + // officially know we have fds. + flat->handle = fcntl(flat->handle, F_DUPFD_CLOEXEC, 0); + flat->cookie = 1; + kernelFields->mHasFds = kernelFields->mFdsKnown = true; + if (!mAllowFds) { + err = FDS_NOT_ALLOWED; + } } } } @@ -563,18 +582,27 @@ void Parcel::restoreAllowFds(bool lastValue) bool Parcel::hasFileDescriptors() const { - if (!mFdsKnown) { + if (const auto* rpcFields = maybeRpcFields()) { + return false; + } + auto* kernelFields = maybeKernelFields(); + if (!kernelFields->mFdsKnown) { scanForFds(); } - return mHasFds; + return kernelFields->mHasFds; } std::vector<sp<IBinder>> Parcel::debugReadAllStrongBinders() const { std::vector<sp<IBinder>> ret; + const auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return ret; + } + size_t initPosition = dataPosition(); - for (size_t i = 0; i < mObjectsSize; i++) { - binder_size_t offset = mObjects[i]; + for (size_t i = 0; i < kernelFields->mObjectsSize; i++) { + binder_size_t offset = kernelFields->mObjects[i]; const flat_binder_object* flat = reinterpret_cast<const flat_binder_object*>(mData + offset); if (flat->hdr.type != BINDER_TYPE_BINDER) continue; @@ -592,9 +620,14 @@ std::vector<sp<IBinder>> Parcel::debugReadAllStrongBinders() const { std::vector<int> Parcel::debugReadAllFileDescriptors() const { std::vector<int> ret; + const auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return ret; + } + size_t initPosition = dataPosition(); - for (size_t i = 0; i < mObjectsSize; i++) { - binder_size_t offset = mObjects[i]; + for (size_t i = 0; i < kernelFields->mObjectsSize; i++) { + binder_size_t offset = kernelFields->mObjects[i]; const flat_binder_object* flat = reinterpret_cast<const flat_binder_object*>(mData + offset); if (flat->hdr.type != BINDER_TYPE_FD) continue; @@ -611,6 +644,10 @@ std::vector<int> Parcel::debugReadAllFileDescriptors() const { } status_t Parcel::hasFileDescriptorsInRange(size_t offset, size_t len, bool* result) const { + const auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return BAD_TYPE; + } if (len > INT32_MAX || offset > INT32_MAX) { // Don't accept size_t values which may have come from an inadvertent conversion from a // negative int. @@ -621,12 +658,15 @@ status_t Parcel::hasFileDescriptorsInRange(size_t offset, size_t len, bool* resu return BAD_VALUE; } *result = false; - for (size_t i = 0; i < mObjectsSize; i++) { - size_t pos = mObjects[i]; + for (size_t i = 0; i < kernelFields->mObjectsSize; i++) { + size_t pos = kernelFields->mObjects[i]; if (pos < offset) continue; if (pos + sizeof(flat_binder_object) > offset + len) { - if (mObjectsSorted) break; - else continue; + if (kernelFields->mObjectsSorted) { + break; + } else { + continue; + } } const flat_binder_object* flat = reinterpret_cast<const flat_binder_object*>(mData + pos); if (flat->hdr.type == BINDER_TYPE_FD) { @@ -654,20 +694,24 @@ void Parcel::markForRpc(const sp<RpcSession>& session) { LOG_ALWAYS_FATAL_IF(mData != nullptr && mOwner == nullptr, "format must be set before data is written OR on IPC data"); - LOG_ALWAYS_FATAL_IF(session == nullptr, "markForRpc requires session"); - mSession = session; + mVariantFields.emplace<RpcFields>(session); } bool Parcel::isForRpc() const { - return mSession != nullptr; + return std::holds_alternative<RpcFields>(mVariantFields); } void Parcel::updateWorkSourceRequestHeaderPosition() const { + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return; + } + // Only update the request headers once. We only want to point // to the first headers read/written. - if (!mRequestHeaderPresent) { - mWorkSourceRequestHeaderPosition = dataPosition(); - mRequestHeaderPresent = true; + if (!kernelFields->mRequestHeaderPresent) { + kernelFields->mWorkSourceRequestHeaderPosition = dataPosition(); + kernelFields->mRequestHeaderPresent = true; } } @@ -686,7 +730,7 @@ status_t Parcel::writeInterfaceToken(const String16& interface) } status_t Parcel::writeInterfaceToken(const char16_t* str, size_t len) { - if (CC_LIKELY(!isForRpc())) { + if (auto* kernelFields = maybeKernelFields()) { const IPCThreadState* threadState = IPCThreadState::self(); writeInt32(threadState->getStrictModePolicy() | STRICT_MODE_PENALTY_GATHER); updateWorkSourceRequestHeaderPosition(); @@ -701,12 +745,16 @@ status_t Parcel::writeInterfaceToken(const char16_t* str, size_t len) { bool Parcel::replaceCallingWorkSourceUid(uid_t uid) { - if (!mRequestHeaderPresent) { + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return false; + } + if (!kernelFields->mRequestHeaderPresent) { return false; } const size_t initialPosition = dataPosition(); - setDataPosition(mWorkSourceRequestHeaderPosition); + setDataPosition(kernelFields->mWorkSourceRequestHeaderPosition); status_t err = writeInt32(uid); setDataPosition(initialPosition); return err == NO_ERROR; @@ -714,12 +762,16 @@ bool Parcel::replaceCallingWorkSourceUid(uid_t uid) uid_t Parcel::readCallingWorkSourceUid() const { - if (!mRequestHeaderPresent) { + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return false; + } + if (!kernelFields->mRequestHeaderPresent) { return IPCThreadState::kUnsetWorkSource; } const size_t initialPosition = dataPosition(); - setDataPosition(mWorkSourceRequestHeaderPosition); + setDataPosition(kernelFields->mWorkSourceRequestHeaderPosition); uid_t uid = readInt32(); setDataPosition(initialPosition); return uid; @@ -740,7 +792,7 @@ bool Parcel::enforceInterface(const char16_t* interface, size_t len, IPCThreadState* threadState) const { - if (CC_LIKELY(!isForRpc())) { + if (auto* kernelFields = maybeKernelFields()) { // StrictModePolicy. int32_t strictPolicy = readInt32(); if (threadState == nullptr) { @@ -795,7 +847,10 @@ binder::Status Parcel::enforceNoDataAvail() const { size_t Parcel::objectsCount() const { - return mObjectsSize; + if (const auto* kernelFields = maybeKernelFields()) { + return kernelFields->mObjectsSize; + } + return 0; } status_t Parcel::errorCheck() const @@ -1401,8 +1456,11 @@ status_t Parcel::write(const FlattenableHelperInterface& val) status_t Parcel::writeObject(const flat_binder_object& val, bool nullMetaData) { + auto* kernelFields = maybeKernelFields(); + LOG_ALWAYS_FATAL_IF(kernelFields == nullptr, "Can't write flat_binder_object to RPC Parcel"); + const bool enoughData = (mDataPos+sizeof(val)) <= mDataCapacity; - const bool enoughObjects = mObjectsSize < mObjectsCapacity; + const bool enoughObjects = kernelFields->mObjectsSize < kernelFields->mObjectsCapacity; if (enoughData && enoughObjects) { restart_write: *reinterpret_cast<flat_binder_object*>(mData+mDataPos) = val; @@ -1413,14 +1471,14 @@ restart_write: // fail before modifying our object index return FDS_NOT_ALLOWED; } - mHasFds = mFdsKnown = true; + kernelFields->mHasFds = kernelFields->mFdsKnown = true; } // Need to write meta-data? if (nullMetaData || val.binder != 0) { - mObjects[mObjectsSize] = mDataPos; + kernelFields->mObjects[kernelFields->mObjectsSize] = mDataPos; acquire_object(ProcessState::self(), val, this); - mObjectsSize++; + kernelFields->mObjectsSize++; } return finishWrite(sizeof(flat_binder_object)); @@ -1431,14 +1489,15 @@ restart_write: if (err != NO_ERROR) return err; } if (!enoughObjects) { - if (mObjectsSize > SIZE_MAX - 2) return NO_MEMORY; // overflow - if ((mObjectsSize + 2) > SIZE_MAX / 3) return NO_MEMORY; // overflow - size_t newSize = ((mObjectsSize+2)*3)/2; + if (kernelFields->mObjectsSize > SIZE_MAX - 2) return NO_MEMORY; // overflow + if ((kernelFields->mObjectsSize + 2) > SIZE_MAX / 3) return NO_MEMORY; // overflow + size_t newSize = ((kernelFields->mObjectsSize + 2) * 3) / 2; if (newSize > SIZE_MAX / sizeof(binder_size_t)) return NO_MEMORY; // overflow - binder_size_t* objects = (binder_size_t*)realloc(mObjects, newSize*sizeof(binder_size_t)); + binder_size_t* objects = + (binder_size_t*)realloc(kernelFields->mObjects, newSize * sizeof(binder_size_t)); if (objects == nullptr) return NO_MEMORY; - mObjects = objects; - mObjectsCapacity = newSize; + kernelFields->mObjects = objects; + kernelFields->mObjectsCapacity = newSize; } goto restart_write; @@ -1452,54 +1511,64 @@ status_t Parcel::writeNoException() status_t Parcel::validateReadData(size_t upperBound) const { + const auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + // Can't validate RPC Parcel reads because the location of binder + // objects is unknown. + return OK; + } + // Don't allow non-object reads on object data - if (mObjectsSorted || mObjectsSize <= 1) { -data_sorted: + if (kernelFields->mObjectsSorted || kernelFields->mObjectsSize <= 1) { + data_sorted: // Expect to check only against the next object - if (mNextObjectHint < mObjectsSize && upperBound > mObjects[mNextObjectHint]) { + if (kernelFields->mNextObjectHint < kernelFields->mObjectsSize && + upperBound > kernelFields->mObjects[kernelFields->mNextObjectHint]) { // For some reason the current read position is greater than the next object // hint. Iterate until we find the right object - size_t nextObject = mNextObjectHint; + size_t nextObject = kernelFields->mNextObjectHint; do { - if (mDataPos < mObjects[nextObject] + sizeof(flat_binder_object)) { + if (mDataPos < kernelFields->mObjects[nextObject] + sizeof(flat_binder_object)) { // Requested info overlaps with an object ALOGE("Attempt to read from protected data in Parcel %p", this); return PERMISSION_DENIED; } nextObject++; - } while (nextObject < mObjectsSize && upperBound > mObjects[nextObject]); - mNextObjectHint = nextObject; + } while (nextObject < kernelFields->mObjectsSize && + upperBound > kernelFields->mObjects[nextObject]); + kernelFields->mNextObjectHint = nextObject; } return NO_ERROR; } // Quickly determine if mObjects is sorted. - binder_size_t* currObj = mObjects + mObjectsSize - 1; + binder_size_t* currObj = kernelFields->mObjects + kernelFields->mObjectsSize - 1; binder_size_t* prevObj = currObj; - while (currObj > mObjects) { + while (currObj > kernelFields->mObjects) { prevObj--; if(*prevObj > *currObj) { goto data_unsorted; } currObj--; } - mObjectsSorted = true; + kernelFields->mObjectsSorted = true; goto data_sorted; data_unsorted: // Insertion Sort mObjects // Great for mostly sorted lists. If randomly sorted or reverse ordered mObjects become common, // switch to std::sort(mObjects, mObjects + mObjectsSize); - for (binder_size_t* iter0 = mObjects + 1; iter0 < mObjects + mObjectsSize; iter0++) { + for (binder_size_t* iter0 = kernelFields->mObjects + 1; + iter0 < kernelFields->mObjects + kernelFields->mObjectsSize; iter0++) { binder_size_t temp = *iter0; binder_size_t* iter1 = iter0 - 1; - while (iter1 >= mObjects && *iter1 > temp) { + while (iter1 >= kernelFields->mObjects && *iter1 > temp) { *(iter1 + 1) = *iter1; iter1--; } *(iter1 + 1) = temp; } - mNextObjectHint = 0; - mObjectsSorted = true; + kernelFields->mNextObjectHint = 0; + kernelFields->mObjectsSorted = true; goto data_sorted; } @@ -1513,7 +1582,8 @@ status_t Parcel::read(void* outData, size_t len) const if ((mDataPos+pad_size(len)) >= mDataPos && (mDataPos+pad_size(len)) <= mDataSize && len <= pad_size(len)) { - if (mObjectsSize > 0) { + const auto* kernelFields = maybeKernelFields(); + if (kernelFields != nullptr && kernelFields->mObjectsSize > 0) { status_t err = validateReadData(mDataPos + pad_size(len)); if(err != NO_ERROR) { // Still increment the data position by the expected length @@ -1540,7 +1610,8 @@ const void* Parcel::readInplace(size_t len) const if ((mDataPos+pad_size(len)) >= mDataPos && (mDataPos+pad_size(len)) <= mDataSize && len <= pad_size(len)) { - if (mObjectsSize > 0) { + const auto* kernelFields = maybeKernelFields(); + if (kernelFields != nullptr && kernelFields->mObjectsSize > 0) { status_t err = validateReadData(mDataPos + pad_size(len)); if(err != NO_ERROR) { // Still increment the data position by the expected length @@ -1587,7 +1658,8 @@ status_t Parcel::readAligned(T *pArg) const { static_assert(std::is_trivially_copyable_v<T>); if ((mDataPos+sizeof(T)) <= mDataSize) { - if (mObjectsSize > 0) { + const auto* kernelFields = maybeKernelFields(); + if (kernelFields != nullptr && kernelFields->mObjectsSize > 0) { status_t err = validateReadData(mDataPos + sizeof(T)); if(err != NO_ERROR) { // Still increment the data position by the expected length @@ -2132,6 +2204,11 @@ status_t Parcel::read(FlattenableHelperInterface& val) const } const flat_binder_object* Parcel::readObject(bool nullMetaData) const { + const auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return nullptr; + } + const size_t DPOS = mDataPos; if ((DPOS+sizeof(flat_binder_object)) <= mDataSize) { const flat_binder_object* obj @@ -2146,9 +2223,9 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const } // Ensure that this object is valid... - binder_size_t* const OBJS = mObjects; - const size_t N = mObjectsSize; - size_t opos = mNextObjectHint; + binder_size_t* const OBJS = kernelFields->mObjects; + const size_t N = kernelFields->mObjectsSize; + size_t opos = kernelFields->mNextObjectHint; if (N > 0) { ALOGV("Parcel %p looking for obj at %zu, hint=%zu", @@ -2167,7 +2244,7 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const // Found it! ALOGV("Parcel %p found obj %zu at index %zu with forward search", this, DPOS, opos); - mNextObjectHint = opos+1; + kernelFields->mNextObjectHint = opos + 1; ALOGV("readObject Setting data pos of %p to %zu", this, mDataPos); return obj; } @@ -2180,7 +2257,7 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const // Found it! ALOGV("Parcel %p found obj %zu at index %zu with backward search", this, DPOS, opos); - mNextObjectHint = opos+1; + kernelFields->mNextObjectHint = opos + 1; ALOGV("readObject Setting data pos of %p to %zu", this, mDataPos); return obj; } @@ -2191,16 +2268,19 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const return nullptr; } -void Parcel::closeFileDescriptors() -{ - size_t i = mObjectsSize; +void Parcel::closeFileDescriptors() { + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return; + } + size_t i = kernelFields->mObjectsSize; if (i > 0) { //ALOGI("Closing file descriptors for %zu objects...", i); } while (i > 0) { i--; - const flat_binder_object* flat - = reinterpret_cast<flat_binder_object*>(mData+mObjects[i]); + const flat_binder_object* flat = + reinterpret_cast<flat_binder_object*>(mData + kernelFields->mObjects[i]); if (flat->hdr.type == BINDER_TYPE_FD) { //ALOGI("Closing fd: %ld", flat->handle); close(flat->handle); @@ -2220,35 +2300,43 @@ size_t Parcel::ipcDataSize() const uintptr_t Parcel::ipcObjects() const { - return reinterpret_cast<uintptr_t>(mObjects); + if (const auto* kernelFields = maybeKernelFields()) { + return reinterpret_cast<uintptr_t>(kernelFields->mObjects); + } + return 0; } size_t Parcel::ipcObjectsCount() const { - return mObjectsSize; + if (const auto* kernelFields = maybeKernelFields()) { + return kernelFields->mObjectsSize; + } + return 0; } -void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, - const binder_size_t* objects, size_t objectsCount, release_func relFunc) -{ +void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, const binder_size_t* objects, + size_t objectsCount, release_func relFunc) { // this code uses 'mOwner == nullptr' to understand whether it owns memory LOG_ALWAYS_FATAL_IF(relFunc == nullptr, "must provide cleanup function"); freeData(); + auto* kernelFields = maybeKernelFields(); + LOG_ALWAYS_FATAL_IF(kernelFields == nullptr); // guaranteed by freeData. + mData = const_cast<uint8_t*>(data); mDataSize = mDataCapacity = dataSize; - mObjects = const_cast<binder_size_t*>(objects); - mObjectsSize = mObjectsCapacity = objectsCount; + kernelFields->mObjects = const_cast<binder_size_t*>(objects); + kernelFields->mObjectsSize = kernelFields->mObjectsCapacity = objectsCount; mOwner = relFunc; binder_size_t minOffset = 0; - for (size_t i = 0; i < mObjectsSize; i++) { - binder_size_t offset = mObjects[i]; + for (size_t i = 0; i < kernelFields->mObjectsSize; i++) { + binder_size_t offset = kernelFields->mObjects[i]; if (offset < minOffset) { ALOGE("%s: bad object offset %" PRIu64 " < %" PRIu64 "\n", __func__, (uint64_t)offset, (uint64_t)minOffset); - mObjectsSize = 0; + kernelFields->mObjectsSize = 0; break; } const flat_binder_object* flat @@ -2266,7 +2354,7 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, // WARNING: callers of ipcSetDataReference need to make sure they // don't rely on mObjectsSize in their release_func. - mObjectsSize = 0; + kernelFields->mObjectsSize = 0; break; } minOffset = offset + sizeof(flat_binder_object); @@ -2274,6 +2362,21 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, scanForFds(); } +void Parcel::rpcSetDataReference(const sp<RpcSession>& session, const uint8_t* data, + size_t dataSize, release_func relFunc) { + // this code uses 'mOwner == nullptr' to understand whether it owns memory + LOG_ALWAYS_FATAL_IF(relFunc == nullptr, "must provide cleanup function"); + + LOG_ALWAYS_FATAL_IF(session == nullptr); + + freeData(); + markForRpc(session); + + mData = const_cast<uint8_t*>(data); + mDataSize = mDataCapacity = dataSize; + mOwner = relFunc; +} + void Parcel::print(TextOutput& to, uint32_t /*flags*/) const { to << "Parcel("; @@ -2284,14 +2387,16 @@ void Parcel::print(TextOutput& to, uint32_t /*flags*/) const } else if (dataSize() > 0) { const uint8_t* DATA = data(); to << indent << HexDump(DATA, dataSize()) << dedent; - const binder_size_t* OBJS = mObjects; - const size_t N = objectsCount(); - for (size_t i=0; i<N; i++) { - const flat_binder_object* flat - = reinterpret_cast<const flat_binder_object*>(DATA+OBJS[i]); - to << endl << "Object #" << i << " @ " << (void*)OBJS[i] << ": " - << TypeCode(flat->hdr.type & 0x7f7f7f00) - << " = " << flat->binder; + if (const auto* kernelFields = maybeKernelFields()) { + const binder_size_t* OBJS = kernelFields->mObjects; + const size_t N = objectsCount(); + for (size_t i = 0; i < N; i++) { + const flat_binder_object* flat = + reinterpret_cast<const flat_binder_object*>(DATA + OBJS[i]); + to << endl + << "Object #" << i << " @ " << (void*)OBJS[i] << ": " + << TypeCode(flat->hdr.type & 0x7f7f7f00) << " = " << flat->binder; + } } } else { to << "NULL"; @@ -2302,34 +2407,42 @@ void Parcel::print(TextOutput& to, uint32_t /*flags*/) const void Parcel::releaseObjects() { - size_t i = mObjectsSize; + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return; + } + + size_t i = kernelFields->mObjectsSize; if (i == 0) { return; } sp<ProcessState> proc(ProcessState::self()); uint8_t* const data = mData; - binder_size_t* const objects = mObjects; + binder_size_t* const objects = kernelFields->mObjects; while (i > 0) { i--; - const flat_binder_object* flat - = reinterpret_cast<flat_binder_object*>(data+objects[i]); + const flat_binder_object* flat = reinterpret_cast<flat_binder_object*>(data + objects[i]); release_object(proc, *flat, this); } } void Parcel::acquireObjects() { - size_t i = mObjectsSize; + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return; + } + + size_t i = kernelFields->mObjectsSize; if (i == 0) { return; } const sp<ProcessState> proc(ProcessState::self()); uint8_t* const data = mData; - binder_size_t* const objects = mObjects; + binder_size_t* const objects = kernelFields->mObjects; while (i > 0) { i--; - const flat_binder_object* flat - = reinterpret_cast<flat_binder_object*>(data+objects[i]); + const flat_binder_object* flat = reinterpret_cast<flat_binder_object*>(data + objects[i]); acquire_object(proc, *flat, this); } } @@ -2345,7 +2458,9 @@ void Parcel::freeDataNoInit() if (mOwner) { LOG_ALLOC("Parcel %p: freeing other owner data", this); //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid()); - mOwner(this, mData, mDataSize, mObjects, mObjectsSize); + auto* kernelFields = maybeKernelFields(); + mOwner(this, mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr, + kernelFields ? kernelFields->mObjectsSize : 0); } else { LOG_ALLOC("Parcel %p: freeing allocated data", this); releaseObjects(); @@ -2358,7 +2473,8 @@ void Parcel::freeDataNoInit() } free(mData); } - if (mObjects) free(mObjects); + auto* kernelFields = maybeKernelFields(); + if (kernelFields && kernelFields->mObjects) free(kernelFields->mObjects); } } @@ -2433,13 +2549,15 @@ status_t Parcel::restartWrite(size_t desired) ALOGV("restartWrite Setting data size of %p to %zu", this, mDataSize); ALOGV("restartWrite Setting data pos of %p to %zu", this, mDataPos); - free(mObjects); - mObjects = nullptr; - mObjectsSize = mObjectsCapacity = 0; - mNextObjectHint = 0; - mObjectsSorted = false; - mHasFds = false; - mFdsKnown = true; + if (auto* kernelFields = maybeKernelFields()) { + free(kernelFields->mObjects); + kernelFields->mObjects = nullptr; + kernelFields->mObjectsSize = kernelFields->mObjectsCapacity = 0; + kernelFields->mNextObjectHint = 0; + kernelFields->mObjectsSorted = false; + kernelFields->mHasFds = false; + kernelFields->mFdsKnown = true; + } mAllowFds = true; return NO_ERROR; @@ -2453,16 +2571,17 @@ status_t Parcel::continueWrite(size_t desired) return BAD_VALUE; } + auto* kernelFields = maybeKernelFields(); + // If shrinking, first adjust for any objects that appear // after the new data size. - size_t objectsSize = mObjectsSize; - if (desired < mDataSize) { + size_t objectsSize = kernelFields ? kernelFields->mObjectsSize : 0; + if (kernelFields && desired < mDataSize) { if (desired == 0) { objectsSize = 0; } else { while (objectsSize > 0) { - if (mObjects[objectsSize-1] < desired) - break; + if (kernelFields->mObjects[objectsSize - 1] < desired) break; objectsSize--; } } @@ -2495,20 +2614,21 @@ status_t Parcel::continueWrite(size_t desired) // Little hack to only acquire references on objects // we will be keeping. - size_t oldObjectsSize = mObjectsSize; - mObjectsSize = objectsSize; + size_t oldObjectsSize = kernelFields->mObjectsSize; + kernelFields->mObjectsSize = objectsSize; acquireObjects(); - mObjectsSize = oldObjectsSize; + kernelFields->mObjectsSize = oldObjectsSize; } if (mData) { memcpy(data, mData, mDataSize < desired ? mDataSize : desired); } - if (objects && mObjects) { - memcpy(objects, mObjects, objectsSize*sizeof(binder_size_t)); + if (objects && kernelFields && kernelFields->mObjects) { + memcpy(objects, kernelFields->mObjects, objectsSize * sizeof(binder_size_t)); } //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid()); - mOwner(this, mData, mDataSize, mObjects, mObjectsSize); + mOwner(this, mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr, + kernelFields ? kernelFields->mObjectsSize : 0); mOwner = nullptr; LOG_ALLOC("Parcel %p: taking ownership of %zu capacity", this, desired); @@ -2516,43 +2636,46 @@ status_t Parcel::continueWrite(size_t desired) gParcelGlobalAllocCount++; mData = data; - mObjects = objects; mDataSize = (mDataSize < desired) ? mDataSize : desired; ALOGV("continueWrite Setting data size of %p to %zu", this, mDataSize); mDataCapacity = desired; - mObjectsSize = mObjectsCapacity = objectsSize; - mNextObjectHint = 0; - mObjectsSorted = false; + if (kernelFields) { + kernelFields->mObjects = objects; + kernelFields->mObjectsSize = kernelFields->mObjectsCapacity = objectsSize; + kernelFields->mNextObjectHint = 0; + kernelFields->mObjectsSorted = false; + } } else if (mData) { - if (objectsSize < mObjectsSize) { + if (kernelFields && objectsSize < kernelFields->mObjectsSize) { // Need to release refs on any objects we are dropping. const sp<ProcessState> proc(ProcessState::self()); - for (size_t i=objectsSize; i<mObjectsSize; i++) { - const flat_binder_object* flat - = reinterpret_cast<flat_binder_object*>(mData+mObjects[i]); + for (size_t i = objectsSize; i < kernelFields->mObjectsSize; i++) { + const flat_binder_object* flat = + reinterpret_cast<flat_binder_object*>(mData + kernelFields->mObjects[i]); if (flat->hdr.type == BINDER_TYPE_FD) { // will need to rescan because we may have lopped off the only FDs - mFdsKnown = false; + kernelFields->mFdsKnown = false; } release_object(proc, *flat, this); } if (objectsSize == 0) { - free(mObjects); - mObjects = nullptr; - mObjectsCapacity = 0; + free(kernelFields->mObjects); + kernelFields->mObjects = nullptr; + kernelFields->mObjectsCapacity = 0; } else { binder_size_t* objects = - (binder_size_t*)realloc(mObjects, objectsSize*sizeof(binder_size_t)); + (binder_size_t*)realloc(kernelFields->mObjects, + objectsSize * sizeof(binder_size_t)); if (objects) { - mObjects = objects; - mObjectsCapacity = objectsSize; + kernelFields->mObjects = objects; + kernelFields->mObjectsCapacity = objectsSize; } } - mObjectsSize = objectsSize; - mNextObjectHint = 0; - mObjectsSorted = false; + kernelFields->mObjectsSize = objectsSize; + kernelFields->mNextObjectHint = 0; + kernelFields->mObjectsSorted = false; } // We own the data, so we can just do a realloc(). @@ -2588,9 +2711,12 @@ status_t Parcel::continueWrite(size_t desired) return NO_MEMORY; } - if(!(mDataCapacity == 0 && mObjects == nullptr - && mObjectsCapacity == 0)) { - ALOGE("continueWrite: %zu/%p/%zu/%zu", mDataCapacity, mObjects, mObjectsCapacity, desired); + if (!(mDataCapacity == 0 && + (kernelFields == nullptr || + (kernelFields->mObjects == nullptr && kernelFields->mObjectsCapacity == 0)))) { + ALOGE("continueWrite: %zu/%p/%zu/%zu", mDataCapacity, + kernelFields ? kernelFields->mObjects : nullptr, + kernelFields ? kernelFields->mObjectsCapacity : 0, desired); } LOG_ALLOC("Parcel %p: allocating with %zu capacity", this, desired); @@ -2617,19 +2743,10 @@ void Parcel::initState() mDataPos = 0; ALOGV("initState Setting data size of %p to %zu", this, mDataSize); ALOGV("initState Setting data pos of %p to %zu", this, mDataPos); - mSession = nullptr; - mObjects = nullptr; - mObjectsSize = 0; - mObjectsCapacity = 0; - mNextObjectHint = 0; - mObjectsSorted = false; - mHasFds = false; - mFdsKnown = true; + mVariantFields.emplace<KernelFields>(); mAllowFds = true; mDeallocZero = false; mOwner = nullptr; - mWorkSourceRequestHeaderPosition = 0; - mRequestHeaderPresent = false; // racing multiple init leads only to multiple identical write if (gMaxFds == 0) { @@ -2645,9 +2762,13 @@ void Parcel::initState() } void Parcel::scanForFds() const { - status_t status = hasFileDescriptorsInRange(0, dataSize(), &mHasFds); + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return; + } + status_t status = hasFileDescriptorsInRange(0, dataSize(), &kernelFields->mHasFds); ALOGE_IF(status != NO_ERROR, "Error %d calling hasFileDescriptorsInRange()", status); - mFdsKnown = true; + kernelFields->mFdsKnown = true; } size_t Parcel::getBlobAshmemSize() const @@ -2660,10 +2781,15 @@ size_t Parcel::getBlobAshmemSize() const size_t Parcel::getOpenAshmemSize() const { + auto* kernelFields = maybeKernelFields(); + if (kernelFields == nullptr) { + return 0; + } + size_t openAshmemSize = 0; - for (size_t i = 0; i < mObjectsSize; i++) { + for (size_t i = 0; i < kernelFields->mObjectsSize; i++) { const flat_binder_object* flat = - reinterpret_cast<const flat_binder_object*>(mData + mObjects[i]); + reinterpret_cast<const flat_binder_object*>(mData + kernelFields->mObjects[i]); // cookie is compared against zero for historical reasons // > obj.cookie = takeOwnership ? 1 : 0; diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp index 4a01d8176d..7faff47627 100644 --- a/libs/binder/ProcessState.cpp +++ b/libs/binder/ProcessState.cpp @@ -35,14 +35,15 @@ #include <errno.h> #include <fcntl.h> -#include <mutex> +#include <pthread.h> #include <stdio.h> #include <stdlib.h> -#include <unistd.h> #include <sys/ioctl.h> #include <sys/mman.h> #include <sys/stat.h> #include <sys/types.h> +#include <unistd.h> +#include <mutex> #define BINDER_VM_SIZE ((1 * 1024 * 1024) - sysconf(_SC_PAGE_SIZE) * 2) #define DEFAULT_MAX_BINDER_THREADS 15 @@ -100,6 +101,11 @@ static void verifyNotForked(bool forked) { sp<ProcessState> ProcessState::init(const char *driver, bool requireDefault) { +#ifdef BINDER_IPC_32BIT + LOG_ALWAYS_FATAL("32-bit binder IPC is not supported for new devices starting in Android P. If " + "you do need to use this mode, please see b/232423610 or file an issue with " + "AOSP upstream as otherwise this will be removed soon."); +#endif if (driver == nullptr) { std::lock_guard<std::mutex> l(gProcessMutex); @@ -170,6 +176,10 @@ void ProcessState::childPostFork() { // the thread handler is installed if (gProcess) { gProcess->mForked = true; + + // "O_CLOFORK" + close(gProcess->mDriverFD); + gProcess->mDriverFD = -1; } gProcessMutex.unlock(); } @@ -182,7 +192,6 @@ void ProcessState::startThreadPool() ALOGW("Extra binder thread started, but 0 threads requested. Do not use " "*startThreadPool when zero threads are requested."); } - mThreadPoolStarted = true; spawnPooledThread(true); } @@ -290,12 +299,17 @@ ProcessState::handle_entry* ProcessState::lookupHandleLocked(int32_t handle) return &mHandleToObject.editItemAt(handle); } +// see b/166779391: cannot change the VNDK interface, so access like this +extern sp<BBinder> the_context_object; + sp<IBinder> ProcessState::getStrongProxyForHandle(int32_t handle) { sp<IBinder> result; AutoMutex _l(mLock); + if (handle == 0 && the_context_object != nullptr) return the_context_object; + handle_entry* e = lookupHandleLocked(handle); if (e != nullptr) { @@ -386,6 +400,9 @@ void ProcessState::spawnPooledThread(bool isMain) ALOGV("Spawning new pooled thread, name=%s\n", name.string()); sp<Thread> t = sp<PoolThread>::make(isMain); t->run(name.string()); + pthread_mutex_lock(&mThreadCountLock); + mKernelStartedThreads++; + pthread_mutex_unlock(&mThreadCountLock); } } @@ -402,12 +419,20 @@ status_t ProcessState::setThreadPoolMaxThreadCount(size_t maxThreads) { return result; } -size_t ProcessState::getThreadPoolMaxThreadCount() const { +size_t ProcessState::getThreadPoolMaxTotalThreadCount() const { // may actually be one more than this, if join is called - if (mThreadPoolStarted) return mMaxThreads; + if (mThreadPoolStarted) { + return mCurrentThreads < mKernelStartedThreads + ? mMaxThreads + : mMaxThreads + mCurrentThreads - mKernelStartedThreads; + } // must not be initialized or maybe has poll thread setup, we // currently don't track this in libbinder - return 0; + LOG_ALWAYS_FATAL_IF(mKernelStartedThreads != 0, + "Expecting 0 kernel started threads but have" + " %zu", + mKernelStartedThreads); + return mCurrentThreads; } #define DRIVER_FEATURES_PATH "/dev/binderfs/features/" @@ -415,6 +440,8 @@ bool ProcessState::isDriverFeatureEnabled(const DriverFeature feature) { static const char* const names[] = { [static_cast<int>(DriverFeature::ONEWAY_SPAM_DETECTION)] = DRIVER_FEATURES_PATH "oneway_spam_detection", + [static_cast<int>(DriverFeature::EXTENDED_ERROR)] = + DRIVER_FEATURES_PATH "extended_error", }; int fd = open(names[static_cast<int>(feature)], O_RDONLY | O_CLOEXEC); char on; @@ -491,6 +518,8 @@ ProcessState::ProcessState(const char* driver) mExecutingThreadsCount(0), mWaitingForThreads(0), mMaxThreads(DEFAULT_MAX_BINDER_THREADS), + mCurrentThreads(0), + mKernelStartedThreads(0), mStarvationStartTimeMs(0), mForked(false), mThreadPoolStarted(false), diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp index ace5cd5052..c67b70abc0 100644 --- a/libs/binder/RpcServer.cpp +++ b/libs/binder/RpcServer.cpp @@ -24,18 +24,19 @@ #include <thread> #include <vector> -#include <android-base/file.h> #include <android-base/hex.h> #include <android-base/scopeguard.h> #include <binder/Parcel.h> #include <binder/RpcServer.h> #include <binder/RpcTransportRaw.h> #include <log/log.h> +#include <utils/Compat.h> #include "FdTrigger.h" #include "RpcSocketAddress.h" #include "RpcState.h" #include "RpcWireFormat.h" +#include "Utils.h" namespace android { @@ -288,7 +289,8 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie RpcConnectionHeader header; if (status == OK) { iovec iov{&header, sizeof(header)}; - status = client->interruptableReadFully(server->mShutdownTrigger.get(), &iov, 1, {}); + status = client->interruptableReadFully(server->mShutdownTrigger.get(), &iov, 1, + std::nullopt); if (status != OK) { ALOGE("Failed to read ID for client connecting to RPC server: %s", statusToString(status).c_str()); @@ -302,8 +304,8 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie if (header.sessionIdSize == kSessionIdBytes) { sessionId.resize(header.sessionIdSize); iovec iov{sessionId.data(), sessionId.size()}; - status = - client->interruptableReadFully(server->mShutdownTrigger.get(), &iov, 1, {}); + status = client->interruptableReadFully(server->mShutdownTrigger.get(), &iov, 1, + std::nullopt); if (status != OK) { ALOGE("Failed to read session ID for client connecting to RPC server: %s", statusToString(status).c_str()); @@ -333,7 +335,8 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie }; iovec iov{&response, sizeof(response)}; - status = client->interruptableWriteFully(server->mShutdownTrigger.get(), &iov, 1, {}); + status = client->interruptableWriteFully(server->mShutdownTrigger.get(), &iov, 1, + std::nullopt); if (status != OK) { ALOGE("Failed to send new session response: %s", statusToString(status).c_str()); // still need to cleanup before we can return @@ -380,10 +383,9 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie return; } - base::unique_fd fd(TEMP_FAILURE_RETRY( - open("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOFOLLOW))); - if (!base::ReadFully(fd, sessionId.data(), sessionId.size())) { - ALOGE("Could not read from /dev/urandom to create session ID"); + auto status = getRandomBytes(sessionId.data(), sessionId.size()); + if (status != OK) { + ALOGE("Failed to read random session ID: %s", strerror(-status)); return; } } while (server->mSessions.end() != server->mSessions.find(sessionId)); diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp index d40778a3d8..5c35dd0ac1 100644 --- a/libs/binder/RpcSession.cpp +++ b/libs/binder/RpcSession.cpp @@ -34,6 +34,7 @@ #include <binder/RpcServer.h> #include <binder/RpcTransportRaw.h> #include <binder/Stability.h> +#include <utils/Compat.h> #include <utils/String8.h> #include "FdTrigger.h" @@ -42,11 +43,7 @@ #include "RpcWireFormat.h" #include "Utils.h" -#ifdef __GLIBC__ -extern "C" pid_t gettid(); -#endif - -#ifndef __ANDROID_RECOVERY__ +#if defined(__ANDROID__) && !defined(__ANDROID_RECOVERY__) #include <android_runtime/vm.h> #include <jni.h> #endif @@ -152,13 +149,7 @@ status_t RpcSession::setupInetClient(const char* addr, unsigned int port) { } status_t RpcSession::setupPreconnectedClient(unique_fd fd, std::function<unique_fd()>&& request) { - // Why passing raw fd? When fd is passed as reference, Clang analyzer sees that the variable - // `fd` is a moved-from object. To work-around the issue, unwrap the raw fd from the outer `fd`, - // pass the raw fd by value to the lambda, and then finally wrap it in unique_fd inside the - // lambda. - return setupClient([&, raw = fd.release()](const std::vector<uint8_t>& sessionId, - bool incoming) -> status_t { - unique_fd fd(raw); + return setupClient([&](const std::vector<uint8_t>& sessionId, bool incoming) -> status_t { if (!fd.ok()) { fd = request(); if (!fd.ok()) return BAD_VALUE; @@ -167,7 +158,9 @@ status_t RpcSession::setupPreconnectedClient(unique_fd fd, std::function<unique_ ALOGE("setupPreconnectedClient: %s", res.error().message().c_str()); return res.error().code() == 0 ? UNKNOWN_ERROR : -res.error().code(); } - return initAndAddConnection(std::move(fd), sessionId, incoming); + status_t status = initAndAddConnection(std::move(fd), sessionId, incoming); + fd = unique_fd(); // Explicitly reset after move to avoid analyzer warning. + return status; }); } @@ -323,7 +316,7 @@ RpcSession::PreJoinSetupResult RpcSession::preJoinSetup( } namespace { -#ifdef __ANDROID_RECOVERY__ +#if !defined(__ANDROID__) || defined(__ANDROID_RECOVERY__) class JavaThreadAttacher {}; #else // RAII object for attaching / detaching current thread to JVM if Android Runtime exists. If @@ -622,7 +615,7 @@ status_t RpcSession::initAndAddConnection(unique_fd fd, const std::vector<uint8_ iovec headerIov{&header, sizeof(header)}; auto sendHeaderStatus = - server->interruptableWriteFully(mShutdownTrigger.get(), &headerIov, 1, {}); + server->interruptableWriteFully(mShutdownTrigger.get(), &headerIov, 1, std::nullopt); if (sendHeaderStatus != OK) { ALOGE("Could not write connection header to socket: %s", statusToString(sendHeaderStatus).c_str()); @@ -632,8 +625,8 @@ status_t RpcSession::initAndAddConnection(unique_fd fd, const std::vector<uint8_ if (sessionId.size() > 0) { iovec sessionIov{const_cast<void*>(static_cast<const void*>(sessionId.data())), sessionId.size()}; - auto sendSessionIdStatus = - server->interruptableWriteFully(mShutdownTrigger.get(), &sessionIov, 1, {}); + auto sendSessionIdStatus = server->interruptableWriteFully(mShutdownTrigger.get(), + &sessionIov, 1, std::nullopt); if (sendSessionIdStatus != OK) { ALOGE("Could not write session ID ('%s') to socket: %s", base::HexString(sessionId.data(), sessionId.size()).c_str(), @@ -696,7 +689,7 @@ status_t RpcSession::addOutgoingConnection(std::unique_ptr<RpcTransport> rpcTran { std::lock_guard<std::mutex> _l(mMutex); connection->rpcTransport = std::move(rpcTransport); - connection->exclusiveTid = gettid(); + connection->exclusiveTid = base::GetThreadId(); mConnections.mOutgoing.push_back(connection); } @@ -753,7 +746,7 @@ sp<RpcSession::RpcConnection> RpcSession::assignIncomingConnectionToThisThread( sp<RpcConnection> session = sp<RpcConnection>::make(); session->rpcTransport = std::move(rpcTransport); - session->exclusiveTid = gettid(); + session->exclusiveTid = base::GetThreadId(); mConnections.mIncoming.push_back(session); mConnections.mMaxIncoming = mConnections.mIncoming.size(); @@ -789,7 +782,7 @@ status_t RpcSession::ExclusiveConnection::find(const sp<RpcSession>& session, Co connection->mConnection = nullptr; connection->mReentrant = false; - pid_t tid = gettid(); + uint64_t tid = base::GetThreadId(); std::unique_lock<std::mutex> _l(session->mMutex); session->mConnections.mWaitingThreads++; @@ -876,7 +869,7 @@ status_t RpcSession::ExclusiveConnection::find(const sp<RpcSession>& session, Co return OK; } -void RpcSession::ExclusiveConnection::findConnection(pid_t tid, sp<RpcConnection>* exclusive, +void RpcSession::ExclusiveConnection::findConnection(uint64_t tid, sp<RpcConnection>* exclusive, sp<RpcConnection>* available, std::vector<sp<RpcConnection>>& sockets, size_t socketsIndexHint) { diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp index 6d89064a5d..f16a9ab98f 100644 --- a/libs/binder/RpcState.cpp +++ b/libs/binder/RpcState.cpp @@ -311,7 +311,7 @@ RpcState::CommandData::CommandData(size_t size) : mSize(size) { status_t RpcState::rpcSend(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, const char* what, iovec* iovs, int niovs, - const std::function<status_t()>& altPoll) { + const std::optional<android::base::function_ref<status_t()>>& altPoll) { for (int i = 0; i < niovs; i++) { LOG_RPC_DETAIL("Sending %s (part %d of %d) on RpcTransport %p: %s", what, i + 1, niovs, connection->rpcTransport.get(), @@ -335,7 +335,7 @@ status_t RpcState::rpcRec(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, const char* what, iovec* iovs, int niovs) { if (status_t status = connection->rpcTransport->interruptableReadFully(session->mShutdownTrigger.get(), - iovs, niovs, {}); + iovs, niovs, std::nullopt); status != OK) { LOG_RPC_DETAIL("Failed to read %s (%d iovs) on RpcTransport %p, error: %s", what, niovs, connection->rpcTransport.get(), statusToString(status).c_str()); @@ -369,7 +369,7 @@ status_t RpcState::sendConnectionInit(const sp<RpcSession::RpcConnection>& conne .msg = RPC_CONNECTION_INIT_OKAY, }; iovec iov{&init, sizeof(init)}; - return rpcSend(connection, session, "connection init", &iov, 1); + return rpcSend(connection, session, "connection init", &iov, 1, std::nullopt); } status_t RpcState::readConnectionInit(const sp<RpcSession::RpcConnection>& connection, @@ -493,14 +493,13 @@ status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connecti } } - LOG_ALWAYS_FATAL_IF(std::numeric_limits<int32_t>::max() - sizeof(RpcWireHeader) - - sizeof(RpcWireTransaction) < - data.dataSize(), + uint32_t bodySize; + LOG_ALWAYS_FATAL_IF(__builtin_add_overflow(sizeof(RpcWireTransaction), data.dataSize(), + &bodySize), "Too much data %zu", data.dataSize()); - RpcWireHeader command{ .command = RPC_COMMAND_TRANSACT, - .bodySize = static_cast<uint32_t>(sizeof(RpcWireTransaction) + data.dataSize()), + .bodySize = bodySize, }; RpcWireTransaction transaction{ @@ -516,31 +515,32 @@ status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connecti // Oneway calls have no sync point, so if many are sent before, whether this // is a twoway or oneway transaction, they may have filled up the socket. - // So, make sure we drain them before polling. - std::function<status_t()> drainRefs = [&] { - if (waitUs > kWaitLogUs) { - ALOGE("Cannot send command, trying to process pending refcounts. Waiting %zuus. Too " - "many oneway calls?", - waitUs); - } - - if (waitUs > 0) { - usleep(waitUs); - waitUs = std::min(kWaitMaxUs, waitUs * 2); - } else { - waitUs = 1; - } - - return drainCommands(connection, session, CommandType::CONTROL_ONLY); - }; + // So, make sure we drain them before polling iovec iovs[]{ {&command, sizeof(RpcWireHeader)}, {&transaction, sizeof(RpcWireTransaction)}, {const_cast<uint8_t*>(data.data()), data.dataSize()}, }; - if (status_t status = - rpcSend(connection, session, "transaction", iovs, arraysize(iovs), drainRefs); + if (status_t status = rpcSend(connection, session, "transaction", iovs, arraysize(iovs), + [&] { + if (waitUs > kWaitLogUs) { + ALOGE("Cannot send command, trying to process pending " + "refcounts. Waiting %zuus. Too " + "many oneway calls?", + waitUs); + } + + if (waitUs > 0) { + usleep(waitUs); + waitUs = std::min(kWaitMaxUs, waitUs * 2); + } else { + waitUs = 1; + } + + return drainCommands(connection, session, + CommandType::CONTROL_ONLY); + }); status != OK) { // TODO(b/167966510): need to undo onBinderLeaving - we know the // refcount isn't successfully transferred. @@ -563,7 +563,7 @@ status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connecti static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize, const binder_size_t* objects, size_t objectsCount) { (void)p; - delete[] const_cast<uint8_t*>(data - offsetof(RpcWireReply, data)); + delete[] const_cast<uint8_t*>(data); (void)dataSize; LOG_ALWAYS_FATAL_IF(objects != nullptr); LOG_ALWAYS_FATAL_IF(objectsCount != 0, "%zu objects remaining", objectsCount); @@ -585,27 +585,30 @@ status_t RpcState::waitForReply(const sp<RpcSession::RpcConnection>& connection, return status; } - CommandData data(command.bodySize); - if (!data.valid()) return NO_MEMORY; - - iovec iov{data.data(), command.bodySize}; - if (status_t status = rpcRec(connection, session, "reply body", &iov, 1); status != OK) - return status; - if (command.bodySize < sizeof(RpcWireReply)) { ALOGE("Expecting %zu but got %" PRId32 " bytes for RpcWireReply. Terminating!", sizeof(RpcWireReply), command.bodySize); (void)session->shutdownAndWait(false); return BAD_VALUE; } - RpcWireReply* rpcReply = reinterpret_cast<RpcWireReply*>(data.data()); - if (rpcReply->status != OK) return rpcReply->status; - data.release(); - reply->ipcSetDataReference(rpcReply->data, command.bodySize - offsetof(RpcWireReply, data), - nullptr, 0, cleanup_reply_data); + RpcWireReply rpcReply; + CommandData data(command.bodySize - sizeof(RpcWireReply)); + if (!data.valid()) return NO_MEMORY; - reply->markForRpc(session); + iovec iovs[]{ + {&rpcReply, sizeof(RpcWireReply)}, + {data.data(), data.size()}, + }; + if (status_t status = rpcRec(connection, session, "reply body", iovs, arraysize(iovs)); + status != OK) + return status; + if (rpcReply.status != OK) return rpcReply.status; + + uint8_t* parcelData = data.data(); + size_t parcelDataSize = data.size(); + data.release(); + reply->rpcSetDataReference(session, parcelData, parcelDataSize, cleanup_reply_data); return OK; } @@ -643,7 +646,7 @@ status_t RpcState::sendDecStrongToTarget(const sp<RpcSession::RpcConnection>& co .bodySize = sizeof(RpcDecStrong), }; iovec iovs[]{{&cmd, sizeof(cmd)}, {&body, sizeof(body)}}; - return rpcSend(connection, session, "dec ref", iovs, arraysize(iovs)); + return rpcSend(connection, session, "dec ref", iovs, arraysize(iovs), std::nullopt); } status_t RpcState::getAndExecuteCommand(const sp<RpcSession::RpcConnection>& connection, @@ -661,13 +664,10 @@ status_t RpcState::getAndExecuteCommand(const sp<RpcSession::RpcConnection>& con status_t RpcState::drainCommands(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, CommandType type) { - uint8_t buf; while (true) { - size_t num_bytes; - status_t status = connection->rpcTransport->peek(&buf, sizeof(buf), &num_bytes); + status_t status = connection->rpcTransport->pollRead(); if (status == WOULD_BLOCK) break; if (status != OK) return status; - if (!num_bytes) break; status = getAndExecuteCommand(connection, session, type); if (status != OK) return status; @@ -828,11 +828,9 @@ processTransactInternalTailCall: // transaction->data is owned by this function. Parcel borrows this data and // only holds onto it for the duration of this function call. Parcel will be // deleted before the 'transactionData' object. - data.ipcSetDataReference(transaction->data, + data.rpcSetDataReference(session, transaction->data, transactionData.size() - offsetof(RpcWireTransaction, data), - nullptr /*object*/, 0 /*objectCount*/, do_nothing_to_transact_data); - data.markForRpc(session); if (target) { bool origAllowNested = connection->allowNested; @@ -943,14 +941,12 @@ processTransactInternalTailCall: replyStatus = flushExcessBinderRefs(session, addr, target); } - LOG_ALWAYS_FATAL_IF(std::numeric_limits<int32_t>::max() - sizeof(RpcWireHeader) - - sizeof(RpcWireReply) < - reply.dataSize(), + uint32_t bodySize; + LOG_ALWAYS_FATAL_IF(__builtin_add_overflow(sizeof(RpcWireReply), reply.dataSize(), &bodySize), "Too much data for reply %zu", reply.dataSize()); - RpcWireHeader cmdReply{ .command = RPC_COMMAND_REPLY, - .bodySize = static_cast<uint32_t>(sizeof(RpcWireReply) + reply.dataSize()), + .bodySize = bodySize, }; RpcWireReply rpcReply{ .status = replyStatus, @@ -961,7 +957,7 @@ processTransactInternalTailCall: {&rpcReply, sizeof(RpcWireReply)}, {const_cast<uint8_t*>(reply.data()), reply.dataSize()}, }; - return rpcSend(connection, session, "reply", iovs, arraysize(iovs)); + return rpcSend(connection, session, "reply", iovs, arraysize(iovs), std::nullopt); } status_t RpcState::processDecStrong(const sp<RpcSession::RpcConnection>& connection, diff --git a/libs/binder/RpcState.h b/libs/binder/RpcState.h index f4a08942b3..9cbe187444 100644 --- a/libs/binder/RpcState.h +++ b/libs/binder/RpcState.h @@ -178,9 +178,10 @@ private: size_t mSize; }; - [[nodiscard]] status_t rpcSend(const sp<RpcSession::RpcConnection>& connection, - const sp<RpcSession>& session, const char* what, iovec* iovs, - int niovs, const std::function<status_t()>& altPoll = nullptr); + [[nodiscard]] status_t rpcSend( + const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, + const char* what, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll); [[nodiscard]] status_t rpcRec(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, const char* what, iovec* iovs, int niovs); diff --git a/libs/binder/RpcTransportRaw.cpp b/libs/binder/RpcTransportRaw.cpp index 7cfc78082e..f9b73fce21 100644 --- a/libs/binder/RpcTransportRaw.cpp +++ b/libs/binder/RpcTransportRaw.cpp @@ -32,26 +32,30 @@ namespace { class RpcTransportRaw : public RpcTransport { public: explicit RpcTransportRaw(android::base::unique_fd socket) : mSocket(std::move(socket)) {} - status_t peek(void* buf, size_t size, size_t* out_size) override { - ssize_t ret = TEMP_FAILURE_RETRY(::recv(mSocket.get(), buf, size, MSG_PEEK)); + status_t pollRead(void) override { + uint8_t buf; + ssize_t ret = TEMP_FAILURE_RETRY( + ::recv(mSocket.get(), &buf, sizeof(buf), MSG_PEEK | MSG_DONTWAIT)); if (ret < 0) { int savedErrno = errno; if (savedErrno == EAGAIN || savedErrno == EWOULDBLOCK) { return WOULD_BLOCK; } - LOG_RPC_DETAIL("RpcTransport peek(): %s", strerror(savedErrno)); + LOG_RPC_DETAIL("RpcTransport poll(): %s", strerror(savedErrno)); return -savedErrno; + } else if (ret == 0) { + return DEAD_OBJECT; } - *out_size = static_cast<size_t>(ret); return OK; } template <typename SendOrReceive> - status_t interruptableReadOrWrite(FdTrigger* fdTrigger, iovec* iovs, int niovs, - SendOrReceive sendOrReceiveFun, const char* funName, - int16_t event, const std::function<status_t()>& altPoll) { + status_t interruptableReadOrWrite( + FdTrigger* fdTrigger, iovec* iovs, int niovs, SendOrReceive sendOrReceiveFun, + const char* funName, int16_t event, + const std::optional<android::base::function_ref<status_t()>>& altPoll) { MAYBE_WAIT_IN_FLAKE_MODE; if (niovs < 0) { @@ -126,7 +130,7 @@ public: } if (altPoll) { - if (status_t status = altPoll(); status != OK) return status; + if (status_t status = (*altPoll)(); status != OK) return status; if (fdTrigger->isTriggered()) { return DEAD_OBJECT; } @@ -139,14 +143,16 @@ public: } } - status_t interruptableWriteFully(FdTrigger* fdTrigger, iovec* iovs, int niovs, - const std::function<status_t()>& altPoll) override { + status_t interruptableWriteFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll) override { return interruptableReadOrWrite(fdTrigger, iovs, niovs, sendmsg, "sendmsg", POLLOUT, altPoll); } - status_t interruptableReadFully(FdTrigger* fdTrigger, iovec* iovs, int niovs, - const std::function<status_t()>& altPoll) override { + status_t interruptableReadFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll) override { return interruptableReadOrWrite(fdTrigger, iovs, niovs, recvmsg, "recvmsg", POLLIN, altPoll); } diff --git a/libs/binder/RpcTransportTls.cpp b/libs/binder/RpcTransportTls.cpp index bc68c379ce..ad5cb0fc56 100644 --- a/libs/binder/RpcTransportTls.cpp +++ b/libs/binder/RpcTransportTls.cpp @@ -181,9 +181,10 @@ public: // |sslError| should be from Ssl::getError(). // If |sslError| is WANT_READ / WANT_WRITE, poll for POLLIN / POLLOUT respectively. Otherwise // return error. Also return error if |fdTrigger| is triggered before or during poll(). - status_t pollForSslError(android::base::borrowed_fd fd, int sslError, FdTrigger* fdTrigger, - const char* fnString, int additionalEvent, - const std::function<status_t()>& altPoll) { + status_t pollForSslError( + android::base::borrowed_fd fd, int sslError, FdTrigger* fdTrigger, const char* fnString, + int additionalEvent, + const std::optional<android::base::function_ref<status_t()>>& altPoll) { switch (sslError) { case SSL_ERROR_WANT_READ: return handlePoll(POLLIN | additionalEvent, fd, fdTrigger, fnString, altPoll); @@ -198,10 +199,11 @@ private: bool mHandled = false; status_t handlePoll(int event, android::base::borrowed_fd fd, FdTrigger* fdTrigger, - const char* fnString, const std::function<status_t()>& altPoll) { + const char* fnString, + const std::optional<android::base::function_ref<status_t()>>& altPoll) { status_t ret; if (altPoll) { - ret = altPoll(); + ret = (*altPoll)(); if (fdTrigger->isTriggered()) ret = DEAD_OBJECT; } else { ret = fdTrigger->triggerablePoll(fd, event); @@ -277,11 +279,13 @@ class RpcTransportTls : public RpcTransport { public: RpcTransportTls(android::base::unique_fd socket, Ssl ssl) : mSocket(std::move(socket)), mSsl(std::move(ssl)) {} - status_t peek(void* buf, size_t size, size_t* out_size) override; - status_t interruptableWriteFully(FdTrigger* fdTrigger, iovec* iovs, int niovs, - const std::function<status_t()>& altPoll) override; - status_t interruptableReadFully(FdTrigger* fdTrigger, iovec* iovs, int niovs, - const std::function<status_t()>& altPoll) override; + status_t pollRead(void) override; + status_t interruptableWriteFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll) override; + status_t interruptableReadFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll) override; private: android::base::unique_fd mSocket; @@ -289,9 +293,9 @@ private: }; // Error code is errno. -status_t RpcTransportTls::peek(void* buf, size_t size, size_t* out_size) { - size_t todo = std::min<size_t>(size, std::numeric_limits<int>::max()); - auto [ret, errorQueue] = mSsl.call(SSL_peek, buf, static_cast<int>(todo)); +status_t RpcTransportTls::pollRead(void) { + uint8_t buf; + auto [ret, errorQueue] = mSsl.call(SSL_peek, &buf, sizeof(buf)); if (ret < 0) { int err = mSsl.getError(ret); if (err == SSL_ERROR_WANT_WRITE || err == SSL_ERROR_WANT_READ) { @@ -304,12 +308,12 @@ status_t RpcTransportTls::peek(void* buf, size_t size, size_t* out_size) { } errorQueue.clear(); LOG_TLS_DETAIL("TLS: Peeked %d bytes!", ret); - *out_size = static_cast<size_t>(ret); return OK; } -status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, iovec* iovs, int niovs, - const std::function<status_t()>& altPoll) { +status_t RpcTransportTls::interruptableWriteFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll) { MAYBE_WAIT_IN_FLAKE_MODE; if (niovs < 0) return BAD_VALUE; @@ -350,8 +354,9 @@ status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, iovec* i return OK; } -status_t RpcTransportTls::interruptableReadFully(FdTrigger* fdTrigger, iovec* iovs, int niovs, - const std::function<status_t()>& altPoll) { +status_t RpcTransportTls::interruptableReadFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll) { MAYBE_WAIT_IN_FLAKE_MODE; if (niovs < 0) return BAD_VALUE; @@ -416,8 +421,8 @@ bool setFdAndDoHandshake(Ssl* ssl, android::base::borrowed_fd fd, FdTrigger* fdT return false; } int sslError = ssl->getError(ret); - status_t pollStatus = - errorQueue.pollForSslError(fd, sslError, fdTrigger, "SSL_do_handshake", 0, {}); + status_t pollStatus = errorQueue.pollForSslError(fd, sslError, fdTrigger, + "SSL_do_handshake", 0, std::nullopt); if (pollStatus != OK) return false; } } diff --git a/libs/binder/RpcWireFormat.h b/libs/binder/RpcWireFormat.h index 171550e620..d9b5ca0dc1 100644 --- a/libs/binder/RpcWireFormat.h +++ b/libs/binder/RpcWireFormat.h @@ -139,7 +139,6 @@ static_assert(sizeof(RpcWireTransaction) == 40); struct RpcWireReply { int32_t status; // transact return - uint8_t data[]; }; static_assert(sizeof(RpcWireReply) == 4); diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING index ebb0d2731c..0232f509a2 100644 --- a/libs/binder/TEST_MAPPING +++ b/libs/binder/TEST_MAPPING @@ -83,5 +83,10 @@ { "name": "rustBinderSerializationTest" } + ], + "hwasan-presubmit": [ + { + "name": "binderLibTest" + } ] } diff --git a/libs/binder/Utils.cpp b/libs/binder/Utils.cpp index d2a5be1102..b0289a7196 100644 --- a/libs/binder/Utils.cpp +++ b/libs/binder/Utils.cpp @@ -16,6 +16,7 @@ #include "Utils.h" +#include <android-base/file.h> #include <string.h> using android::base::ErrnoError; @@ -38,4 +39,17 @@ Result<void> setNonBlocking(android::base::borrowed_fd fd) { return {}; } +status_t getRandomBytes(uint8_t* data, size_t size) { + int ret = TEMP_FAILURE_RETRY(open("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOFOLLOW)); + if (ret == -1) { + return -errno; + } + + base::unique_fd fd(ret); + if (!base::ReadFully(fd, data, size)) { + return -errno; + } + return OK; +} + } // namespace android diff --git a/libs/binder/Utils.h b/libs/binder/Utils.h index ff2fad8834..150d520492 100644 --- a/libs/binder/Utils.h +++ b/libs/binder/Utils.h @@ -20,6 +20,7 @@ #include <android-base/result.h> #include <android-base/unique_fd.h> #include <log/log.h> +#include <utils/Errors.h> #define TEST_AND_RETURN(value, expr) \ do { \ @@ -36,4 +37,6 @@ void zeroMemory(uint8_t* data, size_t size); android::base::Result<void> setNonBlocking(android::base::borrowed_fd fd); +status_t getRandomBytes(uint8_t* data, size_t size); + } // namespace android diff --git a/libs/binder/binder_module.h b/libs/binder/binder_module.h index 793795e1d4..7574c29e8b 100644 --- a/libs/binder/binder_module.h +++ b/libs/binder/binder_module.h @@ -100,4 +100,23 @@ struct binder_frozen_status_info { #define BINDER_ENABLE_ONEWAY_SPAM_DETECTION _IOW('b', 16, __u32) #endif // BINDER_ENABLE_ONEWAY_SPAM_DETECTION +#ifndef BINDER_GET_EXTENDED_ERROR +/* struct binder_extened_error - extended error information + * @id: identifier for the failed operation + * @command: command as defined by binder_driver_return_protocol + * @param: parameter holding a negative errno value + * + * Used with BINDER_GET_EXTENDED_ERROR. This extends the error information + * returned by the driver upon a failed operation. Userspace can pull this + * data to properly handle specific error scenarios. + */ +struct binder_extended_error { + __u32 id; + __u32 command; + __s32 param; +}; + +#define BINDER_GET_EXTENDED_ERROR _IOWR('b', 17, struct binder_extended_error) +#endif // BINDER_GET_EXTENDED_ERROR + #endif // _BINDER_MODULE_H_ diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h index bf02099ffb..cd6a27414d 100644 --- a/libs/binder/include/binder/IPCThreadState.h +++ b/libs/binder/include/binder/IPCThreadState.h @@ -216,6 +216,7 @@ private: static void freeBuffer(Parcel* parcel, const uint8_t* data, size_t dataSize, const binder_size_t* objects, size_t objectsSize); + static void logExtendedError(); const sp<ProcessState> mProcess; Vector<BBinder*> mPendingStrongDerefs; diff --git a/libs/binder/include/binder/MemoryHeapBase.h b/libs/binder/include/binder/MemoryHeapBase.h index 15dd28f08e..c7177bd8eb 100644 --- a/libs/binder/include/binder/MemoryHeapBase.h +++ b/libs/binder/include/binder/MemoryHeapBase.h @@ -26,9 +26,10 @@ namespace android { // --------------------------------------------------------------------------- -class MemoryHeapBase : public virtual BnMemoryHeap +class MemoryHeapBase : public BnMemoryHeap { public: + static constexpr auto MEMFD_ALLOW_SEALING_FLAG = 0x00000800; enum { READ_ONLY = IMemoryHeap::READ_ONLY, // memory won't be mapped locally, but will be mapped in the remote @@ -48,7 +49,7 @@ public: // Clients of shared files can seal at anytime via syscall, leading to // TOC/TOU issues if additional seals prevent access from the creating // process. Alternatively, seccomp fcntl(). - MEMFD_ALLOW_SEALING = 0x00000800 + MEMFD_ALLOW_SEALING = FORCE_MEMFD | MEMFD_ALLOW_SEALING_FLAG }; /* diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index e2b2c5128d..0345a5d8e1 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -20,6 +20,7 @@ #include <map> // for legacy reasons #include <string> #include <type_traits> +#include <variant> #include <vector> #include <android-base/unique_fd.h> @@ -76,6 +77,11 @@ public: size_t dataCapacity() const; status_t setDataSize(size_t size); + + // this must only be used to set a data position that was previously returned from + // dataPosition(). If writes are made, the exact same types of writes must be made (e.g. + // auto i = p.dataPosition(); p.writeInt32(0); p.setDataPosition(i); p.writeInt32(1);). + // Writing over objects, such as file descriptors and binders, is not supported. void setDataPosition(size_t pos) const; status_t setDataCapacity(size_t size); @@ -600,15 +606,19 @@ private: size_t ipcDataSize() const; uintptr_t ipcObjects() const; size_t ipcObjectsCount() const; - void ipcSetDataReference(const uint8_t* data, size_t dataSize, - const binder_size_t* objects, size_t objectsCount, - release_func relFunc); + void ipcSetDataReference(const uint8_t* data, size_t dataSize, const binder_size_t* objects, + size_t objectsCount, release_func relFunc); + void rpcSetDataReference(const sp<RpcSession>& session, const uint8_t* data, size_t dataSize, + release_func relFunc); status_t finishWrite(size_t len); void releaseObjects(); void acquireObjects(); status_t growData(size_t len); + // Clear the Parcel and set the capacity to `desired`. + // Doesn't reset the RPC session association. status_t restartWrite(size_t desired); + // Set the capacity to `desired`, truncating the Parcel if necessary. status_t continueWrite(size_t desired); status_t writePointer(uintptr_t val); status_t readPointer(uintptr_t *pArg) const; @@ -1247,19 +1257,40 @@ private: uint8_t* mData; size_t mDataSize; size_t mDataCapacity; - mutable size_t mDataPos; - binder_size_t* mObjects; - size_t mObjectsSize; - size_t mObjectsCapacity; - mutable size_t mNextObjectHint; - mutable bool mObjectsSorted; + mutable size_t mDataPos; + + // Fields only needed when parcelling for "kernel Binder". + struct KernelFields { + binder_size_t* mObjects = nullptr; + size_t mObjectsSize = 0; + size_t mObjectsCapacity = 0; + mutable size_t mNextObjectHint = 0; - mutable bool mRequestHeaderPresent; + mutable size_t mWorkSourceRequestHeaderPosition = 0; + mutable bool mRequestHeaderPresent = false; + + mutable bool mObjectsSorted = false; + mutable bool mFdsKnown = true; + mutable bool mHasFds = false; + }; + // Fields only needed when parcelling for RPC Binder. + struct RpcFields { + RpcFields(const sp<RpcSession>& session); - mutable size_t mWorkSourceRequestHeaderPosition; + // Should always be non-null. + const sp<RpcSession> mSession; + }; + std::variant<KernelFields, RpcFields> mVariantFields; + + // Pointer to KernelFields in mVariantFields if present. + KernelFields* maybeKernelFields() { return std::get_if<KernelFields>(&mVariantFields); } + const KernelFields* maybeKernelFields() const { + return std::get_if<KernelFields>(&mVariantFields); + } + // Pointer to RpcFields in mVariantFields if present. + RpcFields* maybeRpcFields() { return std::get_if<RpcFields>(&mVariantFields); } + const RpcFields* maybeRpcFields() const { return std::get_if<RpcFields>(&mVariantFields); } - mutable bool mFdsKnown; - mutable bool mHasFds; bool mAllowFds; // if this parcelable is involved in a secure transaction, force the @@ -1268,7 +1299,6 @@ private: release_func mOwner; - sp<RpcSession> mSession; size_t mReserved; class Blob { diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h index 675585ecef..e17a76cf23 100644 --- a/libs/binder/include/binder/ProcessState.h +++ b/libs/binder/include/binder/ProcessState.h @@ -84,14 +84,15 @@ public: void setCallRestriction(CallRestriction restriction); /** - * Get the max number of threads that the kernel can start. - * - * Note: this is the lower bound. Additional threads may be started. + * Get the max number of threads that have joined the thread pool. + * This includes kernel started threads, user joined threads and polling + * threads if used. */ - size_t getThreadPoolMaxThreadCount() const; + size_t getThreadPoolMaxTotalThreadCount() const; enum class DriverFeature { ONEWAY_SPAM_DETECTION, + EXTENDED_ERROR, }; // Determine whether a feature is supported by the binder driver. static bool isDriverFeatureEnabled(const DriverFeature feature); @@ -132,8 +133,12 @@ private: size_t mExecutingThreadsCount; // Number of threads calling IPCThreadState::blockUntilThreadAvailable() size_t mWaitingForThreads; - // Maximum number for binder threads allowed for this process. + // Maximum number of lazy threads to be started in the threadpool by the kernel. size_t mMaxThreads; + // Current number of threads inside the thread pool. + size_t mCurrentThreads; + // Current number of pooled threads inside the thread pool. + size_t mKernelStartedThreads; // Time when thread pool was emptied int64_t mStarvationStartTimeMs; diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h index a5794428de..cb8158489f 100644 --- a/libs/binder/include/binder/RpcSession.h +++ b/libs/binder/include/binder/RpcSession.h @@ -15,6 +15,7 @@ */ #pragma once +#include <android-base/threads.h> #include <android-base/unique_fd.h> #include <binder/IBinder.h> #include <binder/RpcTransport.h> @@ -211,7 +212,7 @@ private: // whether this or another thread is currently using this fd to make // or receive transactions. - std::optional<pid_t> exclusiveTid; + std::optional<uint64_t> exclusiveTid; bool allowNested = false; }; @@ -276,7 +277,7 @@ private: const sp<RpcConnection>& get() { return mConnection; } private: - static void findConnection(pid_t tid, sp<RpcConnection>* exclusive, + static void findConnection(uint64_t tid, sp<RpcConnection>* exclusive, sp<RpcConnection>* available, std::vector<sp<RpcConnection>>& sockets, size_t socketsIndexHint); diff --git a/libs/binder/include/binder/RpcTransport.h b/libs/binder/include/binder/RpcTransport.h index 751c4f905d..ee4b5483be 100644 --- a/libs/binder/include/binder/RpcTransport.h +++ b/libs/binder/include/binder/RpcTransport.h @@ -20,8 +20,10 @@ #include <functional> #include <memory> +#include <optional> #include <string> +#include <android-base/function_ref.h> #include <android-base/unique_fd.h> #include <utils/Errors.h> @@ -39,8 +41,15 @@ class RpcTransport { public: virtual ~RpcTransport() = default; - // replacement of ::recv(MSG_PEEK). Error code may not be set if TLS is enabled. - [[nodiscard]] virtual status_t peek(void *buf, size_t size, size_t *out_size) = 0; + /** + * Poll the transport to check whether there is any data ready to read. + * + * Return: + * OK - There is data available on this transport + * WOULDBLOCK - No data is available + * error - any other error + */ + [[nodiscard]] virtual status_t pollRead(void) = 0; /** * Read (or write), but allow to be interrupted by a trigger. @@ -58,10 +67,10 @@ public: */ [[nodiscard]] virtual status_t interruptableWriteFully( FdTrigger *fdTrigger, iovec *iovs, int niovs, - const std::function<status_t()> &altPoll) = 0; + const std::optional<android::base::function_ref<status_t()>> &altPoll) = 0; [[nodiscard]] virtual status_t interruptableReadFully( FdTrigger *fdTrigger, iovec *iovs, int niovs, - const std::function<status_t()> &altPoll) = 0; + const std::optional<android::base::function_ref<status_t()>> &altPoll) = 0; protected: RpcTransport() = default; diff --git a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h index c8e78fc55d..78bcb432ab 100644 --- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h +++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h @@ -43,10 +43,12 @@ namespace ndk { /** - * analog using std::shared_ptr for internally held refcount + * Binder analog to using std::shared_ptr for an internally held refcount. * * ref must be called at least one time during the lifetime of this object. The recommended way to * construct this object is with SharedRefBase::make. + * + * If you need a "this" shared reference analogous to shared_from_this, use this->ref(). */ class SharedRefBase { public: diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder_jni.h b/libs/binder/ndk/include_ndk/android/binder_ibinder_jni.h index 6880d86e1a..8e288b307e 100644 --- a/libs/binder/ndk/include_ndk/android/binder_ibinder_jni.h +++ b/libs/binder/ndk/include_ndk/android/binder_ibinder_jni.h @@ -56,6 +56,12 @@ __attribute__((warn_unused_result)) AIBinder* AIBinder_fromJavaBinder(JNIEnv* en * If the binder is null, null is returned. If this binder object was originally an IBinder object, * the original java object will be returned. * + * WARNING: this function returns global and local references. This can be + * figured out using GetObjectRefType. Though, when this function is called + * from within a Java context, the local ref will automatically be cleaned + * up. If this is called outside of a Java frame, + * PushObjectFrame/PopObjectFrame can simulate this automatic cleanup. + * * Available since API level 29. * * \param env Java environment. Must not be null. diff --git a/libs/binder/ndk/include_ndk/android/binder_parcel.h b/libs/binder/ndk/include_ndk/android/binder_parcel.h index 84575811f0..f68612c3ba 100644 --- a/libs/binder/ndk/include_ndk/android/binder_parcel.h +++ b/libs/binder/ndk/include_ndk/android/binder_parcel.h @@ -59,6 +59,11 @@ void AParcel_delete(AParcel* parcel) __INTRODUCED_IN(29); /** * Sets the position within the parcel. * + * This must be called with a position that has been previously returned from + * AParcel_getDataPosition. If writes are made after setting the data position, they must + * be made in the exact same sequence used before resetting data position. Writing over + * objects such as binders or file descriptors is not supported. + * * Available since API level 29. * * \param parcel The parcel of which to set the position. diff --git a/libs/binder/ndk/include_platform/android/binder_stability.h b/libs/binder/ndk/include_platform/android/binder_stability.h index d0cd11f6fc..683a43372b 100644 --- a/libs/binder/ndk/include_platform/android/binder_stability.h +++ b/libs/binder/ndk/include_platform/android/binder_stability.h @@ -98,9 +98,9 @@ static inline void AIBinder_forceDowngradeToLocalStability(AIBinder* binder) { * This interface has system<->vendor stability */ // b/227835797 - can't use __INTRODUCED_IN(30) because old targets load this code -#if __ANDROID_MIN_SDK_VERSION__ < 30 +#if defined(__ANDROID_MIN_SDK_VERSION__) && __ANDROID_MIN_SDK_VERSION__ < 30 __attribute__((weak)) -#endif // __ANDROID_MIN_SDK_VERSION__ < 30 +#endif // defined(__ANDROID_MIN_SDK_VERSION__) && __ANDROID_MIN_SDK_VERSION__ < 30 void AIBinder_markVintfStability(AIBinder* binder); __END_DECLS diff --git a/libs/binder/rust/tests/serialization.cpp b/libs/binder/rust/tests/serialization.cpp index ec780f28a1..3f59dab3a9 100644 --- a/libs/binder/rust/tests/serialization.cpp +++ b/libs/binder/rust/tests/serialization.cpp @@ -381,7 +381,7 @@ TEST_F(SerializationTest, SerializeFileDescriptor) { string expected = "TestingFileDescriptors"; vector<char> buf(expected.length()); base::ReadFully(file_descriptors[0].release(), buf.data(), buf.size()); - ASSERT_EQ(expected, string(buf.data())); + ASSERT_EQ(expected, string(buf.data(), expected.length())); } TEST_F(SerializationTest, SerializeIBinder) { diff --git a/libs/binder/rust/tests/serialization.rs b/libs/binder/rust/tests/serialization.rs index b62da7be03..f6bdf5c9ed 100644 --- a/libs/binder/rust/tests/serialization.rs +++ b/libs/binder/rust/tests/serialization.rs @@ -117,8 +117,8 @@ fn on_transact( ) -> Result<(), StatusCode> { match code { bindings::Transaction_TEST_BOOL => { - assert_eq!(parcel.read::<bool>()?, true); - assert_eq!(parcel.read::<bool>()?, false); + assert!(parcel.read::<bool>()?); + assert!(!parcel.read::<bool>()?); assert_eq!(parcel.read::<Vec<bool>>()?, unsafe { bindings::TESTDATA_BOOL }); diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index a3533d831b..2f96d0e0e0 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -319,7 +319,6 @@ cc_test { "libbinder", "libutils", ], - clang: true, cflags: [ "-g", "-Wno-missing-field-initializers", diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp index e1f5ed5381..2c34766faa 100644 --- a/libs/binder/tests/binderAllocationLimits.cpp +++ b/libs/binder/tests/binderAllocationLimits.cpp @@ -15,8 +15,11 @@ */ #include <android-base/logging.h> -#include <binder/Parcel.h> +#include <binder/Binder.h> #include <binder/IServiceManager.h> +#include <binder/Parcel.h> +#include <binder/RpcServer.h> +#include <binder/RpcSession.h> #include <gtest/gtest.h> #include <utils/CallStack.h> @@ -124,12 +127,18 @@ DestructionAction ScopeDisallowMalloc() { }); } +using android::BBinder; +using android::defaultServiceManager; using android::IBinder; +using android::IServiceManager; +using android::OK; using android::Parcel; -using android::String16; -using android::defaultServiceManager; +using android::RpcServer; +using android::RpcSession; using android::sp; -using android::IServiceManager; +using android::status_t; +using android::statusToString; +using android::String16; static sp<IBinder> GetRemoteBinder() { // This gets binder representing the service manager @@ -175,6 +184,36 @@ TEST(BinderAllocation, SmallTransaction) { EXPECT_EQ(mallocs, 1); } +TEST(RpcBinderAllocation, SetupRpcServer) { + std::string tmp = getenv("TMPDIR") ?: "/tmp"; + std::string addr = tmp + "/binderRpcBenchmark"; + (void)unlink(addr.c_str()); + auto server = RpcServer::make(); + server->setRootObject(sp<BBinder>::make()); + + CHECK_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(); + + auto remoteBinder = session->getRootObject(); + + size_t mallocs = 0, totalBytes = 0; + { + const auto on_malloc = OnMalloc([&](size_t bytes) { + mallocs++; + totalBytes += bytes; + }); + CHECK_EQ(OK, remoteBinder->pingBinder()); + } + EXPECT_EQ(mallocs, 2); + EXPECT_EQ(totalBytes, 56); +} + int main(int argc, char** argv) { if (getenv("LIBC_HOOKS_ENABLE") == nullptr) { CHECK(0 == setenv("LIBC_HOOKS_ENABLE", "1", true /*overwrite*/)); diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index b1e17b7892..3e907263fb 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -82,6 +82,7 @@ static char binderserverarg[] = "--binderserver"; static constexpr int kSchedPolicy = SCHED_RR; static constexpr int kSchedPriority = 7; static constexpr int kSchedPriorityMore = 8; +static constexpr int kKernelThreads = 15; static String16 binderLibTestServiceName = String16("test.binderLib"); @@ -115,6 +116,12 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_ECHO_VECTOR, BINDER_LIB_TEST_REJECT_OBJECTS, BINDER_LIB_TEST_CAN_GET_SID, + BINDER_LIB_TEST_GET_MAX_THREAD_COUNT, + BINDER_LIB_TEST_SET_MAX_THREAD_COUNT, + BINDER_LIB_TEST_LOCK_UNLOCK, + BINDER_LIB_TEST_PROCESS_LOCK, + BINDER_LIB_TEST_UNLOCK_AFTER_MS, + BINDER_LIB_TEST_PROCESS_TEMPORARY_LOCK }; pid_t start_server_process(int arg2, bool usePoll = false) @@ -247,13 +254,11 @@ class BinderLibTest : public ::testing::Test { { int32_t id; Parcel data, reply; - sp<IBinder> binder; EXPECT_THAT(m_server->transact(code, data, &reply), StatusEq(NO_ERROR)); - EXPECT_FALSE(binder != nullptr); - binder = reply.readStrongBinder(); - EXPECT_TRUE(binder != nullptr); + sp<IBinder> binder = reply.readStrongBinder(); + EXPECT_NE(nullptr, binder); EXPECT_THAT(reply.readInt32(&id), StatusEq(NO_ERROR)); if (idPtr) *idPtr = id; @@ -442,6 +447,12 @@ TEST_F(BinderLibTest, CannotUseBinderAfterFork) { EXPECT_DEATH({ ProcessState::self(); }, "libbinder ProcessState can not be used after fork"); } +TEST_F(BinderLibTest, AddManagerToManager) { + sp<IServiceManager> sm = defaultServiceManager(); + sp<IBinder> binder = IInterface::asBinder(sm); + EXPECT_EQ(NO_ERROR, sm->addService(String16("binderLibTest-manager"), binder)); +} + TEST_F(BinderLibTest, WasParceled) { auto binder = sp<BBinder>::make(); EXPECT_FALSE(binder->wasParceled()); @@ -1234,6 +1245,76 @@ TEST(ServiceNotifications, Unregister) { EXPECT_EQ(sm->unregisterForNotifications(String16("RogerRafa"), cb), OK); } +TEST_F(BinderLibTest, ThreadPoolAvailableThreads) { + Parcel data, reply; + sp<IBinder> server = addServer(); + ASSERT_TRUE(server != nullptr); + EXPECT_THAT(server->transact(BINDER_LIB_TEST_GET_MAX_THREAD_COUNT, data, &reply), + StatusEq(NO_ERROR)); + int32_t replyi = reply.readInt32(); + // Expect 16 threads: kKernelThreads = 15 + Pool thread == 16 + EXPECT_TRUE(replyi == kKernelThreads || replyi == kKernelThreads + 1); + EXPECT_THAT(server->transact(BINDER_LIB_TEST_PROCESS_LOCK, data, &reply), NO_ERROR); + + /* + * This will use all threads in the pool expect the main pool thread. + * The service should run fine without locking, and the thread count should + * not exceed 16 (15 Max + pool thread). + */ + std::vector<std::thread> ts; + for (size_t i = 0; i < kKernelThreads - 1; i++) { + ts.push_back(std::thread([&] { + EXPECT_THAT(server->transact(BINDER_LIB_TEST_LOCK_UNLOCK, data, &reply), NO_ERROR); + })); + } + + data.writeInt32(1); + // Give a chance for all threads to be used + EXPECT_THAT(server->transact(BINDER_LIB_TEST_UNLOCK_AFTER_MS, data, &reply), NO_ERROR); + + for (auto &t : ts) { + t.join(); + } + + EXPECT_THAT(server->transact(BINDER_LIB_TEST_GET_MAX_THREAD_COUNT, data, &reply), + StatusEq(NO_ERROR)); + replyi = reply.readInt32(); + // No more than 16 threads should exist. + EXPECT_TRUE(replyi == kKernelThreads || replyi == kKernelThreads + 1); +} + +size_t epochMillis() { + using std::chrono::duration_cast; + using std::chrono::milliseconds; + using std::chrono::seconds; + using std::chrono::system_clock; + return duration_cast<milliseconds>(system_clock::now().time_since_epoch()).count(); +} + +TEST_F(BinderLibTest, HangingServices) { + Parcel data, reply; + sp<IBinder> server = addServer(); + ASSERT_TRUE(server != nullptr); + int32_t delay = 1000; // ms + data.writeInt32(delay); + EXPECT_THAT(server->transact(BINDER_LIB_TEST_PROCESS_TEMPORARY_LOCK, data, &reply), NO_ERROR); + std::vector<std::thread> ts; + size_t epochMsBefore = epochMillis(); + for (size_t i = 0; i < kKernelThreads + 1; i++) { + ts.push_back(std::thread([&] { + EXPECT_THAT(server->transact(BINDER_LIB_TEST_LOCK_UNLOCK, data, &reply), NO_ERROR); + })); + } + + for (auto &t : ts) { + t.join(); + } + size_t epochMsAfter = epochMillis(); + + // deadlock occurred and threads only finished after 1s passed. + EXPECT_GE(epochMsAfter, epochMsBefore + delay); +} + class BinderLibRpcTestBase : public BinderLibTest { public: void SetUp() override { @@ -1640,11 +1721,42 @@ public: case BINDER_LIB_TEST_CAN_GET_SID: { return IPCThreadState::self()->getCallingSid() == nullptr ? BAD_VALUE : NO_ERROR; } + case BINDER_LIB_TEST_GET_MAX_THREAD_COUNT: { + reply->writeInt32(ProcessState::self()->getThreadPoolMaxTotalThreadCount()); + return NO_ERROR; + } + case BINDER_LIB_TEST_PROCESS_LOCK: { + m_blockMutex.lock(); + return NO_ERROR; + } + case BINDER_LIB_TEST_LOCK_UNLOCK: { + std::lock_guard<std::mutex> _l(m_blockMutex); + return NO_ERROR; + } + case BINDER_LIB_TEST_UNLOCK_AFTER_MS: { + int32_t ms = data.readInt32(); + return unlockInMs(ms); + } + case BINDER_LIB_TEST_PROCESS_TEMPORARY_LOCK: { + m_blockMutex.lock(); + sp<BinderLibTestService> thisService = this; + int32_t value = data.readInt32(); + // start local thread to unlock in 1s + std::thread t([=] { thisService->unlockInMs(value); }); + t.detach(); + return NO_ERROR; + } default: return UNKNOWN_TRANSACTION; }; } + status_t unlockInMs(int32_t ms) { + usleep(ms * 1000); + m_blockMutex.unlock(); + return NO_ERROR; + } + private: int32_t m_id; int32_t m_nextServerId; @@ -1655,6 +1767,7 @@ private: sp<IBinder> m_strongRef; sp<IBinder> m_callback; bool m_exitOnDestroy; + std::mutex m_blockMutex; }; int run_server(int index, int readypipefd, bool usePoll) @@ -1756,6 +1869,7 @@ int run_server(int index, int readypipefd, bool usePoll) } } } else { + ProcessState::self()->setThreadPoolMaxThreadCount(kKernelThreads); ProcessState::self()->startThreadPool(); IPCThreadState::self()->joinThreadPool(); } diff --git a/libs/binder/tests/binderMemoryHeapBaseUnitTest.cpp b/libs/binder/tests/binderMemoryHeapBaseUnitTest.cpp index 21cb70be17..278dd2bf81 100644 --- a/libs/binder/tests/binderMemoryHeapBaseUnitTest.cpp +++ b/libs/binder/tests/binderMemoryHeapBaseUnitTest.cpp @@ -23,6 +23,7 @@ using namespace android; #ifdef __BIONIC__ TEST(MemoryHeapBase, ForceMemfdRespected) { auto mHeap = sp<MemoryHeapBase>::make(10, MemoryHeapBase::FORCE_MEMFD, "Test mapping"); + ASSERT_NE(mHeap.get(), nullptr); int fd = mHeap->getHeapID(); EXPECT_NE(fd, -1); EXPECT_FALSE(ashmem_valid(fd)); @@ -33,6 +34,7 @@ TEST(MemoryHeapBase, MemfdSealed) { auto mHeap = sp<MemoryHeapBase>::make(8192, MemoryHeapBase::FORCE_MEMFD, "Test mapping"); + ASSERT_NE(mHeap.get(), nullptr); int fd = mHeap->getHeapID(); EXPECT_NE(fd, -1); EXPECT_EQ(fcntl(fd, F_GET_SEALS), F_SEAL_SEAL); @@ -43,6 +45,7 @@ TEST(MemoryHeapBase, MemfdUnsealed) { MemoryHeapBase::FORCE_MEMFD | MemoryHeapBase::MEMFD_ALLOW_SEALING, "Test mapping"); + ASSERT_NE(mHeap.get(), nullptr); int fd = mHeap->getHeapID(); EXPECT_NE(fd, -1); EXPECT_EQ(fcntl(fd, F_GET_SEALS), 0); @@ -53,6 +56,7 @@ TEST(MemoryHeapBase, MemfdSealedProtected) { MemoryHeapBase::FORCE_MEMFD | MemoryHeapBase::READ_ONLY, "Test mapping"); + ASSERT_NE(mHeap.get(), nullptr); int fd = mHeap->getHeapID(); EXPECT_NE(fd, -1); EXPECT_EQ(fcntl(fd, F_GET_SEALS), F_SEAL_SEAL | F_SEAL_FUTURE_WRITE); @@ -64,6 +68,7 @@ TEST(MemoryHeapBase, MemfdUnsealedProtected) { MemoryHeapBase::READ_ONLY | MemoryHeapBase::MEMFD_ALLOW_SEALING, "Test mapping"); + ASSERT_NE(mHeap.get(), nullptr); int fd = mHeap->getHeapID(); EXPECT_NE(fd, -1); EXPECT_EQ(fcntl(fd, F_GET_SEALS), F_SEAL_FUTURE_WRITE); @@ -74,6 +79,7 @@ TEST(MemoryHeapBase, HostMemfdExpected) { auto mHeap = sp<MemoryHeapBase>::make(8192, MemoryHeapBase::READ_ONLY, "Test mapping"); + ASSERT_NE(mHeap.get(), nullptr); int fd = mHeap->getHeapID(); void* ptr = mHeap->getBase(); EXPECT_NE(ptr, MAP_FAILED); @@ -87,6 +93,7 @@ TEST(MemoryHeapBase,HostMemfdException) { MemoryHeapBase::READ_ONLY | MemoryHeapBase::MEMFD_ALLOW_SEALING, "Test mapping"); + ASSERT_NE(mHeap.get(), nullptr); int fd = mHeap->getHeapID(); void* ptr = mHeap->getBase(); EXPECT_EQ(mHeap->getFlags(), MemoryHeapBase::READ_ONLY); diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index c2639e7c67..4161a7a25b 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -1248,9 +1248,14 @@ TEST_P(BinderRpc, Callbacks) { proc.rootIface->doCallback(cb, callbackIsOneway, delayed, kTestString)); } - using std::literals::chrono_literals::operator""s; - std::unique_lock<std::mutex> _l(cb->mMutex); - cb->mCv.wait_for(_l, 1s, [&] { return !cb->mValues.empty(); }); + // if both transactions are synchronous and the response is sent back on the + // same thread, everything should have happened in a nested call. Otherwise, + // the callback will be processed on another thread. + if (callIsOneway || callbackIsOneway || delayed) { + using std::literals::chrono_literals::operator""s; + std::unique_lock<std::mutex> _l(cb->mMutex); + cb->mCv.wait_for(_l, 1s, [&] { return !cb->mValues.empty(); }); + } EXPECT_EQ(cb->mValues.size(), 1) << "callIsOneway: " << callIsOneway @@ -1676,7 +1681,8 @@ public: FdTrigger* fdTrigger) { std::string message(kMessage); iovec messageIov{message.data(), message.size()}; - auto status = serverTransport->interruptableWriteFully(fdTrigger, &messageIov, 1, {}); + auto status = serverTransport->interruptableWriteFully(fdTrigger, &messageIov, 1, + std::nullopt); if (status != OK) return AssertionFailure() << statusToString(status); return AssertionSuccess(); } @@ -1708,8 +1714,9 @@ public: LOG_ALWAYS_FATAL_IF(mClientTransport == nullptr, "setUpTransport not called or failed"); std::string readMessage(expectedMessage.size(), '\0'); iovec readMessageIov{readMessage.data(), readMessage.size()}; - status_t readStatus = mClientTransport->interruptableReadFully(mFdTrigger.get(), - &readMessageIov, 1, {}); + status_t readStatus = + mClientTransport->interruptableReadFully(mFdTrigger.get(), &readMessageIov, 1, + std::nullopt); if (readStatus != OK) { return AssertionFailure() << statusToString(readStatus); } @@ -1904,7 +1911,8 @@ TEST_P(RpcTransportTest, Trigger) { auto serverPostConnect = [&](RpcTransport* serverTransport, FdTrigger* fdTrigger) { std::string message(RpcTransportTestUtils::kMessage); iovec messageIov{message.data(), message.size()}; - auto status = serverTransport->interruptableWriteFully(fdTrigger, &messageIov, 1, {}); + auto status = + serverTransport->interruptableWriteFully(fdTrigger, &messageIov, 1, std::nullopt); if (status != OK) return AssertionFailure() << statusToString(status); { @@ -1915,7 +1923,7 @@ TEST_P(RpcTransportTest, Trigger) { } iovec msg2Iov{msg2.data(), msg2.size()}; - status = serverTransport->interruptableWriteFully(fdTrigger, &msg2Iov, 1, {}); + status = serverTransport->interruptableWriteFully(fdTrigger, &msg2Iov, 1, std::nullopt); if (status != DEAD_OBJECT) return AssertionFailure() << "When FdTrigger is shut down, interruptableWriteFully " "should return DEAD_OBJECT, but it is " diff --git a/libs/binder/tests/parcel_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/Android.bp index e5d32da4cc..2ca6ebdbd2 100644 --- a/libs/binder/tests/parcel_fuzzer/Android.bp +++ b/libs/binder/tests/parcel_fuzzer/Android.bp @@ -59,6 +59,7 @@ cc_fuzz { cc_library_static { name: "libbinder_random_parcel", host_supported: true, + vendor_available: true, target: { darwin: { enabled: false, diff --git a/libs/binder/tests/parcel_fuzzer/binder.cpp b/libs/binder/tests/parcel_fuzzer/binder.cpp index 47ec776990..7059d30bb4 100644 --- a/libs/binder/tests/parcel_fuzzer/binder.cpp +++ b/libs/binder/tests/parcel_fuzzer/binder.cpp @@ -73,20 +73,20 @@ struct BigStruct { uint8_t data[1337]; }; -#define PARCEL_READ_WITH_STATUS(T, FUN) \ - [] (const ::android::Parcel& p, uint8_t /*data*/) {\ - FUZZ_LOG() << "about to read " #T " using " #FUN " with status";\ - T t{};\ - status_t status = p.FUN(&t);\ - FUZZ_LOG() << #T " status: " << status /* << " value: " << t*/;\ +#define PARCEL_READ_WITH_STATUS(T, FUN) \ + [](const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { \ + FUZZ_LOG() << "about to read " #T " using " #FUN " with status"; \ + T t{}; \ + status_t status = p.FUN(&t); \ + FUZZ_LOG() << #T " status: " << status /* << " value: " << t*/; \ } -#define PARCEL_READ_NO_STATUS(T, FUN) \ - [] (const ::android::Parcel& p, uint8_t /*data*/) {\ - FUZZ_LOG() << "about to read " #T " using " #FUN " with no status";\ - T t = p.FUN();\ - (void) t;\ - FUZZ_LOG() << #T " done " /* << " value: " << t*/;\ +#define PARCEL_READ_NO_STATUS(T, FUN) \ + [](const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { \ + FUZZ_LOG() << "about to read " #T " using " #FUN " with no status"; \ + T t = p.FUN(); \ + (void)t; \ + FUZZ_LOG() << #T " done " /* << " value: " << t*/; \ } #define PARCEL_READ_OPT_STATUS(T, FUN) \ @@ -102,7 +102,9 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_NO_STATUS(size_t, dataPosition), PARCEL_READ_NO_STATUS(size_t, dataCapacity), PARCEL_READ_NO_STATUS(::android::binder::Status, enforceNoDataAvail), - [] (const ::android::Parcel& p, uint8_t pos) { + [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { + // aborts on larger values + size_t pos = provider.ConsumeIntegralInRange<size_t>(0, INT32_MAX); FUZZ_LOG() << "about to setDataPosition: " << pos; p.setDataPosition(pos); FUZZ_LOG() << "setDataPosition done"; @@ -111,13 +113,13 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_NO_STATUS(size_t, hasFileDescriptors), PARCEL_READ_NO_STATUS(std::vector<android::sp<android::IBinder>>, debugReadAllStrongBinders), PARCEL_READ_NO_STATUS(std::vector<int>, debugReadAllFileDescriptors), - [] (const ::android::Parcel& p, uint8_t len) { - std::string interface(len, 'a'); + [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { + std::string interface = provider.ConsumeRandomLengthString(); FUZZ_LOG() << "about to enforceInterface: " << interface; bool b = p.enforceInterface(::android::String16(interface.c_str())); FUZZ_LOG() << "enforced interface: " << b; }, - [] (const ::android::Parcel& p, uint8_t /*len*/) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to checkInterface"; android::sp<android::IBinder> aBinder = new android::BBinder(); bool b = p.checkInterface(aBinder.get()); @@ -125,13 +127,16 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { }, PARCEL_READ_NO_STATUS(size_t, objectsCount), PARCEL_READ_NO_STATUS(status_t, errorCheck), - [] (const ::android::Parcel& p, uint8_t len) { + [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { + // Read at least a bit. Unbounded allocation would OOM. + size_t len = provider.ConsumeIntegralInRange<size_t>(0, 1024); FUZZ_LOG() << "about to read void*"; std::vector<uint8_t> data(len); status_t status = p.read(data.data(), len); FUZZ_LOG() << "read status: " << status; }, - [] (const ::android::Parcel& p, uint8_t len) { + [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { + size_t len = provider.ConsumeIntegral<size_t>(); FUZZ_LOG() << "about to readInplace"; const void* r = p.readInplace(len); FUZZ_LOG() << "readInplace done. pointer: " << r << " bytes: " << (r ? HexString(r, len) : "null"); @@ -149,13 +154,13 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_WITH_STATUS(std::string, readUtf8FromUtf16), PARCEL_READ_WITH_STATUS(std::unique_ptr<std::string>, readUtf8FromUtf16), PARCEL_READ_WITH_STATUS(std::optional<std::string>, readUtf8FromUtf16), - [] (const ::android::Parcel& p, uint8_t /*data*/) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to read c-str"; const char* str = p.readCString(); FUZZ_LOG() << "read c-str: " << (str ? str : "<empty string>"); }, PARCEL_READ_OPT_STATUS(android::String8, readString8), - [] (const ::android::Parcel& p, uint8_t /*data*/) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to readString8Inplace"; size_t outLen = 0; const char* str = p.readString8Inplace(&outLen); @@ -165,7 +170,7 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_OPT_STATUS(android::String16, readString16), PARCEL_READ_WITH_STATUS(std::unique_ptr<android::String16>, readString16), PARCEL_READ_WITH_STATUS(std::optional<android::String16>, readString16), - [] (const ::android::Parcel& p, uint8_t /*data*/) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to readString16Inplace"; size_t outLen = 0; const char16_t* str = p.readString16Inplace(&outLen); @@ -263,13 +268,13 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_WITH_STATUS(std::optional<std::array<std::array<std::optional<ExampleParcelable> COMMA 3> COMMA 4>>, readFixedArray), #undef COMMA - [] (const android::Parcel& p, uint8_t /*len*/) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to read flattenable"; ExampleFlattenable f; status_t status = p.read(f); FUZZ_LOG() << "read flattenable: " << status; }, - [] (const android::Parcel& p, uint8_t /*len*/) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to read lite flattenable"; ExampleLightFlattenable f; status_t status = p.read(f); @@ -284,7 +289,7 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_WITH_STATUS(std::unique_ptr<std::vector<BigStruct>>, resizeOutVector), PARCEL_READ_NO_STATUS(int32_t, readExceptionCode), - [] (const android::Parcel& p, uint8_t /*len*/) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to readNativeHandle"; native_handle_t* t = p.readNativeHandle(); FUZZ_LOG() << "readNativeHandle: " << t; @@ -303,15 +308,16 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_WITH_STATUS(std::optional<std::vector<android::base::unique_fd>>, readUniqueFileDescriptorVector), PARCEL_READ_WITH_STATUS(std::vector<android::base::unique_fd>, readUniqueFileDescriptorVector), - [] (const android::Parcel& p, uint8_t len) { + [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { + size_t len = provider.ConsumeIntegral<size_t>(); FUZZ_LOG() << "about to readBlob"; ::android::Parcel::ReadableBlob blob; status_t status = p.readBlob(len, &blob); FUZZ_LOG() << "readBlob status: " << status; }, - [] (const android::Parcel& p, uint8_t options) { + [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { FUZZ_LOG() << "about to readObject"; - bool nullMetaData = options & 0x1; + bool nullMetaData = provider.ConsumeBool(); const void* obj = static_cast<const void*>(p.readObject(nullMetaData)); FUZZ_LOG() << "readObject: " << obj; }, @@ -319,20 +325,19 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_NO_STATUS(size_t, getOpenAshmemSize), // additional parcelable objects defined in libbinder - [] (const ::android::Parcel& p, uint8_t data) { + [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { using ::android::os::ParcelableHolder; using ::android::Parcelable; FUZZ_LOG() << "about to read ParcelableHolder using readParcelable with status"; - Parcelable::Stability stability = Parcelable::Stability::STABILITY_LOCAL; - if ( (data & 1) == 1 ) { - stability = Parcelable::Stability::STABILITY_VINTF; - } + Parcelable::Stability stability = provider.ConsumeBool() + ? Parcelable::Stability::STABILITY_LOCAL + : Parcelable::Stability::STABILITY_VINTF; ParcelableHolder t = ParcelableHolder(stability); status_t status = p.readParcelable(&t); FUZZ_LOG() << "ParcelableHolder status: " << status; }, PARCEL_READ_WITH_STATUS(android::os::PersistableBundle, readParcelable), - [] (const ::android::Parcel& p, uint8_t /* data */) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to call hasFileDescriptorsInRange() with status"; size_t offset = p.readUint32(); size_t length = p.readUint32(); @@ -340,7 +345,7 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { status_t status = p.hasFileDescriptorsInRange(offset, length, &result); FUZZ_LOG() << " status: " << status << " result: " << result; }, - [] (const ::android::Parcel& p, uint8_t /* data */) { + [] (const ::android::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to call compareDataInRange() with status"; size_t thisOffset = p.readUint32(); size_t otherOffset = p.readUint32(); diff --git a/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp b/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp index 5aeb5cc6f2..26d67704b2 100644 --- a/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp +++ b/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp @@ -70,7 +70,7 @@ binder_status_t ISomeInterface::readFromParcel(const AParcel* parcel, } #define PARCEL_READ(T, FUN) \ - [](const NdkParcelAdapter& p, uint8_t /*data*/) { \ + [](const NdkParcelAdapter& p, FuzzedDataProvider& /*provider*/) { \ FUZZ_LOG() << "about to read " #T " using " #FUN " with status"; \ T t{}; \ binder_status_t status = FUN(p.aParcel(), &t); \ @@ -80,32 +80,37 @@ binder_status_t ISomeInterface::readFromParcel(const AParcel* parcel, // clang-format off std::vector<ParcelRead<NdkParcelAdapter>> BINDER_NDK_PARCEL_READ_FUNCTIONS{ // methods from binder_parcel.h - [](const NdkParcelAdapter& p, uint8_t pos) { + [](const NdkParcelAdapter& p, FuzzedDataProvider& provider) { + // aborts on larger values + size_t pos = provider.ConsumeIntegralInRange<size_t>(0, INT32_MAX); FUZZ_LOG() << "about to set data position to " << pos; binder_status_t status = AParcel_setDataPosition(p.aParcel(), pos); FUZZ_LOG() << "set data position: " << status; }, - [](const NdkParcelAdapter& p, uint8_t /*data*/) { + [](const NdkParcelAdapter& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to read status header"; ndk::ScopedAStatus t; binder_status_t status = AParcel_readStatusHeader(p.aParcel(), t.getR()); FUZZ_LOG() << "read status header: " << status; }, - [](const NdkParcelAdapter& p, uint8_t /*data*/) { + [](const NdkParcelAdapter& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to getDataSize the parcel"; AParcel_getDataSize(p.aParcel()); FUZZ_LOG() << "getDataSize done"; }, - [](const NdkParcelAdapter& p, uint8_t data) { + [](const NdkParcelAdapter& p, FuzzedDataProvider& provider) { FUZZ_LOG() << "about to read a ParcelableHolder"; - ndk::AParcelableHolder ph {(data % 2 == 1) ? ndk::STABILITY_LOCAL : ndk::STABILITY_VINTF}; + ndk::AParcelableHolder ph {provider.ConsumeBool() ? ndk::STABILITY_LOCAL : ndk::STABILITY_VINTF}; binder_status_t status = AParcel_readParcelable(p.aParcel(), &ph); FUZZ_LOG() << "read the ParcelableHolder: " << status; }, - [](const NdkParcelAdapter& p, uint8_t data) { - FUZZ_LOG() << "about to appendFrom"; + [](const NdkParcelAdapter& p, FuzzedDataProvider& provider) { + size_t offset = provider.ConsumeIntegral<size_t>(); + size_t pos = provider.ConsumeIntegral<size_t>(); + FUZZ_LOG() << "about to appendFrom " << pos; + // TODO: create random parcel AParcel* parcel = AParcel_create(); - binder_status_t status = AParcel_appendFrom(p.aParcel(), parcel, 0, data); + binder_status_t status = AParcel_appendFrom(p.aParcel(), parcel, offset, pos); AParcel_delete(parcel); FUZZ_LOG() << "appendFrom: " << status; }, diff --git a/libs/binder/tests/parcel_fuzzer/binder_ndk.h b/libs/binder/tests/parcel_fuzzer/binder_ndk.h index cf24ab932b..81e79b5382 100644 --- a/libs/binder/tests/parcel_fuzzer/binder_ndk.h +++ b/libs/binder/tests/parcel_fuzzer/binder_ndk.h @@ -43,6 +43,10 @@ public: return aParcel()->get()->setData(buffer, len); } + android::status_t appendFrom(const NdkParcelAdapter* parcel, int32_t start, int32_t len) { + return AParcel_appendFrom(parcel->aParcel(), aParcel(), start, len); + } + private: ndk::ScopedAParcel mParcel; }; diff --git a/libs/binder/tests/parcel_fuzzer/hwbinder.cpp b/libs/binder/tests/parcel_fuzzer/hwbinder.cpp index ee9840f133..438e8ae9b2 100644 --- a/libs/binder/tests/parcel_fuzzer/hwbinder.cpp +++ b/libs/binder/tests/parcel_fuzzer/hwbinder.cpp @@ -35,19 +35,19 @@ std::ostream& operator<<(std::ostream& os, const ::android::sp<::android::hardwa #define PARCEL_READ_OPT_STATUS(T, FUN) \ PARCEL_READ_NO_STATUS(T, FUN), PARCEL_READ_WITH_STATUS(T, FUN) -#define PARCEL_READ_NO_STATUS(T, FUN) \ - [] (const ::android::hardware::Parcel& p, uint8_t /*data*/) {\ - FUZZ_LOG() << "about to read " #T " using " #FUN " with no status";\ - T t = p.FUN();\ - FUZZ_LOG() << #T " value: " << t;\ +#define PARCEL_READ_NO_STATUS(T, FUN) \ + [](const ::android::hardware::Parcel& p, FuzzedDataProvider& /*provider*/) { \ + FUZZ_LOG() << "about to read " #T " using " #FUN " with no status"; \ + T t = p.FUN(); \ + FUZZ_LOG() << #T " value: " << t; \ } -#define PARCEL_READ_WITH_STATUS(T, FUN) \ - [] (const ::android::hardware::Parcel& p, uint8_t /*data*/) {\ - FUZZ_LOG() << "about to read " #T " using " #FUN " with status";\ - T t;\ - status_t status = p.FUN(&t);\ - FUZZ_LOG() << #T " status: " << status << " value: " << t;\ +#define PARCEL_READ_WITH_STATUS(T, FUN) \ + [](const ::android::hardware::Parcel& p, FuzzedDataProvider& /*provider*/) { \ + FUZZ_LOG() << "about to read " #T " using " #FUN " with status"; \ + T t; \ + status_t status = p.FUN(&t); \ + FUZZ_LOG() << #T " status: " << status << " value: " << t; \ } // clang-format off @@ -56,27 +56,30 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI PARCEL_READ_NO_STATUS(size_t, dataAvail), PARCEL_READ_NO_STATUS(size_t, dataPosition), PARCEL_READ_NO_STATUS(size_t, dataCapacity), - [] (const ::android::hardware::Parcel& p, uint8_t pos) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { + // aborts on larger values + size_t pos = provider.ConsumeIntegralInRange<size_t>(0, INT32_MAX); FUZZ_LOG() << "about to setDataPosition: " << pos; p.setDataPosition(pos); FUZZ_LOG() << "setDataPosition done"; }, - [] (const ::android::hardware::Parcel& p, uint8_t length) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { FUZZ_LOG() << "about to enforceInterface"; - std::string interfaceName(length, 'a'); - bool okay = p.enforceInterface(interfaceName.c_str()); + bool okay = p.enforceInterface(provider.ConsumeRandomLengthString().c_str()); FUZZ_LOG() << "enforceInterface status: " << okay; }, PARCEL_READ_NO_STATUS(size_t, objectsCount), - [] (const ::android::hardware::Parcel& p, uint8_t length) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { + // Read at least a bit. Unbounded allocation would OOM. + size_t length = provider.ConsumeIntegralInRange<size_t>(0, 1024); FUZZ_LOG() << "about to read"; std::vector<uint8_t> data (length); status_t status = p.read(data.data(), length); FUZZ_LOG() << "read status: " << status << " data: " << HexString(data.data(), data.size()); }, - [] (const ::android::hardware::Parcel& p, uint8_t length) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { + size_t length = provider.ConsumeIntegral<size_t>(); FUZZ_LOG() << "about to read"; - std::vector<uint8_t> data (length); const void* inplace = p.readInplace(length); FUZZ_LOG() << "read status: " << (inplace ? HexString(inplace, length) : "null"); }, @@ -91,14 +94,14 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI PARCEL_READ_OPT_STATUS(float, readFloat), PARCEL_READ_OPT_STATUS(double, readDouble), PARCEL_READ_OPT_STATUS(bool, readBool), - [] (const ::android::hardware::Parcel& p, uint8_t /*data*/) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to readCString"; const char* str = p.readCString(); FUZZ_LOG() << "readCString " << (str ? str : "<null>"); }, PARCEL_READ_OPT_STATUS(::android::String16, readString16), PARCEL_READ_WITH_STATUS(std::unique_ptr<::android::String16>, readString16), - [] (const ::android::hardware::Parcel& p, uint8_t /*data*/) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to readString16Inplace"; size_t outSize = 0; const char16_t* str = p.readString16Inplace(&outSize); @@ -106,7 +109,8 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI }, PARCEL_READ_OPT_STATUS(::android::sp<::android::hardware::IBinder>, readStrongBinder), PARCEL_READ_WITH_STATUS(::android::sp<::android::hardware::IBinder>, readNullableStrongBinder), - [] (const ::android::hardware::Parcel& p, uint8_t size) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { + size_t size = provider.ConsumeIntegral<size_t>(); FUZZ_LOG() << "about to readBuffer"; size_t handle = 0; const void* data = nullptr; @@ -116,7 +120,8 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI // should be null since we don't create any IPC objects CHECK(data == nullptr) << data; }, - [] (const ::android::hardware::Parcel& p, uint8_t size) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { + size_t size = provider.ConsumeIntegral<size_t>(); FUZZ_LOG() << "about to readNullableBuffer"; size_t handle = 0; const void* data = nullptr; @@ -126,7 +131,8 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI // should be null since we don't create any IPC objects CHECK(data == nullptr) << data; }, - [] (const ::android::hardware::Parcel& p, uint8_t size) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { + size_t size = provider.ConsumeIntegral<size_t>(); FUZZ_LOG() << "about to readEmbeddedBuffer"; size_t handle = 0; size_t parent_buffer_handle = 0; @@ -138,7 +144,8 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI // should be null since we don't create any IPC objects CHECK(data == nullptr) << data; }, - [] (const ::android::hardware::Parcel& p, uint8_t size) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& provider) { + size_t size = provider.ConsumeIntegral<size_t>(); FUZZ_LOG() << "about to readNullableEmbeddedBuffer"; size_t handle = 0; size_t parent_buffer_handle = 0; @@ -150,7 +157,7 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI // should be null since we don't create any IPC objects CHECK(data == nullptr) << data; }, - [] (const ::android::hardware::Parcel& p, uint8_t /*data*/) { + [] (const ::android::hardware::Parcel& p, FuzzedDataProvider& /*provider*/) { FUZZ_LOG() << "about to readNativeHandleNoDup"; const native_handle_t* handle = nullptr; status_t status = p.readNativeHandleNoDup(&handle); diff --git a/libs/binder/tests/parcel_fuzzer/main.cpp b/libs/binder/tests/parcel_fuzzer/main.cpp index f435dae659..180a177689 100644 --- a/libs/binder/tests/parcel_fuzzer/main.cpp +++ b/libs/binder/tests/parcel_fuzzer/main.cpp @@ -83,22 +83,36 @@ void doReadFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads, FUZZ_LOG() << "input: " << HexString(p.data(), p.dataSize()); FUZZ_LOG() << "instructions: " << HexString(instructions.data(), instructions.size()); - for (size_t i = 0; i + 1 < instructions.size(); i += 2) { - uint8_t a = instructions[i]; - uint8_t readIdx = a % reads.size(); + FuzzedDataProvider instructionsProvider(instructions.data(), instructions.size()); + while (instructionsProvider.remaining_bytes() > 0) { + uint8_t idx = instructionsProvider.ConsumeIntegralInRange<uint8_t>(0, reads.size() - 1); - uint8_t b = instructions[i + 1]; + FUZZ_LOG() << "Instruction " << idx << " avail: " << p.dataAvail() + << " pos: " << p.dataPosition() << " cap: " << p.dataCapacity(); - FUZZ_LOG() << "Instruction: " << (i / 2) + 1 << "/" << instructions.size() / 2 - << " cmd: " << static_cast<size_t>(a) << " (" << static_cast<size_t>(readIdx) - << ") arg: " << static_cast<size_t>(b) << " size: " << p.dataSize() - << " avail: " << p.dataAvail() << " pos: " << p.dataPosition() - << " cap: " << p.dataCapacity(); - - reads[readIdx](p, b); + reads[idx](p, instructionsProvider); } } +// Append two random parcels. +template <typename P> +void doAppendFuzz(const char* backend, FuzzedDataProvider&& provider) { + int32_t start = provider.ConsumeIntegral<int32_t>(); + int32_t len = provider.ConsumeIntegral<int32_t>(); + + std::vector<uint8_t> bytes = provider.ConsumeBytes<uint8_t>( + provider.ConsumeIntegralInRange<size_t>(0, provider.remaining_bytes())); + + P p0, p1; + fillRandomParcel(&p0, FuzzedDataProvider(bytes.data(), bytes.size())); + fillRandomParcel(&p1, std::move(provider)); + + FUZZ_LOG() << "backend: " << backend; + FUZZ_LOG() << "start: " << start << " len: " << len; + + p0.appendFrom(&p1, start, len); +} + void* NothingClass_onCreate(void* args) { return args; } @@ -148,6 +162,12 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { doReadFuzz<NdkParcelAdapter>("binder_ndk", BINDER_NDK_PARCEL_READ_FUNCTIONS, std::move(provider)); }, + [](FuzzedDataProvider&& provider) { + doAppendFuzz<::android::Parcel>("binder", std::move(provider)); + }, + [](FuzzedDataProvider&& provider) { + doAppendFuzz<NdkParcelAdapter>("binder_ndk", std::move(provider)); + }, }; provider.PickValueInArray(fuzzBackend)(std::move(provider)); diff --git a/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h b/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h index b68a8a91e6..765a93e8c9 100644 --- a/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h +++ b/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h @@ -15,5 +15,9 @@ */ #pragma once +#include <fuzzer/FuzzedDataProvider.h> + +#include <functional> + template <typename P> -using ParcelRead = std::function<void(const P& p, uint8_t data)>; +using ParcelRead = std::function<void(const P& p, FuzzedDataProvider& provider)>; diff --git a/libs/cputimeinstate/cputimeinstate.cpp b/libs/cputimeinstate/cputimeinstate.cpp index 7e9bb7d468..d1690437ef 100644 --- a/libs/cputimeinstate/cputimeinstate.cpp +++ b/libs/cputimeinstate/cputimeinstate.cpp @@ -300,11 +300,11 @@ std::optional<std::vector<std::vector<uint64_t>>> getUidCpuFreqTimes(uint32_t ui } std::vector<tis_val_t> vals(gNCpus); - time_key_t key = {.uid = uid}; for (uint32_t i = 0; i <= (maxFreqCount - 1) / FREQS_PER_ENTRY; ++i) { - key.bucket = i; + const time_key_t key = {.uid = uid, .bucket = i}; if (findMapEntry(gTisMapFd, &key, vals.data())) { - if (errno != ENOENT || getFirstMapKey(gTisMapFd, &key)) return {}; + time_key_t tmpKey; + if (errno != ENOENT || getFirstMapKey(gTisMapFd, &tmpKey)) return {}; continue; } @@ -412,10 +412,11 @@ std::optional<concurrent_time_t> getUidConcurrentTimes(uint32_t uid, bool retry) concurrent_time_t ret = {.active = std::vector<uint64_t>(gNCpus, 0)}; for (const auto &cpuList : gPolicyCpus) ret.policy.emplace_back(cpuList.size(), 0); std::vector<concurrent_val_t> vals(gNCpus); - time_key_t key = {.uid = uid}; - for (key.bucket = 0; key.bucket <= (gNCpus - 1) / CPUS_PER_ENTRY; ++key.bucket) { + for (uint32_t i = 0; i <= (gNCpus - 1) / CPUS_PER_ENTRY; ++i) { + const time_key_t key = {.uid = uid, .bucket = i}; if (findMapEntry(gConcurrentMapFd, &key, vals.data())) { - if (errno != ENOENT || getFirstMapKey(gConcurrentMapFd, &key)) return {}; + time_key_t tmpKey; + if (errno != ENOENT || getFirstMapKey(gConcurrentMapFd, &tmpKey)) return {}; continue; } auto offset = key.bucket * CPUS_PER_ENTRY; diff --git a/libs/graphicsenv/OWNERS b/libs/graphicsenv/OWNERS index 8c284647f2..347c4e0db1 100644 --- a/libs/graphicsenv/OWNERS +++ b/libs/graphicsenv/OWNERS @@ -1,4 +1,10 @@ +abdolrashidi@google.com +cclao@google.com chrisforbes@google.com cnorthrop@google.com +ianelliott@google.com +lfy@google.com lpy@google.com -timvp@google.com +romanl@google.com +vantablack@google.com +yuxinhu@google.com diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index d634c58c53..9ba80c0c0f 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -316,7 +316,6 @@ filegroup { cc_defaults { name: "libgui_bufferqueue-defaults", - clang: true, cflags: [ "-Wall", "-Werror", diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index e58543a245..fc68ad27c5 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -15,7 +15,6 @@ cc_test { name: "libgui_test", test_suites: ["device-tests"], - clang: true, cflags: [ "-Wall", "-Werror", @@ -75,7 +74,6 @@ cc_test { name: "libgui_multilib_test", test_suites: ["device-tests"], - clang: true, cflags: [ "-Wall", "-Werror", @@ -102,7 +100,6 @@ cc_test { name: "SurfaceParcelable_test", test_suites: ["device-tests"], - clang: true, cflags: [ "-Wall", "-Werror", @@ -131,7 +128,6 @@ cc_test { cc_test { name: "SamplingDemo", - clang: true, cflags: [ "-Wall", "-Werror", diff --git a/libs/input/Android.bp b/libs/input/Android.bp index 5d7874af77..de42b058e5 100644 --- a/libs/input/Android.bp +++ b/libs/input/Android.bp @@ -58,8 +58,6 @@ cc_library { "VirtualKeyMap.cpp", ], - clang: true, - header_libs: ["jni_headers"], export_header_lib_headers: ["jni_headers"], diff --git a/libs/nativedisplay/Android.bp b/libs/nativedisplay/Android.bp index ed728dcb45..4659b96b11 100644 --- a/libs/nativedisplay/Android.bp +++ b/libs/nativedisplay/Android.bp @@ -33,7 +33,7 @@ license { cc_library_headers { name: "libnativedisplay_headers", - export_include_dirs: ["include",], + export_include_dirs: ["include"], } cc_library_shared { @@ -43,8 +43,6 @@ cc_library_shared { "include-private", ], - clang: true, - cflags: [ "-Wall", "-Werror", diff --git a/libs/nativewindow/Android.bp b/libs/nativewindow/Android.bp index d30efa1851..dd07319791 100644 --- a/libs/nativewindow/Android.bp +++ b/libs/nativewindow/Android.bp @@ -77,8 +77,6 @@ cc_library { "include-private", ], - clang: true, - cflags: [ "-Wall", "-Werror", diff --git a/libs/renderengine/Android.bp b/libs/renderengine/Android.bp index cb92df388b..f6f57dde7d 100644 --- a/libs/renderengine/Android.bp +++ b/libs/renderengine/Android.bp @@ -111,7 +111,6 @@ cc_library_static { name: "librenderengine", defaults: ["librenderengine_defaults"], double_loadable: true, - clang: true, cflags: [ "-fvisibility=hidden", "-Werror=format", diff --git a/libs/sensor/Android.bp b/libs/sensor/Android.bp index edd453a936..2b93c6e85e 100644 --- a/libs/sensor/Android.bp +++ b/libs/sensor/Android.bp @@ -24,7 +24,6 @@ package { cc_library_shared { name: "libsensor", - clang: true, cflags: [ "-Wall", "-Werror", @@ -53,5 +52,9 @@ cc_library_shared { export_include_dirs: ["include"], - export_shared_lib_headers: ["libbinder", "libpermission", "libhardware"], + export_shared_lib_headers: [ + "libbinder", + "libpermission", + "libhardware", + ], } diff --git a/libs/sensor/tests/Android.bp b/libs/sensor/tests/Android.bp index 8fdb003a5d..ac4be44a4a 100644 --- a/libs/sensor/tests/Android.bp +++ b/libs/sensor/tests/Android.bp @@ -24,9 +24,10 @@ package { cc_test { name: "libsensor_test", - clang: true, - - cflags: ["-Wall", "-Werror"], + cflags: [ + "-Wall", + "-Werror", + ], srcs: [ "Sensor_test.cpp", diff --git a/libs/ui/Android.bp b/libs/ui/Android.bp index d138495036..0f771a9070 100644 --- a/libs/ui/Android.bp +++ b/libs/ui/Android.bp @@ -36,7 +36,6 @@ cc_library_headers { cc_defaults { name: "libui-defaults", - clang: true, cflags: [ "-Wall", "-Werror", @@ -111,7 +110,6 @@ cc_library_shared { }, double_loadable: true, - clang: true, cflags: [ "-Wall", "-Werror", diff --git a/libs/ui/tools/Android.bp b/libs/ui/tools/Android.bp index c28c303c0c..5d6070cf95 100644 --- a/libs/ui/tools/Android.bp +++ b/libs/ui/tools/Android.bp @@ -25,7 +25,7 @@ package { cc_defaults { name: "libui_tools_default", - clang_cflags: [ + cflags: [ "-Wall", "-Wextra", "-Werror", diff --git a/libs/vr/libbroadcastring/Android.bp b/libs/vr/libbroadcastring/Android.bp index fa449aeb55..e72ca74ba7 100644 --- a/libs/vr/libbroadcastring/Android.bp +++ b/libs/vr/libbroadcastring/Android.bp @@ -26,7 +26,6 @@ cc_library_static { cc_test { name: "broadcast_ring_tests", - clang: true, cflags: [ "-Wall", "-Wextra", diff --git a/libs/vr/libpdx/Android.bp b/libs/vr/libpdx/Android.bp index c1f6da3b10..c95603bcf7 100644 --- a/libs/vr/libpdx/Android.bp +++ b/libs/vr/libpdx/Android.bp @@ -16,7 +16,6 @@ cc_library_headers { cc_library_static { name: "libpdx", - clang: true, cflags: [ "-Wall", "-Wextra", @@ -42,7 +41,6 @@ cc_library_static { cc_test { name: "pdx_tests", - clang: true, cflags: [ "-Wall", "-Wextra", @@ -72,7 +70,6 @@ cc_test { // Code analysis target. cc_test { name: "pdx_encoder_performance_test", - clang: true, cflags: [ "-Wall", "-Wextra", diff --git a/libs/vr/libpdx/fuzz/Android.bp b/libs/vr/libpdx/fuzz/Android.bp index cc32b1822b..ac831ceda0 100644 --- a/libs/vr/libpdx/fuzz/Android.bp +++ b/libs/vr/libpdx/fuzz/Android.bp @@ -9,7 +9,6 @@ package { cc_fuzz { name: "libpdx_service_dispatcher_fuzzer", - clang: true, srcs: [ "service_dispatcher_fuzzer.cpp", ], @@ -24,13 +23,12 @@ cc_fuzz { shared_libs: [ "libutils", "liblog", - "libcutils" + "libcutils", ], } cc_fuzz { name: "libpdx_message_fuzzer", - clang: true, srcs: [ "message_fuzzer.cpp", ], @@ -45,13 +43,12 @@ cc_fuzz { shared_libs: [ "libutils", "liblog", - "libcutils" + "libcutils", ], } cc_fuzz { name: "libpdx_serialization_fuzzer", - clang: true, srcs: [ "serialization_fuzzer.cpp", ], @@ -66,6 +63,6 @@ cc_fuzz { shared_libs: [ "libutils", "liblog", - "libcutils" + "libcutils", ], } diff --git a/libs/vr/libpdx_default_transport/Android.bp b/libs/vr/libpdx_default_transport/Android.bp index 804685747e..a5758b589f 100644 --- a/libs/vr/libpdx_default_transport/Android.bp +++ b/libs/vr/libpdx_default_transport/Android.bp @@ -9,7 +9,6 @@ package { cc_defaults { name: "pdx_default_transport_compiler_defaults", - clang: true, cflags: [ "-Wall", "-Wextra", @@ -26,7 +25,10 @@ cc_defaults { cc_defaults { name: "pdx_use_transport_servicefs", export_include_dirs: ["private/servicefs"], - whole_static_libs: ["libpdx_servicefs", "libservicefs"], + whole_static_libs: [ + "libpdx_servicefs", + "libservicefs", + ], } cc_defaults { diff --git a/libs/vr/libpdx_uds/Android.bp b/libs/vr/libpdx_uds/Android.bp index 216ca9f236..7f88dafc81 100644 --- a/libs/vr/libpdx_uds/Android.bp +++ b/libs/vr/libpdx_uds/Android.bp @@ -9,7 +9,6 @@ package { cc_library_static { name: "libpdx_uds", - clang: true, cflags: [ "-Wall", "-Wextra", @@ -41,7 +40,6 @@ cc_library_static { cc_test { name: "libpdx_uds_tests", - clang: true, cflags: [ "-Wall", "-Wextra", diff --git a/opengl/OWNERS b/opengl/OWNERS index a47fb9a573..379f7638f0 100644 --- a/opengl/OWNERS +++ b/opengl/OWNERS @@ -6,7 +6,6 @@ ianelliott@google.com jessehall@google.com lfy@google.com lpy@google.com -timvp@google.com romanl@google.com vantablack@google.com yuxinhu@google.com diff --git a/opengl/libs/Android.bp b/opengl/libs/Android.bp index c9fce8ad52..c1e935aab0 100644 --- a/opengl/libs/Android.bp +++ b/opengl/libs/Android.bp @@ -108,7 +108,6 @@ cc_defaults { // In particular, DO NOT add libutils nor anything "above" libui "libgraphicsenv", "libnativewindow", - "libbacktrace", "libbase", ], } @@ -165,6 +164,7 @@ cc_library_shared { "libnativeloader_lazy", "libutils", "libSurfaceFlingerProp", + "libunwindstack", ], static_libs: [ "libEGL_getProcAddress", diff --git a/opengl/libs/EGL/CallStack.h b/opengl/libs/EGL/CallStack.h index b7fdf97cbe..96437c37d9 100644 --- a/opengl/libs/EGL/CallStack.h +++ b/opengl/libs/EGL/CallStack.h @@ -16,8 +16,8 @@ #pragma once -#include <backtrace/Backtrace.h> #include <log/log.h> +#include <unwindstack/AndroidUnwinder.h> #include <memory> @@ -26,12 +26,15 @@ public: // Create a callstack with the current thread's stack trace. // Immediately dump it to logcat using the given logtag. static void log(const char* logtag) noexcept { - std::unique_ptr<Backtrace> backtrace( - Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD)); - if (backtrace->Unwind(2)) { - for (size_t i = 0, c = backtrace->NumFrames(); i < c; i++) { + unwindstack::AndroidLocalUnwinder unwinder; + unwindstack::AndroidUnwinderData data; + if (unwinder.Unwind(data)) { + for (size_t i = 2, c = data.frames.size(); i < c; i++) { + auto& frame = data.frames[i]; + // Trim the first two frames. + frame.num -= 2; __android_log_print(ANDROID_LOG_DEBUG, logtag, "%s", - backtrace->FormatFrameData(i).c_str()); + unwinder.FormatFrame(frame).c_str()); } } } diff --git a/opengl/tests/gl2_cameraeye/AndroidManifest.xml b/opengl/tests/gl2_cameraeye/AndroidManifest.xml index c53f7be0b0..a4674e129d 100644 --- a/opengl/tests/gl2_cameraeye/AndroidManifest.xml +++ b/opengl/tests/gl2_cameraeye/AndroidManifest.xml @@ -26,7 +26,7 @@ <uses-feature android:name="android.hardware.camera.autofocus" /> <uses-feature android:glEsVersion="0x00020000" /> <application android:label="@string/gl2cameraeye_name"> - <activity android:name="GL2CameraEye"> + <activity android:name="GL2CameraEye" android:exported="true"> <intent-filter> <action android:name="android.intent.action.MAIN"/> <category android:name="android.intent.category.LAUNCHER"/> diff --git a/opengl/tests/gl2_java/AndroidManifest.xml b/opengl/tests/gl2_java/AndroidManifest.xml index 8bb6840a16..500adb5c14 100644 --- a/opengl/tests/gl2_java/AndroidManifest.xml +++ b/opengl/tests/gl2_java/AndroidManifest.xml @@ -22,7 +22,8 @@ <activity android:name="GL2JavaActivity" android:theme="@android:style/Theme.NoTitleBar.Fullscreen" android:launchMode="singleTask" - android:configChanges="orientation|keyboardHidden"> + android:configChanges="orientation|keyboardHidden" + android:exported="true"> <intent-filter> <action android:name="android.intent.action.MAIN" /> <category android:name="android.intent.category.LAUNCHER" /> diff --git a/opengl/tests/gl2_jni/AndroidManifest.xml b/opengl/tests/gl2_jni/AndroidManifest.xml index 1827e5f377..b4ce99b102 100644 --- a/opengl/tests/gl2_jni/AndroidManifest.xml +++ b/opengl/tests/gl2_jni/AndroidManifest.xml @@ -21,7 +21,8 @@ <activity android:name="GL2JNIActivity" android:theme="@android:style/Theme.NoTitleBar.Fullscreen" android:launchMode="singleTask" - android:configChanges="orientation|keyboardHidden"> + android:configChanges="orientation|keyboardHidden" + android:exported="true"> <intent-filter> <action android:name="android.intent.action.MAIN" /> <category android:name="android.intent.category.LAUNCHER" /> diff --git a/opengl/tests/gl_jni/AndroidManifest.xml b/opengl/tests/gl_jni/AndroidManifest.xml index 5d0ec966f4..bedab56659 100644 --- a/opengl/tests/gl_jni/AndroidManifest.xml +++ b/opengl/tests/gl_jni/AndroidManifest.xml @@ -24,7 +24,8 @@ android:theme="@android:style/Theme.NoTitleBar.Fullscreen" android:launchMode="singleTask" android:screenOrientation="landscape" - android:configChanges="orientation|keyboardHidden"> + android:configChanges="orientation|keyboardHidden" + android:exported="true"> <intent-filter> <action android:name="android.intent.action.MAIN" /> <category android:name="android.intent.category.LAUNCHER" /> diff --git a/opengl/tests/lighting1709/AndroidManifest.xml b/opengl/tests/lighting1709/AndroidManifest.xml index 6c23d422f5..d766be9ed5 100644 --- a/opengl/tests/lighting1709/AndroidManifest.xml +++ b/opengl/tests/lighting1709/AndroidManifest.xml @@ -2,7 +2,7 @@ package="com.android.lightingtest"> <application> - <activity android:name="ClearActivity" android:label="LightingTest"> + <activity android:name="ClearActivity" android:label="LightingTest" android:exported="true"> <intent-filter> <action android:name="android.intent.action.MAIN" /> <category android:name="android.intent.category.DEFAULT" /> diff --git a/opengl/tests/testPauseResume/AndroidManifest.xml b/opengl/tests/testPauseResume/AndroidManifest.xml index 1879bc3217..ae82a8286a 100644 --- a/opengl/tests/testPauseResume/AndroidManifest.xml +++ b/opengl/tests/testPauseResume/AndroidManifest.xml @@ -24,7 +24,8 @@ android:theme="@android:style/Theme.NoTitleBar.Fullscreen" android:launchMode="singleTask" android:screenOrientation="landscape" - android:configChanges="orientation|keyboardHidden"> + android:configChanges="orientation|keyboardHidden" + android:exported="true"> <intent-filter> <action android:name="android.intent.action.MAIN" /> <category android:name="android.intent.category.LAUNCHER" /> diff --git a/services/gpuservice/tests/unittests/Android.bp b/services/gpuservice/tests/unittests/Android.bp index 4fb0d2e734..86f6c7febb 100644 --- a/services/gpuservice/tests/unittests/Android.bp +++ b/services/gpuservice/tests/unittests/Android.bp @@ -35,6 +35,7 @@ cc_test { header_libs: ["bpf_headers"], shared_libs: [ "libbase", + "libbinder", "libbpf_bcc", "libcutils", "libgfxstats", diff --git a/services/gpuservice/tests/unittests/GpuStatsTest.cpp b/services/gpuservice/tests/unittests/GpuStatsTest.cpp index 20c8ccf9bf..0baf1f93da 100644 --- a/services/gpuservice/tests/unittests/GpuStatsTest.cpp +++ b/services/gpuservice/tests/unittests/GpuStatsTest.cpp @@ -18,6 +18,7 @@ #define LOG_TAG "gpuservice_unittest" #include <unistd.h> +#include <binder/ProcessState.h> #include <cutils/properties.h> #include <gmock/gmock.h> #include <gpustats/GpuStats.h> @@ -79,6 +80,10 @@ public: void SetUp() override { mCpuVulkanVersion = property_get_int32("ro.cpuvulkan.version", 0); mGlesVersion = property_get_int32("ro.opengles.version", 0); + + // start the thread pool + sp<ProcessState> ps(ProcessState::self()); + ps->startThreadPool(); } std::unique_ptr<GpuStats> mGpuStats = std::make_unique<GpuStats>(); diff --git a/services/utils/tests/Android.bp b/services/utils/tests/Android.bp index 54cf5b7404..cfa8a08c66 100644 --- a/services/utils/tests/Android.bp +++ b/services/utils/tests/Android.bp @@ -34,5 +34,4 @@ cc_test { "libgmock", "libserviceutils", ], - clang: true, } diff --git a/vulkan/libvulkan/Android.bp b/vulkan/libvulkan/Android.bp index 440c5b144a..5719b5cf61 100644 --- a/vulkan/libvulkan/Android.bp +++ b/vulkan/libvulkan/Android.bp @@ -37,7 +37,6 @@ cc_library_shared { "vulkan_headers", ], }, - clang: true, sanitize: { misc_undefined: ["integer"], }, diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp index c4b1487267..48d6fd0bd5 100644 --- a/vulkan/libvulkan/swapchain.cpp +++ b/vulkan/libvulkan/swapchain.cpp @@ -939,8 +939,7 @@ VkResult GetPhysicalDeviceSurfacePresentModesKHR(VkPhysicalDevice pdev, // VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR and // VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR. We technically cannot // know if VK_PRESENT_MODE_SHARED_MAILBOX_KHR is supported without a - // surface, and that cannot be relied upon. - present_modes.push_back(VK_PRESENT_MODE_MAILBOX_KHR); + // surface, and that cannot be relied upon. Therefore, don't return it. present_modes.push_back(VK_PRESENT_MODE_FIFO_KHR); } else { ANativeWindow* window = SurfaceFromHandle(surface)->window.get(); diff --git a/vulkan/nulldrv/Android.bp b/vulkan/nulldrv/Android.bp index 0daad9c634..a6d540bb93 100644 --- a/vulkan/nulldrv/Android.bp +++ b/vulkan/nulldrv/Android.bp @@ -27,7 +27,6 @@ cc_library_shared { proprietary: true, relative_install_path: "hw", - clang: true, cflags: [ "-fvisibility=hidden", "-fstrict-aliasing", diff --git a/vulkan/vkjson/Android.bp b/vulkan/vkjson/Android.bp index fa0258bc06..b6d3a0b45f 100644 --- a/vulkan/vkjson/Android.bp +++ b/vulkan/vkjson/Android.bp @@ -37,7 +37,6 @@ cc_library_shared { cc_library_static { name: "libvkjson_ndk", - clang: true, srcs: [ "vkjson.cc", "vkjson_instance.cc", |