diff options
68 files changed, 1345 insertions, 285 deletions
diff --git a/cmds/installd/tests/Android.bp b/cmds/installd/tests/Android.bp index 07f73b9029..61fe316225 100644 --- a/cmds/installd/tests/Android.bp +++ b/cmds/installd/tests/Android.bp @@ -77,10 +77,8 @@ cc_test { }, } -cc_test { - name: "installd_service_test", - test_suites: ["device-tests"], - srcs: ["installd_service_test.cpp"], +cc_defaults { + name: "installd_service_test_defaults", cflags: [ "-Wall", "-Werror", @@ -106,8 +104,6 @@ cc_test { "liblogwrap", "libc++fs", ], - test_config: "installd_service_test.xml", - product_variables: { arc: { exclude_srcs: [ @@ -125,6 +121,14 @@ cc_test { } cc_test { + name: "installd_service_test", + test_suites: ["device-tests"], + srcs: ["installd_service_test.cpp"], + defaults: ["installd_service_test_defaults"], + test_config: "installd_service_test.xml", +} + +cc_test { name: "installd_dexopt_test", test_suites: ["device-tests"], srcs: ["installd_dexopt_test.cpp"], @@ -209,3 +213,19 @@ cc_test { "liblog", ], } + +cc_fuzz { + name: "installd_service_fuzzer", + defaults: [ + "service_fuzzer_defaults", + "fuzzer_disable_leaks", + "installd_service_test_defaults", + ], + srcs: ["fuzzers/InstalldServiceFuzzer.cpp"], + fuzz_config: { + cc: [ + "android-package-manager-team@google.com", + ], + triage_assignee: "waghpawan@google.com", + }, +} diff --git a/cmds/installd/tests/fuzzers/InstalldServiceFuzzer.cpp b/cmds/installd/tests/fuzzers/InstalldServiceFuzzer.cpp new file mode 100644 index 0000000000..b1c6940207 --- /dev/null +++ b/cmds/installd/tests/fuzzers/InstalldServiceFuzzer.cpp @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include <fuzzbinder/libbinder_driver.h> + +#include "InstalldNativeService.h" +#include "dexopt.h" + +using ::android::fuzzService; +using ::android::sp; +using ::android::installd::InstalldNativeService; + +namespace android { +namespace installd { + +bool calculate_oat_file_path(char path[PKG_PATH_MAX], const char* oat_dir, const char* apk_path, + const char* instruction_set) { + return calculate_oat_file_path_default(path, oat_dir, apk_path, instruction_set); +} + +bool calculate_odex_file_path(char path[PKG_PATH_MAX], const char* apk_path, + const char* instruction_set) { + return calculate_odex_file_path_default(path, apk_path, instruction_set); +} + +bool create_cache_path(char path[PKG_PATH_MAX], const char* src, const char* instruction_set) { + return create_cache_path_default(path, src, instruction_set); +} + +bool force_compile_without_image() { + return false; +} + +} // namespace installd +} // namespace android + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + auto service = sp<InstalldNativeService>::make(); + fuzzService(service, FuzzedDataProvider(data, size)); + return 0; +}
\ No newline at end of file diff --git a/cmds/lshal/libprocpartition/Android.bp b/cmds/lshal/libprocpartition/Android.bp index af85666276..d0e4b74543 100644 --- a/cmds/lshal/libprocpartition/Android.bp +++ b/cmds/lshal/libprocpartition/Android.bp @@ -37,4 +37,8 @@ cc_library_static { "include", ], min_sdk_version: "30", + apex_available: [ + "//apex_available:platform", + "com.android.neuralnetworks", + ], } diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index 4472db10c0..98a70ed973 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -301,7 +301,7 @@ sp<IBinder> ServiceManager::tryGetService(const std::string& name, bool startIfN } if (!out && startIfNotFound) { - tryStartService(name); + tryStartService(ctx, name); } if (out) { @@ -651,10 +651,11 @@ void ServiceManager::binderDied(const wp<IBinder>& who) { } } -void ServiceManager::tryStartService(const std::string& name) { - ALOGI("Since '%s' could not be found, trying to start it as a lazy AIDL service. (if it's not " - "configured to be a lazy service, it may be stuck starting or still starting).", - name.c_str()); +void ServiceManager::tryStartService(const Access::CallingContext& ctx, const std::string& name) { + ALOGI("Since '%s' could not be found (requested by debug pid %d), trying to start it as a lazy " + "AIDL service. (if it's not configured to be a lazy service, it may be stuck starting or " + "still starting).", + name.c_str(), ctx.debugPid); std::thread([=] { if (!base::SetProperty("ctl.interface_start", "aidl/" + name)) { diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h index 3aa6731eb3..3b925a48cb 100644 --- a/cmds/servicemanager/ServiceManager.h +++ b/cmds/servicemanager/ServiceManager.h @@ -67,7 +67,7 @@ public: void clear(); protected: - virtual void tryStartService(const std::string& name); + virtual void tryStartService(const Access::CallingContext& ctx, const std::string& name); private: struct Service { diff --git a/cmds/servicemanager/main.cpp b/cmds/servicemanager/main.cpp index c1a04dde89..86a45e61ea 100644 --- a/cmds/servicemanager/main.cpp +++ b/cmds/servicemanager/main.cpp @@ -131,7 +131,9 @@ int main(int argc, char** argv) { } IPCThreadState::self()->setTheContextObject(manager); - ps->becomeContextManager(); + if (!ps->becomeContextManager()) { + LOG(FATAL) << "Could not become context manager"; + } sp<Looper> looper = Looper::prepare(false /*allowNonCallbacks*/); diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp index 0fd8d8ee2a..9e5b8a445a 100644 --- a/cmds/servicemanager/test_sm.cpp +++ b/cmds/servicemanager/test_sm.cpp @@ -27,11 +27,14 @@ #include "Access.h" #include "ServiceManager.h" -using android::sp; using android::Access; using android::BBinder; using android::IBinder; using android::ServiceManager; +using android::sp; +using android::base::EndsWith; +using android::base::GetProperty; +using android::base::StartsWith; using android::binder::Status; using android::os::BnServiceCallback; using android::os::IServiceManager; @@ -62,7 +65,7 @@ public: class MockServiceManager : public ServiceManager { public: MockServiceManager(std::unique_ptr<Access>&& access) : ServiceManager(std::move(access)) {} - MOCK_METHOD1(tryStartService, void(const std::string& name)); + MOCK_METHOD2(tryStartService, void(const Access::CallingContext&, const std::string& name)); }; static sp<ServiceManager> getPermissiveServiceManager() { @@ -77,9 +80,11 @@ static sp<ServiceManager> getPermissiveServiceManager() { return sm; } -static bool isCuttlefish() { - return android::base::StartsWith(android::base::GetProperty("ro.product.vendor.device", ""), - "vsoc_"); +// Determines if test device is a cuttlefish phone device +static bool isCuttlefishPhone() { + auto device = GetProperty("ro.product.vendor.device", ""); + auto product = GetProperty("ro.product.vendor.name", ""); + return StartsWith(device, "vsoc_") && EndsWith(product, "_phone"); } TEST(AddService, HappyHappy) { @@ -314,7 +319,7 @@ TEST(ListServices, CriticalServices) { } TEST(Vintf, UpdatableViaApex) { - if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices"; + if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices"; auto sm = getPermissiveServiceManager(); std::optional<std::string> updatableViaApex; @@ -326,7 +331,7 @@ TEST(Vintf, UpdatableViaApex) { } TEST(Vintf, UpdatableViaApex_InvalidNameReturnsNullOpt) { - if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices"; + if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices"; auto sm = getPermissiveServiceManager(); std::optional<std::string> updatableViaApex; @@ -337,7 +342,7 @@ TEST(Vintf, UpdatableViaApex_InvalidNameReturnsNullOpt) { } TEST(Vintf, GetUpdatableNames) { - if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices"; + if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices"; auto sm = getPermissiveServiceManager(); std::vector<std::string> names; @@ -348,7 +353,7 @@ TEST(Vintf, GetUpdatableNames) { } TEST(Vintf, GetUpdatableNames_InvalidApexNameReturnsEmpty) { - if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices"; + if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices"; auto sm = getPermissiveServiceManager(); std::vector<std::string> names; diff --git a/libs/arect/Android.bp b/libs/arect/Android.bp index 5e539f24e1..1a9766d72e 100644 --- a/libs/arect/Android.bp +++ b/libs/arect/Android.bp @@ -72,6 +72,7 @@ cc_library_static { "//apex_available:platform", "com.android.media", "com.android.media.swcodec", + "com.android.neuralnetworks", ], } diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index a4f2b070c4..34331381db 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -281,14 +281,6 @@ cc_defaults { cflags: [ "-DBINDER_WITH_KERNEL_IPC", ], - arch: { - // TODO(b/254713216): undefined symbol in BufferedTextOutput::getBuffer - riscv64: { - lto: { - thin: false, - }, - }, - }, } cc_library { @@ -550,6 +542,7 @@ cc_library { ":__subpackages__", "//packages/modules/Virtualization/javalib/jni", "//packages/modules/Virtualization/vm_payload", + "//packages/modules/Virtualization/demo_native", "//device/google/cuttlefish/shared/minidroid:__subpackages__", "//system/software_defined_vehicle:__subpackages__", ], diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index 3e49656575..0f4a6cabde 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -58,15 +58,15 @@ static_assert(sizeof(BBinder) == 20); // global b/c b/230079120 - consistent symbol table #ifdef BINDER_RPC_DEV_SERVERS -bool kEnableRpcDevServers = true; +constexpr bool kEnableRpcDevServers = true; #else -bool kEnableRpcDevServers = false; +constexpr bool kEnableRpcDevServers = false; #endif #ifdef BINDER_ENABLE_RECORDING -bool kEnableRecording = true; +constexpr bool kEnableRecording = true; #else -bool kEnableRecording = false; +constexpr bool kEnableRecording = false; #endif // Log any reply transactions for which the data exceeds this size diff --git a/libs/binder/MemoryHeapBase.cpp b/libs/binder/MemoryHeapBase.cpp index 8fe1d2bb3d..3da06ba4db 100644 --- a/libs/binder/MemoryHeapBase.cpp +++ b/libs/binder/MemoryHeapBase.cpp @@ -78,7 +78,7 @@ MemoryHeapBase::MemoryHeapBase(size_t size, uint32_t flags, char const * name) 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)); - munmap(mBase, mSize); + if (mNeedUnmap) munmap(mBase, mSize); mBase = nullptr; mSize = 0; close(fd); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 0aca163eab..2c2a1b636e 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -947,7 +947,10 @@ bool Parcel::enforceInterface(const char16_t* interface, threadState->setCallingWorkSourceUidWithoutPropagation(workSource); // vendor header int32_t header = readInt32(); - if (header != kHeader) { + + // fuzzers skip this check, because it is for protecting the underlying ABI, but + // we don't want it to reduce our coverage + if (header != kHeader && !mServiceFuzzing) { ALOGE("Expecting header 0x%x but found 0x%x. Mixing copies of libbinder?", kHeader, header); return false; @@ -966,10 +969,18 @@ bool Parcel::enforceInterface(const char16_t* interface, (!len || !memcmp(parcel_interface, interface, len * sizeof (char16_t)))) { return true; } else { - ALOGW("**** enforceInterface() expected '%s' but read '%s'", - String8(interface, len).string(), - String8(parcel_interface, parcel_interface_len).string()); - return false; + if (mServiceFuzzing) { + // ignore. Theoretically, this could cause a few false positives, because + // people could assume things about getInterfaceDescriptor if they pass + // this point, but it would be extremely fragile. It's more important that + // we fuzz with the above things read from the Parcel. + return true; + } else { + ALOGW("**** enforceInterface() expected '%s' but read '%s'", + String8(interface, len).string(), + String8(parcel_interface, parcel_interface_len).string()); + return false; + } } } @@ -977,6 +988,10 @@ void Parcel::setEnforceNoDataAvail(bool enforceNoDataAvail) { mEnforceNoDataAvail = enforceNoDataAvail; } +void Parcel::setServiceFuzzing() { + mServiceFuzzing = true; +} + binder::Status Parcel::enforceNoDataAvail() const { if (!mEnforceNoDataAvail) { return binder::Status::ok(); @@ -1722,7 +1737,9 @@ status_t Parcel::validateReadData(size_t upperBound) const do { 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); + if (!mServiceFuzzing) { + ALOGE("Attempt to read from protected data in Parcel %p", this); + } return PERMISSION_DENIED; } nextObject++; @@ -2092,7 +2109,11 @@ String8 Parcel::readString8() const size_t len; const char* str = readString8Inplace(&len); if (str) return String8(str, len); - ALOGE("Reading a NULL string not supported here."); + + if (!mServiceFuzzing) { + ALOGE("Reading a NULL string not supported here."); + } + return String8(); } @@ -2132,7 +2153,11 @@ String16 Parcel::readString16() const size_t len; const char16_t* str = readString16Inplace(&len); if (str) return String16(str, len); - ALOGE("Reading a NULL string not supported here."); + + if (!mServiceFuzzing) { + ALOGE("Reading a NULL string not supported here."); + } + return String16(); } @@ -2172,7 +2197,9 @@ status_t Parcel::readStrongBinder(sp<IBinder>* val) const { status_t status = readNullableStrongBinder(val); if (status == OK && !val->get()) { - ALOGW("Expecting binder but got null!"); + if (!mServiceFuzzing) { + ALOGW("Expecting binder but got null!"); + } status = UNEXPECTED_NULL; } return status; @@ -2237,9 +2264,11 @@ int Parcel::readFileDescriptor() const { if (const auto* rpcFields = maybeRpcFields()) { if (!std::binary_search(rpcFields->mObjectPositions.begin(), rpcFields->mObjectPositions.end(), mDataPos)) { - ALOGW("Attempt to read file descriptor from Parcel %p at offset %zu that is not in the " - "object list", - this, mDataPos); + if (!mServiceFuzzing) { + ALOGW("Attempt to read file descriptor from Parcel %p at offset %zu that is not in " + "the object list", + this, mDataPos); + } return BAD_TYPE; } @@ -2497,8 +2526,11 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const return obj; } } - ALOGW("Attempt to read object from Parcel %p at offset %zu that is not in the object list", - this, DPOS); + if (!mServiceFuzzing) { + ALOGW("Attempt to read object from Parcel %p at offset %zu that is not in the object " + "list", + this, DPOS); + } } return nullptr; } @@ -3093,6 +3125,7 @@ void Parcel::initState() mDeallocZero = false; mOwner = nullptr; mEnforceNoDataAvail = true; + mServiceFuzzing = false; } void Parcel::scanForFds() const { diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp index 9282856f5c..55fc16de45 100644 --- a/libs/binder/RpcServer.cpp +++ b/libs/binder/RpcServer.cpp @@ -81,6 +81,7 @@ status_t RpcServer::setupInetServer(const char* address, unsigned int port, auto aiStart = InetSocketAddress::getAddrInfo(address, port); if (aiStart == nullptr) return UNKNOWN_ERROR; for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) { + if (ai->ai_addr == nullptr) continue; InetSocketAddress socketAddress(ai->ai_addr, ai->ai_addrlen, address, port); if (status_t status = setupSocketServer(socketAddress); status != OK) { continue; @@ -123,8 +124,13 @@ size_t RpcServer::getMaxThreads() { return mMaxThreads; } -void RpcServer::setProtocolVersion(uint32_t version) { +bool RpcServer::setProtocolVersion(uint32_t version) { + if (!RpcState::validateProtocolVersion(version)) { + return false; + } + mProtocolVersion = version; + return true; } void RpcServer::setSupportedFileDescriptorTransportModes( @@ -148,7 +154,7 @@ void RpcServer::setRootObjectWeak(const wp<IBinder>& binder) { mRootObjectWeak = binder; } void RpcServer::setPerSessionRootObject( - std::function<sp<IBinder>(const void*, size_t)>&& makeObject) { + std::function<sp<IBinder>(wp<RpcSession> session, const void*, size_t)>&& makeObject) { RpcMutexLockGuard _l(mLock); mRootObject.clear(); mRootObjectWeak.clear(); @@ -161,6 +167,12 @@ void RpcServer::setConnectionFilter(std::function<bool(const void*, size_t)>&& f mConnectionFilter = std::move(filter); } +void RpcServer::setServerSocketModifier(std::function<void(base::borrowed_fd)>&& modifier) { + RpcMutexLockGuard _l(mLock); + LOG_ALWAYS_FATAL_IF(mServer.fd != -1, "Already started"); + mServerSocketModifier = std::move(modifier); +} + sp<IBinder> RpcServer::getRootObject() { RpcMutexLockGuard _l(mLock); bool hasWeak = mRootObjectWeak.unsafe_get(); @@ -335,6 +347,8 @@ bool RpcServer::shutdown() { mJoinThread.reset(); } + mServer = RpcTransportFd(); + LOG_RPC_DETAIL("Finished waiting on shutdown."); mShutdownTrigger = nullptr; @@ -501,7 +515,8 @@ void RpcServer::establishConnection( // if null, falls back to server root sp<IBinder> sessionSpecificRoot; if (server->mRootObjectFactory != nullptr) { - sessionSpecificRoot = server->mRootObjectFactory(addr.data(), addrLen); + sessionSpecificRoot = + server->mRootObjectFactory(wp<RpcSession>(session), addr.data(), addrLen); if (sessionSpecificRoot == nullptr) { ALOGE("Warning: server returned null from root object factory"); } @@ -556,6 +571,14 @@ status_t RpcServer::setupSocketServer(const RpcSocketAddress& addr) { ALOGE("Could not create socket at %s: %s", addr.toString().c_str(), strerror(savedErrno)); return -savedErrno; } + + { + RpcMutexLockGuard _l(mLock); + if (mServerSocketModifier != nullptr) { + mServerSocketModifier(socket_fd); + } + } + if (0 != TEMP_FAILURE_RETRY(bind(socket_fd.get(), addr.addr(), addr.addrSize()))) { int savedErrno = errno; ALOGE("Could not bind socket at %s: %s", addr.toString().c_str(), strerror(savedErrno)); diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp index fbad0f7756..c3dee1650e 100644 --- a/libs/binder/RpcSession.cpp +++ b/libs/binder/RpcSession.cpp @@ -104,11 +104,7 @@ size_t RpcSession::getMaxOutgoingThreads() { } bool RpcSession::setProtocolVersionInternal(uint32_t version, bool checkStarted) { - if (version >= RPC_WIRE_PROTOCOL_VERSION_NEXT && - version != RPC_WIRE_PROTOCOL_VERSION_EXPERIMENTAL) { - ALOGE("Cannot start RPC session with version %u which is unknown (current protocol version " - "is %u).", - version, RPC_WIRE_PROTOCOL_VERSION); + if (!RpcState::validateProtocolVersion(version)) { return false; } diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp index 03fa69973d..5c1b2305a6 100644 --- a/libs/binder/RpcState.cpp +++ b/libs/binder/RpcState.cpp @@ -34,6 +34,10 @@ #include <inttypes.h> +#ifdef __ANDROID__ +#include <cutils/properties.h> +#endif + namespace android { using base::StringPrintf; @@ -398,6 +402,31 @@ status_t RpcState::rpcRec( return OK; } +bool RpcState::validateProtocolVersion(uint32_t version) { + if (version == RPC_WIRE_PROTOCOL_VERSION_EXPERIMENTAL) { +#if defined(__ANDROID__) + char codename[PROPERTY_VALUE_MAX]; + property_get("ro.build.version.codename", codename, ""); + if (!strcmp(codename, "REL")) { + ALOGE("Cannot use experimental RPC binder protocol on a release branch."); + return false; + } +#else + // don't restrict on other platforms, though experimental should + // only really be used for testing, we don't have a good way to see + // what is shipping outside of Android +#endif + } else if (version >= RPC_WIRE_PROTOCOL_VERSION_NEXT) { + ALOGE("Cannot use RPC binder protocol version %u which is unknown (current protocol " + "version " + "is %u).", + version, RPC_WIRE_PROTOCOL_VERSION); + return false; + } + + return true; +} + status_t RpcState::readNewSessionResponse(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, uint32_t* version) { RpcNewSessionResponse response; diff --git a/libs/binder/RpcState.h b/libs/binder/RpcState.h index 0e23ea7515..1fe71a5a78 100644 --- a/libs/binder/RpcState.h +++ b/libs/binder/RpcState.h @@ -63,6 +63,8 @@ public: RpcState(); ~RpcState(); + [[nodiscard]] static bool validateProtocolVersion(uint32_t version); + [[nodiscard]] status_t readNewSessionResponse(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, uint32_t* version); [[nodiscard]] status_t sendConnectionInit(const sp<RpcSession::RpcConnection>& connection, diff --git a/libs/binder/RpcTransportRaw.cpp b/libs/binder/RpcTransportRaw.cpp index cd067bfee7..f3575cc7d8 100644 --- a/libs/binder/RpcTransportRaw.cpp +++ b/libs/binder/RpcTransportRaw.cpp @@ -29,8 +29,6 @@ namespace android { -namespace { - // RpcTransport with TLS disabled. class RpcTransportRaw : public RpcTransport { public: @@ -96,8 +94,6 @@ public: std::vector<uint8_t> getCertificate(RpcCertificateFormat) const override { return {}; } }; -} // namespace - std::unique_ptr<RpcTransportCtx> RpcTransportCtxFactoryRaw::newServerCtx() const { return std::make_unique<RpcTransportCtxRaw>(); } diff --git a/libs/binder/RpcTransportTipcAndroid.cpp b/libs/binder/RpcTransportTipcAndroid.cpp index d5a6da2e3d..0c81d83032 100644 --- a/libs/binder/RpcTransportTipcAndroid.cpp +++ b/libs/binder/RpcTransportTipcAndroid.cpp @@ -31,8 +31,6 @@ using android::base::Result; namespace android { -namespace { - // RpcTransport for writing Trusty IPC clients in Android. class RpcTransportTipcAndroid : public RpcTransport { public: @@ -217,8 +215,6 @@ public: std::vector<uint8_t> getCertificate(RpcCertificateFormat) const override { return {}; } }; -} // namespace - std::unique_ptr<RpcTransportCtx> RpcTransportCtxFactoryTipcAndroid::newServerCtx() const { return std::make_unique<RpcTransportCtxTipcAndroid>(); } diff --git a/libs/binder/RpcTransportTls.cpp b/libs/binder/RpcTransportTls.cpp index 3e98ecca9b..785f6ce2ce 100644 --- a/libs/binder/RpcTransportTls.cpp +++ b/libs/binder/RpcTransportTls.cpp @@ -275,6 +275,8 @@ private: bssl::UniquePtr<SSL> mSsl; }; +} // namespace + class RpcTransportTls : public RpcTransport { public: RpcTransportTls(RpcTransportFd socket, Ssl ssl) @@ -411,7 +413,8 @@ status_t RpcTransportTls::interruptableReadFully( } // For |ssl|, set internal FD to |fd|, and do handshake. Handshake is triggerable by |fdTrigger|. -bool setFdAndDoHandshake(Ssl* ssl, const android::RpcTransportFd& socket, FdTrigger* fdTrigger) { +static bool setFdAndDoHandshake(Ssl* ssl, const android::RpcTransportFd& socket, + FdTrigger* fdTrigger) { bssl::UniquePtr<BIO> bio = newSocketBio(socket.fd); TEST_AND_RETURN(false, bio != nullptr); auto [_, errorQueue] = ssl->call(SSL_set_bio, bio.get(), bio.get()); @@ -540,8 +543,6 @@ protected: } }; -} // namespace - std::unique_ptr<RpcTransportCtx> RpcTransportCtxFactoryTls::newServerCtx() const { return android::RpcTransportCtxTls::create<RpcTransportCtxTlsServer>(mCertVerifier, mAuth.get()); diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING index 0e8e18747b..2b3ff4407a 100644 --- a/libs/binder/TEST_MAPPING +++ b/libs/binder/TEST_MAPPING @@ -16,9 +16,15 @@ "name": "binderDriverInterfaceTest" }, { + "name": "binderRecordReplayTest" + }, + { "name": "binderHostDeviceTest" }, { + "name": "binderParcelBenchmark" + }, + { "name": "binderTextOutputTest" }, { @@ -58,6 +64,9 @@ "name": "libbinderthreadstateutils_test" }, { + "name": "fuzz_service_test" + }, + { "name": "CtsOsTestCases", "options": [ { diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index e28d374b26..15bb325459 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -149,6 +149,10 @@ public: // This Api is used by fuzzers to skip dataAvail checks. void setEnforceNoDataAvail(bool enforceNoDataAvail); + // When fuzzing, we want to remove certain ABI checks that cause significant + // lost coverage, and we also want to avoid logs that cost too much to write. + void setServiceFuzzing(); + void freeData(); size_t objectsCount() const; @@ -1330,6 +1334,7 @@ private: // Set this to false to skip dataAvail checks. bool mEnforceNoDataAvail; + bool mServiceFuzzing; release_func mOwner; diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h index ce578e3f5c..81391e96e7 100644 --- a/libs/binder/include/binder/ProcessState.h +++ b/libs/binder/include/binder/ProcessState.h @@ -55,7 +55,7 @@ public: // For main functions - dangerous for libraries to use void startThreadPool(); - bool becomeContextManager(); + [[nodiscard]] bool becomeContextManager(); sp<IBinder> getStrongProxyForHandle(int32_t handle); void expungeHandle(int32_t handle, IBinder* binder); diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h index 1001b64ede..b804f7b92a 100644 --- a/libs/binder/include/binder/RpcServer.h +++ b/libs/binder/include/binder/RpcServer.h @@ -137,7 +137,7 @@ public: * used. However, this can be used in order to prevent newer protocol * versions from ever being used. This is expected to be useful for testing. */ - void setProtocolVersion(uint32_t version); + [[nodiscard]] bool setProtocolVersion(uint32_t version); /** * Set the supported transports for sending and receiving file descriptors. @@ -163,14 +163,18 @@ public: * Allows a root object to be created for each session. * * Takes one argument: a callable that is invoked once per new session. - * The callable takes two arguments: a type-erased pointer to an OS- and - * transport-specific address structure, e.g., sockaddr_vm for vsock, and - * an integer representing the size in bytes of that structure. The - * callable should validate the size, then cast the type-erased pointer - * to a pointer to the actual type of the address, e.g., const void* to - * const sockaddr_vm*. + * The callable takes three arguments: + * - a weak pointer to the session. If you want to hold onto this in the root object, then + * you should keep a weak pointer, and promote it when needed. For instance, if you refer + * to this from the root object, then you could get ahold of transport-specific information. + * - a type-erased pointer to an OS- and transport-specific address structure, e.g., + * sockaddr_vm for vsock + * - an integer representing the size in bytes of that structure. The callable should + * validate the size, then cast the type-erased pointer to a pointer to the actual type of the + * address, e.g., const void* to const sockaddr_vm*. */ - void setPerSessionRootObject(std::function<sp<IBinder>(const void*, size_t)>&& object); + void setPerSessionRootObject( + std::function<sp<IBinder>(wp<RpcSession> session, const void*, size_t)>&& object); sp<IBinder> getRootObject(); /** @@ -184,6 +188,13 @@ public: void setConnectionFilter(std::function<bool(const void*, size_t)>&& filter); /** + * Set optional modifier of each newly created server socket. + * + * The only argument is a successfully created file descriptor, not bound to an address yet. + */ + void setServerSocketModifier(std::function<void(base::borrowed_fd)>&& modifier); + + /** * See RpcTransportCtx::getCertificate */ std::vector<uint8_t> getCertificate(RpcCertificateFormat); @@ -265,8 +276,9 @@ private: sp<IBinder> mRootObject; wp<IBinder> mRootObjectWeak; - std::function<sp<IBinder>(const void*, size_t)> mRootObjectFactory; + std::function<sp<IBinder>(wp<RpcSession>, const void*, size_t)> mRootObjectFactory; std::function<bool(const void*, size_t)> mConnectionFilter; + std::function<void(base::borrowed_fd)> mServerSocketModifier; std::map<std::vector<uint8_t>, sp<RpcSession>> mSessions; std::unique_ptr<FdTrigger> mShutdownTrigger; RpcConditionVariable mShutdownCv; diff --git a/libs/binder/include/binder/RpcTransport.h b/libs/binder/include/binder/RpcTransport.h index fd52a3a1a9..6db9ad983c 100644 --- a/libs/binder/include/binder/RpcTransport.h +++ b/libs/binder/include/binder/RpcTransport.h @@ -39,6 +39,16 @@ namespace android { class FdTrigger; struct RpcTransportFd; +// for 'friend' +class RpcTransportRaw; +class RpcTransportTls; +class RpcTransportTipcAndroid; +class RpcTransportTipcTrusty; +class RpcTransportCtxRaw; +class RpcTransportCtxTls; +class RpcTransportCtxTipcAndroid; +class RpcTransportCtxTipcTrusty; + // Represents a socket connection. // No thread-safety is guaranteed for these APIs. class RpcTransport { @@ -92,7 +102,21 @@ public: */ [[nodiscard]] virtual bool isWaiting() = 0; -protected: +private: + // limit the classes which can implement RpcTransport. Being able to change this + // interface is important to allow development of RPC binder. In the past, we + // changed this interface to use iovec for efficiency, and we added FDs to the + // interface. If another transport is needed, it should be added directly here. + // non-socket FDs likely also need changes in RpcSession in order to get + // connected, and similarly to how addrinfo was type-erased from RPC binder + // interfaces when RpcTransportTipc* was added, other changes may be needed + // to add more transports. + + friend class ::android::RpcTransportRaw; + friend class ::android::RpcTransportTls; + friend class ::android::RpcTransportTipcAndroid; + friend class ::android::RpcTransportTipcTrusty; + RpcTransport() = default; }; @@ -117,7 +141,13 @@ public: [[nodiscard]] virtual std::vector<uint8_t> getCertificate( RpcCertificateFormat format) const = 0; -protected: +private: + // see comment on RpcTransport + friend class ::android::RpcTransportCtxRaw; + friend class ::android::RpcTransportCtxTls; + friend class ::android::RpcTransportCtxTipcAndroid; + friend class ::android::RpcTransportCtxTipcTrusty; + RpcTransportCtx() = default; }; @@ -140,7 +170,7 @@ protected: RpcTransportCtxFactory() = default; }; -struct RpcTransportFd { +struct RpcTransportFd final { private: mutable bool isPolling{false}; diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp index cefc42f25e..eba0556023 100644 --- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp +++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp @@ -107,11 +107,13 @@ class MyBinderNdkUnitTest : public aidl::BnBinderNdkUnitTest { } static bool activeServicesCallback(bool hasClients, void* context) { if (hasClients) { + LOG(INFO) << "hasClients, so not unregistering."; return false; } // Unregister all services if (!AServiceManager_tryUnregister()) { + LOG(INFO) << "Could not unregister service the first time."; // Prevent shutdown (test will fail) return false; } @@ -121,6 +123,7 @@ class MyBinderNdkUnitTest : public aidl::BnBinderNdkUnitTest { // Unregister again before shutdown if (!AServiceManager_tryUnregister()) { + LOG(INFO) << "Could not unregister service the second time."; // Prevent shutdown (test will fail) return false; } @@ -128,6 +131,7 @@ class MyBinderNdkUnitTest : public aidl::BnBinderNdkUnitTest { // Check if the context was passed correctly MyBinderNdkUnitTest* service = static_cast<MyBinderNdkUnitTest*>(context); if (service->contextTestValue != kContextTestValue) { + LOG(INFO) << "Incorrect context value."; // Prevent shutdown (test will fail) return false; } @@ -479,6 +483,8 @@ TEST(NdkBinder, ForcedPersistenceTest) { } TEST(NdkBinder, ActiveServicesCallbackTest) { + LOG(INFO) << "ActiveServicesCallbackTest starting"; + ndk::SpAIBinder binder(AServiceManager_waitForService(kActiveServicesNdkUnitTestService)); std::shared_ptr<aidl::IBinderNdkUnitTest> service = aidl::IBinderNdkUnitTest::fromBinder(binder); @@ -489,6 +495,7 @@ TEST(NdkBinder, ActiveServicesCallbackTest) { service = nullptr; IPCThreadState::self()->flushCommands(); + LOG(INFO) << "ActiveServicesCallbackTest about to sleep"; sleep(kShutdownWaitTime); ASSERT_FALSE(isServiceRunning(kActiveServicesNdkUnitTestService)) @@ -497,14 +504,28 @@ TEST(NdkBinder, ActiveServicesCallbackTest) { struct DeathRecipientCookie { std::function<void(void)>*onDeath, *onUnlink; + + // may contain additional data + // - if it contains AIBinder, then you must call AIBinder_unlinkToDeath manually, + // because it would form a strong reference cycle + // - if it points to a data member of another structure, this should have a weak + // promotable reference or a strong reference, in case that object is deleted + // while the death recipient is firing }; void LambdaOnDeath(void* cookie) { auto funcs = static_cast<DeathRecipientCookie*>(cookie); + + // may reference other cookie members + (*funcs->onDeath)(); }; void LambdaOnUnlink(void* cookie) { auto funcs = static_cast<DeathRecipientCookie*>(cookie); (*funcs->onUnlink)(); + + // may reference other cookie members + + delete funcs; }; TEST(NdkBinder, DeathRecipient) { using namespace std::chrono_literals; @@ -536,12 +557,12 @@ TEST(NdkBinder, DeathRecipient) { unlinkCv.notify_one(); }; - DeathRecipientCookie cookie = {&onDeath, &onUnlink}; + DeathRecipientCookie* cookie = new DeathRecipientCookie{&onDeath, &onUnlink}; AIBinder_DeathRecipient* recipient = AIBinder_DeathRecipient_new(LambdaOnDeath); AIBinder_DeathRecipient_setOnUnlinked(recipient, LambdaOnUnlink); - EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient, static_cast<void*>(&cookie))); + EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient, static_cast<void*>(cookie))); // the binder driver should return this if the service dies during the transaction EXPECT_EQ(STATUS_DEAD_OBJECT, foo->die()); diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index d0e35de3f7..b90b40bac8 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -1122,6 +1122,10 @@ macro_rules! declare_binder_enum { } impl $crate::binder_impl::Deserialize for $enum { + type UninitType = Self; + fn uninit() -> Self::UninitType { Self::UninitType::default() } + fn from_init(value: Self) -> Self::UninitType { value } + fn deserialize(parcel: &$crate::binder_impl::BorrowedParcel<'_>) -> std::result::Result<Self, $crate::StatusCode> { parcel.read().map(Self) } diff --git a/libs/binder/rust/src/error.rs b/libs/binder/rust/src/error.rs index f6b09ed8fe..ba260624bc 100644 --- a/libs/binder/rust/src/error.rs +++ b/libs/binder/rust/src/error.rs @@ -20,6 +20,7 @@ use crate::sys; use std::error; use std::ffi::{CStr, CString}; use std::fmt::{Debug, Display, Formatter, Result as FmtResult}; +use std::ptr; use std::result; pub use sys::binder_status_t as status_t; @@ -92,7 +93,7 @@ fn parse_exception_code(code: i32) -> ExceptionCode { /// track of and chain binder errors along with service specific errors. /// /// Used in AIDL transactions to represent failed transactions. -pub struct Status(*mut sys::AStatus); +pub struct Status(ptr::NonNull<sys::AStatus>); // Safety: The `AStatus` that the `Status` points to must have an entirely thread-safe API for the // duration of the `Status` object's lifetime. We ensure this by not allowing mutation of a `Status` @@ -119,7 +120,7 @@ impl Status { // Rust takes ownership of the returned pointer. sys::AStatus_newOk() }; - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } /// Create a status object from a service specific error @@ -147,7 +148,7 @@ impl Status { sys::AStatus_fromServiceSpecificError(err) } }; - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } /// Creates a status object from a service specific error. @@ -161,7 +162,7 @@ impl Status { let ptr = unsafe { sys::AStatus_fromExceptionCodeWithMessage(exception as i32, message.as_ptr()) }; - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } else { exception.into() } @@ -181,7 +182,7 @@ impl Status { /// /// This constructor is safe iff `ptr` is a valid pointer to an `AStatus`. pub(crate) unsafe fn from_ptr(ptr: *mut sys::AStatus) -> Self { - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } /// Returns `true` if this status represents a successful transaction. @@ -326,7 +327,7 @@ impl From<status_t> for Status { // UNKNOWN_ERROR. sys::AStatus_fromStatus(status) }; - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } } @@ -338,7 +339,7 @@ impl From<ExceptionCode> for Status { // Unknown values will be coerced into EX_TRANSACTION_FAILED. sys::AStatus_fromExceptionCode(code as i32) }; - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } } @@ -367,7 +368,7 @@ impl Drop for Status { // pointee, so we need to delete it here. We know that the pointer // will be valid here since `Status` always contains a valid pointer // while it is alive. - sys::AStatus_delete(self.0); + sys::AStatus_delete(self.0.as_mut()); } } } @@ -381,11 +382,15 @@ impl Drop for Status { /// `Status` object is still alive. unsafe impl AsNative<sys::AStatus> for Status { fn as_native(&self) -> *const sys::AStatus { - self.0 + self.0.as_ptr() } fn as_native_mut(&mut self) -> *mut sys::AStatus { - self.0 + unsafe { + // Safety: The pointer will be valid here since `Status` always + // contains a valid and initialized pointer while it is alive. + self.0.as_mut() + } } } diff --git a/libs/binder/rust/src/parcel/file_descriptor.rs b/libs/binder/rust/src/parcel/file_descriptor.rs index de6d64934a..7fe37f3b68 100644 --- a/libs/binder/rust/src/parcel/file_descriptor.rs +++ b/libs/binder/rust/src/parcel/file_descriptor.rs @@ -132,6 +132,14 @@ impl DeserializeOption for ParcelFileDescriptor { } impl Deserialize for ParcelFileDescriptor { + type UninitType = Option<Self>; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { Deserialize::deserialize(parcel).transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL)) } diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs index 4b658fc74d..5d8c11cf94 100644 --- a/libs/binder/rust/src/parcel/parcelable.rs +++ b/libs/binder/rust/src/parcel/parcelable.rs @@ -14,7 +14,7 @@ * limitations under the License. */ -use crate::binder::{AsNative, FromIBinder, Stability, Strong}; +use crate::binder::{AsNative, FromIBinder, Interface, Stability, Strong}; use crate::error::{status_result, status_t, Result, Status, StatusCode}; use crate::parcel::BorrowedParcel; use crate::proxy::SpIBinder; @@ -22,7 +22,7 @@ use crate::sys; use std::convert::{TryFrom, TryInto}; use std::ffi::c_void; -use std::mem::{self, ManuallyDrop, MaybeUninit}; +use std::mem::{self, ManuallyDrop}; use std::os::raw::c_char; use std::ptr; use std::slice; @@ -60,6 +60,26 @@ pub trait Serialize { /// A struct whose instances can be restored from a [`Parcel`]. // Might be able to hook this up as a serde backend in the future? pub trait Deserialize: Sized { + /// Type for the uninitialized value of this type. Will be either `Self` + /// if the type implements `Default`, `Option<Self>` otherwise. + type UninitType; + + /// Assert at compile-time that `Self` and `Self::UninitType` have the same + /// size and alignment. This will either fail to compile or evaluate to `true`. + /// The only two macros that work here are `panic!` and `assert!`, so we cannot + /// use `assert_eq!`. + const ASSERT_UNINIT_SIZE_AND_ALIGNMENT: bool = { + assert!(std::mem::size_of::<Self>() == std::mem::size_of::<Self::UninitType>()); + assert!(std::mem::align_of::<Self>() == std::mem::align_of::<Self::UninitType>()); + true + }; + + /// Return an uninitialized or default-initialized value for this type. + fn uninit() -> Self::UninitType; + + /// Convert an initialized value of type `Self` into `Self::UninitType`. + fn from_init(value: Self) -> Self::UninitType; + /// Deserialize an instance from the given [`Parcel`]. fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self>; @@ -121,7 +141,7 @@ unsafe extern "C" fn serialize_element<T: Serialize>( pub trait DeserializeArray: Deserialize { /// Deserialize an array of type from the given parcel. fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> { - let mut vec: Option<Vec<MaybeUninit<Self>>> = None; + let mut vec: Option<Vec<Self::UninitType>> = None; let res = unsafe { // Safety: Safe FFI, vec is the correct opaque type expected by // allocate_vec and deserialize_element. @@ -136,8 +156,8 @@ pub trait DeserializeArray: Deserialize { let vec: Option<Vec<Self>> = unsafe { // Safety: We are assuming that the NDK correctly initialized every // element of the vector by now, so we know that all the - // MaybeUninits are now properly initialized. We can transmute from - // Vec<MaybeUninit<T>> to Vec<T> because MaybeUninit<T> has the same + // UninitTypes are now properly initialized. We can transmute from + // Vec<T::UninitType> to Vec<T> because T::UninitType has the same // alignment and size as T, so the pointer to the vector allocation // will be compatible. mem::transmute(vec) @@ -149,14 +169,14 @@ pub trait DeserializeArray: Deserialize { /// Callback to deserialize a parcelable element. /// /// The opaque array data pointer must be a mutable pointer to an -/// `Option<Vec<MaybeUninit<T>>>` with at least enough elements for `index` to be valid +/// `Option<Vec<T::UninitType>>` with at least enough elements for `index` to be valid /// (zero-based). unsafe extern "C" fn deserialize_element<T: Deserialize>( parcel: *const sys::AParcel, array: *mut c_void, index: usize, ) -> status_t { - let vec = &mut *(array as *mut Option<Vec<MaybeUninit<T>>>); + let vec = &mut *(array as *mut Option<Vec<T::UninitType>>); let vec = match vec { Some(v) => v, None => return StatusCode::BAD_INDEX as status_t, @@ -170,7 +190,7 @@ unsafe extern "C" fn deserialize_element<T: Deserialize>( Ok(e) => e, Err(code) => return code as status_t, }; - ptr::write(vec[index].as_mut_ptr(), element); + vec[index] = T::from_init(element); StatusCode::OK as status_t } @@ -233,15 +253,15 @@ pub trait DeserializeOption: Deserialize { /// # Safety /// /// The opaque data pointer passed to the array read function must be a mutable -/// pointer to an `Option<Vec<MaybeUninit<T>>>`. `buffer` will be assigned a mutable pointer +/// pointer to an `Option<Vec<T::UninitType>>`. `buffer` will be assigned a mutable pointer /// to the allocated vector data if this function returns true. -unsafe extern "C" fn allocate_vec_with_buffer<T>( +unsafe extern "C" fn allocate_vec_with_buffer<T: Deserialize>( data: *mut c_void, len: i32, buffer: *mut *mut T, ) -> bool { let res = allocate_vec::<T>(data, len); - let vec = &mut *(data as *mut Option<Vec<MaybeUninit<T>>>); + let vec = &mut *(data as *mut Option<Vec<T::UninitType>>); if let Some(new_vec) = vec { *buffer = new_vec.as_mut_ptr() as *mut T; } @@ -253,20 +273,18 @@ unsafe extern "C" fn allocate_vec_with_buffer<T>( /// # Safety /// /// The opaque data pointer passed to the array read function must be a mutable -/// pointer to an `Option<Vec<MaybeUninit<T>>>`. -unsafe extern "C" fn allocate_vec<T>(data: *mut c_void, len: i32) -> bool { - let vec = &mut *(data as *mut Option<Vec<MaybeUninit<T>>>); +/// pointer to an `Option<Vec<T::UninitType>>`. +unsafe extern "C" fn allocate_vec<T: Deserialize>(data: *mut c_void, len: i32) -> bool { + let vec = &mut *(data as *mut Option<Vec<T::UninitType>>); if len < 0 { *vec = None; return true; } - let mut new_vec: Vec<MaybeUninit<T>> = Vec::with_capacity(len as usize); - // Safety: We are filling the vector with uninitialized data here, but this - // is safe because the vector contains MaybeUninit elements which can be - // uninitialized. We're putting off the actual unsafe bit, transmuting the - // vector to a Vec<T> until the contents are initialized. - new_vec.set_len(len as usize); + // Assert at compile time that `T` and `T::UninitType` have the same size and alignment. + let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT; + let mut new_vec: Vec<T::UninitType> = Vec::with_capacity(len as usize); + new_vec.resize_with(len as usize, T::uninit); ptr::write(vec, Some(new_vec)); true @@ -283,8 +301,11 @@ macro_rules! parcelable_primitives { } /// Safety: All elements in the vector must be properly initialized. -unsafe fn vec_assume_init<T>(vec: Vec<MaybeUninit<T>>) -> Vec<T> { - // We can convert from Vec<MaybeUninit<T>> to Vec<T> because MaybeUninit<T> +unsafe fn vec_assume_init<T: Deserialize>(vec: Vec<T::UninitType>) -> Vec<T> { + // Assert at compile time that `T` and `T::UninitType` have the same size and alignment. + let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT; + + // We can convert from Vec<T::UninitType> to Vec<T> because T::UninitType // has the same alignment and size as T, so the pointer to the vector // allocation will be compatible. let mut vec = ManuallyDrop::new(vec); @@ -307,6 +328,9 @@ macro_rules! impl_parcelable { {Deserialize, $ty:ty, $read_fn:path} => { impl Deserialize for $ty { + type UninitType = Self; + fn uninit() -> Self::UninitType { Self::UninitType::default() } + fn from_init(value: Self) -> Self::UninitType { value } fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { let mut val = Self::default(); unsafe { @@ -348,11 +372,11 @@ macro_rules! impl_parcelable { {DeserializeArray, $ty:ty, $read_array_fn:path} => { impl DeserializeArray for $ty { fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> { - let mut vec: Option<Vec<MaybeUninit<Self>>> = None; + let mut vec: Option<Vec<Self::UninitType>> = None; let status = unsafe { // Safety: `Parcel` always contains a valid pointer to an // `AParcel`. `allocate_vec<T>` expects the opaque pointer to - // be of type `*mut Option<Vec<MaybeUninit<T>>>`, so `&mut vec` is + // be of type `*mut Option<Vec<T::UninitType>>`, so `&mut vec` is // correct for it. $read_array_fn( parcel.as_native(), @@ -364,7 +388,7 @@ macro_rules! impl_parcelable { let vec: Option<Vec<Self>> = unsafe { // Safety: We are assuming that the NDK correctly // initialized every element of the vector by now, so we - // know that all the MaybeUninits are now properly + // know that all the UninitTypes are now properly // initialized. vec.map(|vec| vec_assume_init(vec)) }; @@ -440,6 +464,14 @@ impl Serialize for u8 { } impl Deserialize for u8 { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { i8::deserialize(parcel).map(|v| v as u8) } @@ -471,6 +503,14 @@ impl Serialize for i16 { } impl Deserialize for i16 { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { u16::deserialize(parcel).map(|v| v as i16) } @@ -547,6 +587,14 @@ impl SerializeOption for String { } impl Deserialize for Option<String> { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { let mut vec: Option<Vec<u8>> = None; let status = unsafe { @@ -575,6 +623,14 @@ impl Deserialize for Option<String> { impl DeserializeArray for Option<String> {} impl Deserialize for String { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { Deserialize::deserialize(parcel).transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL)) } @@ -611,6 +667,14 @@ impl<T: SerializeArray> SerializeOption for Vec<T> { } impl<T: DeserializeArray> Deserialize for Vec<T> { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { DeserializeArray::deserialize_array(parcel) .transpose() @@ -640,6 +704,14 @@ impl<T: SerializeArray, const N: usize> SerializeOption for [T; N] { impl<T: SerializeArray, const N: usize> SerializeArray for [T; N] {} impl<T: DeserializeArray, const N: usize> Deserialize for [T; N] { + type UninitType = [T::UninitType; N]; + fn uninit() -> Self::UninitType { + [(); N].map(|_| T::uninit()) + } + fn from_init(value: Self) -> Self::UninitType { + value.map(T::from_init) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { let vec = DeserializeArray::deserialize_array(parcel) .transpose() @@ -664,6 +736,14 @@ impl Serialize for Stability { } impl Deserialize for Stability { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { i32::deserialize(parcel).and_then(Stability::try_from) } @@ -682,6 +762,14 @@ impl Serialize for Status { } impl Deserialize for Status { + type UninitType = Option<Self>; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { let mut status_ptr = ptr::null_mut(); let ret_status = unsafe { @@ -717,12 +805,29 @@ impl<T: SerializeOption + FromIBinder + ?Sized> SerializeOption for Strong<T> { impl<T: Serialize + FromIBinder + ?Sized> SerializeArray for Strong<T> {} impl<T: FromIBinder + ?Sized> Deserialize for Strong<T> { + type UninitType = Option<Strong<T>>; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { let ibinder: SpIBinder = parcel.read()?; FromIBinder::try_from(ibinder) } } +struct AssertIBinder; +impl Interface for AssertIBinder {} +impl FromIBinder for AssertIBinder { + // This is only needed so we can assert on the size of Strong<AssertIBinder> + fn try_from(_: SpIBinder) -> Result<Strong<Self>> { + unimplemented!() + } +} + impl<T: FromIBinder + ?Sized> DeserializeOption for Strong<T> { fn deserialize_option(parcel: &BorrowedParcel<'_>) -> Result<Option<Self>> { let ibinder: Option<SpIBinder> = parcel.read()?; @@ -752,6 +857,14 @@ impl<T: SerializeOption> Serialize for Option<T> { } impl<T: DeserializeOption> Deserialize for Option<T> { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { DeserializeOption::deserialize_option(parcel) } @@ -821,6 +934,9 @@ macro_rules! impl_deserialize_for_parcelable { }; ($parcelable:ident < $( $param:ident ),* > ) => { impl < $($param: Default),* > $crate::binder_impl::Deserialize for $parcelable < $($param),* > { + type UninitType = Self; + fn uninit() -> Self::UninitType { Self::UninitType::default() } + fn from_init(value: Self) -> Self::UninitType { value } fn deserialize( parcel: &$crate::binder_impl::BorrowedParcel<'_>, ) -> std::result::Result<Self, $crate::StatusCode> { @@ -876,6 +992,14 @@ impl<T: Serialize> Serialize for Box<T> { } impl<T: Deserialize> Deserialize for Box<T> { + type UninitType = Option<Self>; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { Deserialize::deserialize(parcel).map(Box::new) } @@ -900,6 +1024,7 @@ mod tests { #[test] fn test_custom_parcelable() { + #[derive(Default)] struct Custom(u32, bool, String, Vec<String>); impl Serialize for Custom { @@ -912,6 +1037,14 @@ mod tests { } impl Deserialize for Custom { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> { Ok(Custom( parcel.read()?, diff --git a/libs/binder/rust/src/parcel/parcelable_holder.rs b/libs/binder/rust/src/parcel/parcelable_holder.rs index c829d37dca..383cc83509 100644 --- a/libs/binder/rust/src/parcel/parcelable_holder.rs +++ b/libs/binder/rust/src/parcel/parcelable_holder.rs @@ -169,6 +169,14 @@ impl Serialize for ParcelableHolder { } impl Deserialize for ParcelableHolder { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::new(Default::default()) + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self, StatusCode> { let status: i32 = parcel.read()?; if status == NULL_PARCELABLE_FLAG { diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs index 254efaed9a..036f6b4f01 100644 --- a/libs/binder/rust/src/proxy.rs +++ b/libs/binder/rust/src/proxy.rs @@ -439,6 +439,14 @@ impl SerializeOption for SpIBinder { impl SerializeArray for SpIBinder {} impl Deserialize for SpIBinder { + type UninitType = Option<Self>; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<SpIBinder> { parcel.read_binder().transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL)) } diff --git a/libs/binder/rust/tests/parcel_fuzzer/Android.bp b/libs/binder/rust/tests/parcel_fuzzer/Android.bp index df8a2afb03..ac968237a0 100644 --- a/libs/binder/rust/tests/parcel_fuzzer/Android.bp +++ b/libs/binder/rust/tests/parcel_fuzzer/Android.bp @@ -21,6 +21,7 @@ rust_fuzz { "waghpawan@google.com", "smoreland@google.com", ], + triage_assignee: "waghpawan@google.com", // hotlist "AIDL fuzzers bugs" on buganizer hotlists: ["4637097"], }, diff --git a/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp b/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp index 5cb406afc2..89126ca2a7 100644 --- a/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp +++ b/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp @@ -19,6 +19,11 @@ rust_fuzz { srcs: [ "service_fuzzer.rs", ], + shared_libs: [ + "libbinder", + "libbinder_ndk", + "libutils", + ], rustlibs: [ "libbinder_rs", "libbinder_random_parcel_rs", @@ -29,6 +34,7 @@ rust_fuzz { "waghpawan@google.com", "smoreland@google.com", ], + triage_assignee: "waghpawan@google.com", // hotlist "AIDL fuzzers bugs" on buganizer hotlists: ["4637097"], }, diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index cad364d8e8..41856f9b79 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -76,8 +76,10 @@ cc_test { ], static_libs: [ "binderRecordReplayTestIface-cpp", + "binderReadParcelIface-cpp", ], test_suites: ["general-tests"], + require_root: true, } aidl_interface { @@ -86,6 +88,13 @@ aidl_interface { srcs: [ "IBinderRecordReplayTest.aidl", ], + imports: ["binderReadParcelIface"], + backend: { + java: { + enabled: true, + platform_apis: true, + }, + }, } cc_test { @@ -689,6 +698,7 @@ cc_benchmark { "liblog", "libutils", ], + test_suites: ["general-tests"], } cc_test_host { @@ -791,3 +801,15 @@ cc_defaults { hotlists: ["4637097"], }, } + +cc_defaults { + name: "fuzzer_disable_leaks", + fuzz_config: { + asan_options: [ + "detect_leaks=0", + ], + hwasan_options: [ + "detect_leaks=0", + ], + }, +} diff --git a/libs/binder/tests/IBinderRecordReplayTest.aidl b/libs/binder/tests/IBinderRecordReplayTest.aidl index 3c8c722da4..bd6b03c6e0 100644 --- a/libs/binder/tests/IBinderRecordReplayTest.aidl +++ b/libs/binder/tests/IBinderRecordReplayTest.aidl @@ -13,8 +13,60 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import parcelables.SingleDataParcelable; interface IBinderRecordReplayTest { + void setByte(byte input); + byte getByte(); + + void setChar(char input); + char getChar(); + + void setBoolean(boolean input); + boolean getBoolean(); + void setInt(int input); int getInt(); + + void setFloat(float input); + float getFloat(); + + void setLong(long input); + long getLong(); + + void setDouble(double input); + double getDouble(); + + void setString(String input); + String getString(); + + void setSingleDataParcelable(in SingleDataParcelable p); + SingleDataParcelable getSingleDataParcelable(); + + void setByteArray(in byte[] input); + byte[] getByteArray(); + + void setCharArray(in char[] input); + char[] getCharArray(); + + void setBooleanArray(in boolean[] input); + boolean[] getBooleanArray(); + + void setIntArray(in int[] input); + int[] getIntArray(); + + void setFloatArray(in float[] input); + float[] getFloatArray(); + + void setLongArray(in long[] input); + long[] getLongArray(); + + void setDoubleArray(in double[] input); + double[] getDoubleArray(); + + void setStringArray(in String[] input); + String[] getStringArray(); + + void setSingleDataParcelableArray(in SingleDataParcelable[] input); + SingleDataParcelable[] getSingleDataParcelableArray(); } diff --git a/libs/binder/tests/binderRecordReplayTest.cpp b/libs/binder/tests/binderRecordReplayTest.cpp index 55148acf14..17d5c8a219 100644 --- a/libs/binder/tests/binderRecordReplayTest.cpp +++ b/libs/binder/tests/binderRecordReplayTest.cpp @@ -27,72 +27,248 @@ #include <sys/prctl.h> +#include "parcelables/SingleDataParcelable.h" + using namespace android; using android::binder::Status; using android::binder::debug::RecordedTransaction; +using parcelables::SingleDataParcelable; const String16 kServerName = String16("binderRecordReplay"); +#define GENERATE_GETTER_SETTER_PRIMITIVE(name, T) \ + Status set##name(T input) { \ + m##name = input; \ + return Status::ok(); \ + } \ + \ + Status get##name(T* output) { \ + *output = m##name; \ + return Status::ok(); \ + } \ + T m##name + +#define GENERATE_GETTER_SETTER(name, T) \ + Status set##name(const T& input) { \ + m##name = input; \ + return Status::ok(); \ + } \ + \ + Status get##name(T* output) { \ + *output = m##name; \ + return Status::ok(); \ + } \ + T m##name + class MyRecordReplay : public BnBinderRecordReplayTest { public: - Status setInt(int input) { - mInt = input; - return Status::ok(); + GENERATE_GETTER_SETTER_PRIMITIVE(Boolean, bool); + GENERATE_GETTER_SETTER_PRIMITIVE(Byte, int8_t); + GENERATE_GETTER_SETTER_PRIMITIVE(Int, int); + GENERATE_GETTER_SETTER_PRIMITIVE(Char, char16_t); + GENERATE_GETTER_SETTER_PRIMITIVE(Long, int64_t); + GENERATE_GETTER_SETTER_PRIMITIVE(Float, float); + GENERATE_GETTER_SETTER_PRIMITIVE(Double, double); + + GENERATE_GETTER_SETTER(String, String16); + GENERATE_GETTER_SETTER(SingleDataParcelable, SingleDataParcelable); + + GENERATE_GETTER_SETTER(BooleanArray, std::vector<bool>); + GENERATE_GETTER_SETTER(ByteArray, std::vector<uint8_t>); + GENERATE_GETTER_SETTER(IntArray, std::vector<int>); + GENERATE_GETTER_SETTER(CharArray, std::vector<char16_t>); + GENERATE_GETTER_SETTER(LongArray, std::vector<int64_t>); + GENERATE_GETTER_SETTER(FloatArray, std::vector<float>); + GENERATE_GETTER_SETTER(DoubleArray, std::vector<double>); + GENERATE_GETTER_SETTER(StringArray, std::vector<::android::String16>); + GENERATE_GETTER_SETTER(SingleDataParcelableArray, std::vector<SingleDataParcelable>); +}; + +class BinderRecordReplayTest : public ::testing::Test { +public: + void SetUp() override { + // get the remote service + auto binder = defaultServiceManager()->getService(kServerName); + ASSERT_NE(nullptr, binder); + mInterface = interface_cast<IBinderRecordReplayTest>(binder); + mBpBinder = binder->remoteBinder(); + ASSERT_NE(nullptr, mBpBinder); } - Status getInt(int* output) { - *output = mInt; - return Status::ok(); + + template <typename T, typename U> + void recordReplay(Status (IBinderRecordReplayTest::*set)(T), U recordedValue, + Status (IBinderRecordReplayTest::*get)(U*), U changedValue) { + base::unique_fd fd(open("/data/local/tmp/binderRecordReplayTest.rec", + O_RDWR | O_CREAT | O_CLOEXEC, 0666)); + ASSERT_TRUE(fd.ok()); + + // record a transaction + mBpBinder->startRecordingBinder(fd); + auto status = (*mInterface.*set)(recordedValue); + EXPECT_TRUE(status.isOk()); + mBpBinder->stopRecordingBinder(); + + // test transaction does the thing we expect it to do + U output; + status = (*mInterface.*get)(&output); + EXPECT_TRUE(status.isOk()); + EXPECT_EQ(output, recordedValue); + + // write over the existing state + status = (*mInterface.*set)(changedValue); + EXPECT_TRUE(status.isOk()); + + status = (*mInterface.*get)(&output); + EXPECT_TRUE(status.isOk()); + + EXPECT_EQ(output, changedValue); + + // replay transaction + ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET)); + std::optional<RecordedTransaction> transaction = RecordedTransaction::fromFile(fd); + ASSERT_NE(transaction, std::nullopt); + + // TODO: move logic to replay RecordedTransaction into RecordedTransaction + Parcel data; + data.setData(transaction->getDataParcel().data(), transaction->getDataParcel().dataSize()); + auto result = + mBpBinder->transact(transaction->getCode(), data, nullptr, transaction->getFlags()); + + // make sure recording does the thing we expect it to do + EXPECT_EQ(OK, result); + + status = (*mInterface.*get)(&output); + EXPECT_TRUE(status.isOk()); + EXPECT_EQ(output, recordedValue); } private: - int mInt = 0; + sp<BpBinder> mBpBinder; + sp<IBinderRecordReplayTest> mInterface; }; -TEST(BinderClearBuf, RecordReplayRepeatInt) { - // get the remote service - sp<IBinder> binder = defaultServiceManager()->getService(kServerName); - ASSERT_NE(nullptr, binder); - sp<IBinderRecordReplayTest> iface = interface_cast<IBinderRecordReplayTest>(binder); - sp<BpBinder> bpBinder = binder->remoteBinder(); - ASSERT_NE(nullptr, bpBinder); - - base::unique_fd fd( - open("/data/local/tmp/binderRecordReplayTest.rec", O_RDWR | O_CREAT | O_CLOEXEC, 0666)); - ASSERT_TRUE(fd.ok()); - - // record a transaction - bpBinder->startRecordingBinder(fd); - EXPECT_TRUE(iface->setInt(3).isOk()); - bpBinder->stopRecordingBinder(); - - // test transaction does the thing we expect it to do - int output; - EXPECT_TRUE(iface->getInt(&output).isOk()); - EXPECT_EQ(output, 3); - - // write over the existing state - EXPECT_TRUE(iface->setInt(5).isOk()); - EXPECT_TRUE(iface->getInt(&output).isOk()); - EXPECT_EQ(output, 5); - - // replay transaction - ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET)); - std::optional<RecordedTransaction> transaction = RecordedTransaction::fromFile(fd); - ASSERT_NE(transaction, std::nullopt); - - // TODO: move logic to replay RecordedTransaction into RecordedTransaction - Parcel data; - data.setData(transaction->getDataParcel().data(), transaction->getDataParcel().dataSize()); - status_t status = binder->remoteBinder()->transact(transaction->getCode(), data, nullptr, - transaction->getFlags()); - - // make sure recording does the thing we expect it to do - EXPECT_EQ(OK, status); - EXPECT_TRUE(iface->getInt(&output).isOk()); - EXPECT_EQ(output, 3); - - // TODO: we should also make sure we can convert the recording to a fuzzer - // corpus entry, and we will be able to replay it in the same way +TEST_F(BinderRecordReplayTest, ReplayByte) { + recordReplay(&IBinderRecordReplayTest::setByte, int8_t{122}, &IBinderRecordReplayTest::getByte, + int8_t{90}); +} + +TEST_F(BinderRecordReplayTest, ReplayBoolean) { + recordReplay(&IBinderRecordReplayTest::setBoolean, true, &IBinderRecordReplayTest::getBoolean, + false); +} + +TEST_F(BinderRecordReplayTest, ReplayChar) { + recordReplay(&IBinderRecordReplayTest::setChar, char16_t{'G'}, + &IBinderRecordReplayTest::getChar, char16_t{'K'}); +} + +TEST_F(BinderRecordReplayTest, ReplayInt) { + recordReplay(&IBinderRecordReplayTest::setInt, 3, &IBinderRecordReplayTest::getInt, 5); +} + +TEST_F(BinderRecordReplayTest, ReplayFloat) { + recordReplay(&IBinderRecordReplayTest::setFloat, 1.1f, &IBinderRecordReplayTest::getFloat, + 22.0f); +} + +TEST_F(BinderRecordReplayTest, ReplayLong) { + recordReplay(&IBinderRecordReplayTest::setLong, int64_t{1LL << 55}, + &IBinderRecordReplayTest::getLong, int64_t{1LL << 12}); +} + +TEST_F(BinderRecordReplayTest, ReplayDouble) { + recordReplay(&IBinderRecordReplayTest::setDouble, 0.00, &IBinderRecordReplayTest::getDouble, + 1.11); +} + +TEST_F(BinderRecordReplayTest, ReplayString) { + const ::android::String16& input1 = String16("This is saved string"); + const ::android::String16& input2 = String16("This is changed string"); + recordReplay(&IBinderRecordReplayTest::setString, input1, &IBinderRecordReplayTest::getString, + input2); +} + +TEST_F(BinderRecordReplayTest, ReplaySingleDataParcelable) { + SingleDataParcelable saved, changed; + saved.data = 3; + changed.data = 5; + recordReplay(&IBinderRecordReplayTest::setSingleDataParcelable, saved, + &IBinderRecordReplayTest::getSingleDataParcelable, changed); +} + +TEST_F(BinderRecordReplayTest, ReplayByteArray) { + std::vector<uint8_t> savedArray = {uint8_t{255}, uint8_t{0}, uint8_t{127}}; + std::vector<uint8_t> changedArray = {uint8_t{2}, uint8_t{7}, uint8_t{117}}; + recordReplay(&IBinderRecordReplayTest::setByteArray, savedArray, + &IBinderRecordReplayTest::getByteArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplayBooleanArray) { + std::vector<bool> savedArray = {true, false, true}; + std::vector<bool> changedArray = {false, true, false}; + recordReplay(&IBinderRecordReplayTest::setBooleanArray, savedArray, + &IBinderRecordReplayTest::getBooleanArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplayCharArray) { + std::vector<char16_t> savedArray = {char16_t{'G'}, char16_t{'L'}, char16_t{'K'}, char16_t{'T'}}; + std::vector<char16_t> changedArray = {char16_t{'X'}, char16_t{'Y'}, char16_t{'Z'}}; + recordReplay(&IBinderRecordReplayTest::setCharArray, savedArray, + &IBinderRecordReplayTest::getCharArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplayIntArray) { + std::vector<int> savedArray = {12, 45, 178}; + std::vector<int> changedArray = {32, 14, 78, 1899}; + recordReplay(&IBinderRecordReplayTest::setIntArray, savedArray, + &IBinderRecordReplayTest::getIntArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplayFloatArray) { + std::vector<float> savedArray = {12.14f, 45.56f, 123.178f}; + std::vector<float> changedArray = {0.00f, 14.0f, 718.1f, 1899.122f, 3268.123f}; + recordReplay(&IBinderRecordReplayTest::setFloatArray, savedArray, + &IBinderRecordReplayTest::getFloatArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplayLongArray) { + std::vector<int64_t> savedArray = {int64_t{1LL << 11}, int64_t{1LL << 55}, int64_t{1LL << 45}}; + std::vector<int64_t> changedArray = {int64_t{1LL << 1}, int64_t{1LL << 21}, int64_t{1LL << 33}, + int64_t{1LL << 62}}; + recordReplay(&IBinderRecordReplayTest::setLongArray, savedArray, + &IBinderRecordReplayTest::getLongArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplayDoubleArray) { + std::vector<double> savedArray = {12.1412313, 45.561232, 123.1781111}; + std::vector<double> changedArray = {0.00111, 14.32130, 712312318.19, 1899212.122, + 322168.122123}; + recordReplay(&IBinderRecordReplayTest::setDoubleArray, savedArray, + &IBinderRecordReplayTest::getDoubleArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplayStringArray) { + std::vector<String16> savedArray = {String16("This is saved value"), String16(), + String16("\0\0", 2), String16("\xF3\x01\xAC\xAD\x21\xAF")}; + + std::vector<String16> changedArray = {String16("This is changed value"), + String16("\xF0\x90\x90\xB7\xE2\x82\xAC")}; + recordReplay(&IBinderRecordReplayTest::setStringArray, savedArray, + &IBinderRecordReplayTest::getStringArray, changedArray); +} + +TEST_F(BinderRecordReplayTest, ReplaySingleDataParcelableArray) { + SingleDataParcelable s1, s2, s3, s4, s5; + s1.data = 5213; + s2.data = 1512; + s3.data = 4233; + s4.data = 123124; + s5.data = 0; + std::vector<SingleDataParcelable> saved = {s1, s2, s3}; + std::vector<SingleDataParcelable> changed = {s4, s5}; + + recordReplay(&IBinderRecordReplayTest::setSingleDataParcelableArray, saved, + &IBinderRecordReplayTest::getSingleDataParcelableArray, changed); } int main(int argc, char** argv) { diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp index 593927306e..9c96c4182d 100644 --- a/libs/binder/tests/binderRpcBenchmark.cpp +++ b/libs/binder/tests/binderRpcBenchmark.cpp @@ -129,12 +129,33 @@ static sp<IBinder> getBinderForOptions(benchmark::State& state) { } } +static void SetLabel(benchmark::State& state) { + Transport transport = static_cast<Transport>(state.range(0)); + switch (transport) { +#ifdef __BIONIC__ + case KERNEL: + state.SetLabel("kernel"); + break; +#endif + case RPC: + state.SetLabel("rpc"); + break; + case RPC_TLS: + state.SetLabel("rpc_tls"); + break; + default: + LOG(FATAL) << "Unknown transport value: " << transport; + } +} + void BM_pingTransaction(benchmark::State& state) { sp<IBinder> binder = getBinderForOptions(state); while (state.KeepRunning()) { CHECK_EQ(OK, binder->pingBinder()); } + + SetLabel(state); } BENCHMARK(BM_pingTransaction)->ArgsProduct({kTransportList}); @@ -164,6 +185,8 @@ void BM_repeatTwoPageString(benchmark::State& state) { Status ret = iface->repeatString(str, &out); CHECK(ret.isOk()) << ret; } + + SetLabel(state); } BENCHMARK(BM_repeatTwoPageString)->ArgsProduct({kTransportList}); @@ -182,6 +205,8 @@ void BM_throughputForTransportAndBytes(benchmark::State& state) { Status ret = iface->repeatBytes(bytes, &out); CHECK(ret.isOk()) << ret; } + + SetLabel(state); } BENCHMARK(BM_throughputForTransportAndBytes) ->ArgsProduct({kTransportList, @@ -201,6 +226,8 @@ void BM_repeatBinder(benchmark::State& state) { Status ret = iface->repeatBinder(binder, &out); CHECK(ret.isOk()) << ret; } + + SetLabel(state); } BENCHMARK(BM_repeatBinder)->ArgsProduct({kTransportList}); @@ -228,11 +255,6 @@ int main(int argc, char** argv) { ::benchmark::Initialize(&argc, argv); if (::benchmark::ReportUnrecognizedArguments(argc, argv)) return 1; - std::cerr << "Tests suffixes:" << std::endl; - std::cerr << "\t.../" << Transport::KERNEL << " is KERNEL" << std::endl; - std::cerr << "\t.../" << Transport::RPC << " is RPC" << std::endl; - std::cerr << "\t.../" << Transport::RPC_TLS << " is RPC with TLS" << std::endl; - #ifdef __BIONIC__ if (0 == fork()) { prctl(PR_SET_PDEATHSIG, SIGHUP); // racey, okay diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index d6aa6c636f..1ff1de4db8 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -461,8 +461,11 @@ static void testThreadPoolOverSaturated(sp<IBinderRpcTest> iface, size_t numCall EXPECT_GE(epochMsAfter, epochMsBefore + 2 * sleepMs); - // Potential flake, but make sure calls are handled in parallel. - EXPECT_LE(epochMsAfter, epochMsBefore + 4 * sleepMs); + // Potential flake, but make sure calls are handled in parallel. Due + // to past flakes, this only checks that the amount of time taken has + // some parallelism. Other tests such as ThreadPoolGreaterThanEqualRequested + // check this more exactly. + EXPECT_LE(epochMsAfter, epochMsBefore + (numCalls - 1) * sleepMs); } TEST_P(BinderRpc, ThreadPoolOverSaturated) { @@ -687,6 +690,8 @@ TEST_P(BinderRpc, SessionWithIncomingThreadpoolDoesntLeak) { } EXPECT_EQ(nullptr, session.promote()); + + sleep(1); // give time for remote session to shutdown } TEST_P(BinderRpc, SingleDeathRecipient) { @@ -1361,7 +1366,7 @@ TEST_P(BinderRpcServerOnly, SetExternalServerTest) { base::unique_fd sink(TEMP_FAILURE_RETRY(open("/dev/null", O_RDWR))); int sinkFd = sink.get(); auto server = RpcServer::make(newTlsFactory(std::get<0>(GetParam()))); - server->setProtocolVersion(std::get<1>(GetParam())); + ASSERT_TRUE(server->setProtocolVersion(std::get<1>(GetParam()))); ASSERT_FALSE(server->hasServer()); ASSERT_EQ(OK, server->setupExternalServer(std::move(sink))); ASSERT_TRUE(server->hasServer()); @@ -1377,7 +1382,7 @@ TEST_P(BinderRpcServerOnly, Shutdown) { auto addr = allocateSocketAddress(); auto server = RpcServer::make(newTlsFactory(std::get<0>(GetParam()))); - server->setProtocolVersion(std::get<1>(GetParam())); + ASSERT_TRUE(server->setProtocolVersion(std::get<1>(GetParam()))); ASSERT_EQ(OK, server->setupUnixDomainServer(addr.c_str())); auto joinEnds = std::make_shared<OneOffSignal>(); @@ -1426,7 +1431,9 @@ public: std::unique_ptr<RpcAuth> auth = std::make_unique<RpcAuthSelfSigned>()) { auto [socketType, rpcSecurity, certificateFormat, serverVersion] = param; auto rpcServer = RpcServer::make(newTlsFactory(rpcSecurity)); - rpcServer->setProtocolVersion(serverVersion); + if (!rpcServer->setProtocolVersion(serverVersion)) { + return AssertionFailure() << "Invalid protocol version: " << serverVersion; + } switch (socketType) { case SocketType::PRECONNECTED: { return AssertionFailure() << "Not supported by this test"; diff --git a/libs/binder/tests/binderRpcTestService.cpp b/libs/binder/tests/binderRpcTestService.cpp index 66221c59bd..7435f308d1 100644 --- a/libs/binder/tests/binderRpcTestService.cpp +++ b/libs/binder/tests/binderRpcTestService.cpp @@ -118,7 +118,7 @@ int main(int argc, char* argv[]) { auto certVerifier = std::make_shared<RpcCertificateVerifierSimple>(); sp<RpcServer> server = RpcServer::make(newTlsFactory(rpcSecurity, certVerifier)); - server->setProtocolVersion(serverConfig.serverVersion); + CHECK(server->setProtocolVersion(serverConfig.serverVersion)); server->setMaxThreads(serverConfig.numThreads); server->setSupportedFileDescriptorTransportModes(serverSupportedFileDescriptorTransportModes); @@ -165,7 +165,12 @@ int main(int argc, char* argv[]) { } } - server->setPerSessionRootObject([&](const void* addrPtr, size_t len) { + server->setPerSessionRootObject([&](wp<RpcSession> session, const void* addrPtr, size_t len) { + { + sp<RpcSession> spSession = session.promote(); + CHECK_NE(nullptr, spSession.get()); + } + // UNIX sockets with abstract addresses return // sizeof(sa_family_t)==2 in addrlen CHECK_GE(len, sizeof(sa_family_t)); diff --git a/libs/binder/tests/binderRpcTestServiceTrusty.cpp b/libs/binder/tests/binderRpcTestServiceTrusty.cpp index 85573895e9..cb632e95bf 100644 --- a/libs/binder/tests/binderRpcTestServiceTrusty.cpp +++ b/libs/binder/tests/binderRpcTestServiceTrusty.cpp @@ -90,15 +90,18 @@ int main(void) { auto server = std::move(*serverOrErr); serverInfo.server = server; - serverInfo.server->setProtocolVersion(serverVersion); - serverInfo.server->setPerSessionRootObject([=](const void* /*addrPtr*/, size_t /*len*/) { - auto service = sp<MyBinderRpcTestTrusty>::make(); - // Assign a unique connection identifier to service->port so - // getClientPort returns a unique value per connection - service->port = ++gConnectionCounter; - service->server = server; - return service; - }); + if (!serverInfo.server->setProtocolVersion(serverVersion)) { + return EXIT_FAILURE; + } + serverInfo.server->setPerSessionRootObject( + [=](wp<RpcSession> /*session*/, const void* /*addrPtr*/, size_t /*len*/) { + auto service = sp<MyBinderRpcTestTrusty>::make(); + // Assign a unique connection identifier to service->port so + // getClientPort returns a unique value per connection + service->port = ++gConnectionCounter; + service->server = server; + return service; + }); servers.push_back(std::move(serverInfo)); } diff --git a/libs/binder/tests/binderSafeInterfaceTest.cpp b/libs/binder/tests/binderSafeInterfaceTest.cpp index c857d62cb2..5e8a32a61b 100644 --- a/libs/binder/tests/binderSafeInterfaceTest.cpp +++ b/libs/binder/tests/binderSafeInterfaceTest.cpp @@ -35,6 +35,7 @@ #include <optional> +#include <inttypes.h> #include <sys/eventfd.h> #include <sys/prctl.h> @@ -686,10 +687,12 @@ TEST_F(SafeInterfaceTest, TestIncrementNativeHandle) { // Determine the maximum number of fds this process can have open struct rlimit limit {}; ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &limit)); - uint32_t maxFds = static_cast<uint32_t>(limit.rlim_cur); + uint64_t maxFds = limit.rlim_cur; + + ALOG(LOG_INFO, "SafeInterfaceTest", "%s max FDs: %" PRIu64, __PRETTY_FUNCTION__, maxFds); // Perform this test enough times to rule out fd leaks - for (uint32_t iter = 0; iter < (2 * maxFds); ++iter) { + for (uint32_t iter = 0; iter < (maxFds + 100); ++iter) { native_handle* handle = native_handle_create(1 /*numFds*/, 1 /*numInts*/); ASSERT_NE(nullptr, handle); handle->data[0] = dup(eventFd.get()); diff --git a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_driver.h b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_driver.h index a9a6197439..cb37cfaa27 100644 --- a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_driver.h +++ b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_driver.h @@ -19,7 +19,17 @@ #include <binder/IBinder.h> #include <fuzzer/FuzzedDataProvider.h> +#include <vector> + namespace android { + +/** + * See fuzzService, but fuzzes multiple services at the same time. + * + * Consumes providers. + */ +void fuzzService(const std::vector<sp<IBinder>>& binders, FuzzedDataProvider&& provider); + /** * Based on the random data in provider, construct an arbitrary number of * Parcel objects and send them to the service in serial. @@ -34,4 +44,5 @@ namespace android { * } */ void fuzzService(const sp<IBinder>& binder, FuzzedDataProvider&& provider); + } // namespace android diff --git a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_ndk_driver.h b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_ndk_driver.h index f2b782337c..d8bf87a58c 100644 --- a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_ndk_driver.h +++ b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/libbinder_ndk_driver.h @@ -16,10 +16,21 @@ #pragma once +#include <android/binder_auto_utils.h> #include <android/binder_parcel.h> #include <fuzzer/FuzzedDataProvider.h> +#include <vector> + namespace android { + +/** + * See fuzzService, but fuzzes multiple services at the same time. + * + * Consumes providers. + */ +void fuzzService(const std::vector<ndk::SpAIBinder>& binders, FuzzedDataProvider&& provider); + /** * Based on the random data in provider, construct an arbitrary number of * Parcel objects and send them to the service in serial. diff --git a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp index 8bef33f2ca..24a9345193 100644 --- a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp +++ b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp @@ -24,60 +24,94 @@ namespace android { void fuzzService(const sp<IBinder>& binder, FuzzedDataProvider&& provider) { - sp<IBinder> target; + fuzzService(std::vector<sp<IBinder>>{binder}, std::move(provider)); +} +void fuzzService(const std::vector<sp<IBinder>>& binders, FuzzedDataProvider&& provider) { RandomParcelOptions options{ - .extraBinders = {binder}, + .extraBinders = binders, .extraFds = {}, }; + // Always take so that a perturbation of just the one ConsumeBool byte will always + // take the same path, but with a different UID. Without this, the fuzzer needs to + // guess both the change in value and the shift at the same time. + int64_t maybeSetUid = provider.ConsumeIntegral<int64_t>(); if (provider.ConsumeBool()) { // set calling uid - IPCThreadState::self()->restoreCallingIdentity(provider.ConsumeIntegral<int64_t>()); + IPCThreadState::self()->restoreCallingIdentity(maybeSetUid); } while (provider.remaining_bytes() > 0) { - // Most of the AIDL services will have small set of transaction codes. - uint32_t code = provider.ConsumeBool() ? provider.ConsumeIntegral<uint32_t>() - : provider.ConsumeIntegralInRange<uint32_t>(0, 100); - uint32_t flags = provider.ConsumeIntegral<uint32_t>(); - Parcel data; - // for increased fuzz coverage - data.setEnforceNoDataAvail(provider.ConsumeBool()); + provider.PickValueInArray<std::function<void()>>({ + [&]() { + // Most of the AIDL services will have small set of transaction codes. + uint32_t code = provider.ConsumeBool() + ? provider.ConsumeIntegral<uint32_t>() + : provider.ConsumeIntegralInRange<uint32_t>(0, 100); + uint32_t flags = provider.ConsumeIntegral<uint32_t>(); + Parcel data; + // for increased fuzz coverage + data.setEnforceNoDataAvail(false); + data.setServiceFuzzing(); - sp<IBinder> target = options.extraBinders.at( - provider.ConsumeIntegralInRange<size_t>(0, options.extraBinders.size() - 1)); - options.writeHeader = [&target](Parcel* p, FuzzedDataProvider& provider) { - // most code will be behind checks that the head of the Parcel - // is exactly this, so make it easier for fuzzers to reach this - if (provider.ConsumeBool()) { - p->writeInterfaceToken(target->getInterfaceDescriptor()); - } - }; + sp<IBinder> target = options.extraBinders.at( + provider.ConsumeIntegralInRange<size_t>(0, + options.extraBinders.size() - + 1)); + options.writeHeader = [&target](Parcel* p, FuzzedDataProvider& provider) { + // most code will be behind checks that the head of the Parcel + // is exactly this, so make it easier for fuzzers to reach this + if (provider.ConsumeBool()) { + p->writeInterfaceToken(target->getInterfaceDescriptor()); + } + }; - std::vector<uint8_t> subData = provider.ConsumeBytes<uint8_t>( - provider.ConsumeIntegralInRange<size_t>(0, provider.remaining_bytes())); - fillRandomParcel(&data, FuzzedDataProvider(subData.data(), subData.size()), &options); + std::vector<uint8_t> subData = provider.ConsumeBytes<uint8_t>( + provider.ConsumeIntegralInRange<size_t>(0, provider.remaining_bytes())); + fillRandomParcel(&data, FuzzedDataProvider(subData.data(), subData.size()), + &options); - Parcel reply; - // for increased fuzz coverage - reply.setEnforceNoDataAvail(provider.ConsumeBool()); - (void)target->transact(code, data, &reply, flags); + Parcel reply; + // for increased fuzz coverage + reply.setEnforceNoDataAvail(false); + reply.setServiceFuzzing(); + (void)target->transact(code, data, &reply, flags); - // feed back in binders and fds that are returned from the service, so that - // we can fuzz those binders, and use the fds and binders to feed back into - // the binders - auto retBinders = reply.debugReadAllStrongBinders(); - options.extraBinders.insert(options.extraBinders.end(), retBinders.begin(), - retBinders.end()); - auto retFds = reply.debugReadAllFileDescriptors(); - for (size_t i = 0; i < retFds.size(); i++) { - options.extraFds.push_back(base::unique_fd(dup(retFds[i]))); - } + // feed back in binders and fds that are returned from the service, so that + // we can fuzz those binders, and use the fds and binders to feed back into + // the binders + auto retBinders = reply.debugReadAllStrongBinders(); + options.extraBinders.insert(options.extraBinders.end(), retBinders.begin(), + retBinders.end()); + auto retFds = reply.debugReadAllFileDescriptors(); + for (size_t i = 0; i < retFds.size(); i++) { + options.extraFds.push_back(base::unique_fd(dup(retFds[i]))); + } + }, + [&]() { + if (options.extraFds.size() == 0) { + return; + } + uint32_t toDelete = + provider.ConsumeIntegralInRange<uint32_t>(0, + options.extraFds.size() - 1); + options.extraFds.erase(options.extraFds.begin() + toDelete); + }, + [&]() { + if (options.extraBinders.size() <= 1) { + return; + } + uint32_t toDelete = + provider.ConsumeIntegralInRange<uint32_t>(0, + options.extraBinders.size() - + 1); + options.extraBinders.erase(options.extraBinders.begin() + toDelete); + }, + })(); } // invariants - auto ps = ProcessState::selfOrNull(); if (ps) { CHECK_EQ(0, ps->getThreadPoolMaxTotalThreadCount()) diff --git a/libs/binder/tests/parcel_fuzzer/libbinder_ndk_driver.cpp b/libs/binder/tests/parcel_fuzzer/libbinder_ndk_driver.cpp index a1fb70131e..0b0ca34586 100644 --- a/libs/binder/tests/parcel_fuzzer/libbinder_ndk_driver.cpp +++ b/libs/binder/tests/parcel_fuzzer/libbinder_ndk_driver.cpp @@ -24,6 +24,15 @@ namespace android { +void fuzzService(const std::vector<ndk::SpAIBinder>& binders, FuzzedDataProvider&& provider) { + std::vector<sp<IBinder>> cppBinders; + for (const auto& binder : binders) { + cppBinders.push_back(binder.get()->getBinder()); + } + + fuzzService(cppBinders, std::move(provider)); +} + void fuzzService(AIBinder* binder, FuzzedDataProvider&& provider) { fuzzService(binder->getBinder(), std::move(provider)); } diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/test_fuzzer/Android.bp new file mode 100644 index 0000000000..690c39afc9 --- /dev/null +++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/Android.bp @@ -0,0 +1,64 @@ +package { + default_applicable_licenses: ["frameworks_native_license"], +} + +aidl_interface { + name: "testServiceIface", + host_supported: true, + unstable: true, + srcs: [ + "ITestService.aidl", + ], + backend: { + java: { + enabled: true, + platform_apis: true, + }, + rust: { + enabled: true, + }, + }, +} + +// Adding this fuzzer to test the fuzzService functionality +cc_fuzz { + name: "test_service_fuzzer_should_crash", + defaults: [ + "service_fuzzer_defaults", + ], + static_libs: [ + "liblog", + "testServiceIface-cpp", + ], + host_supported: true, + srcs: ["TestServiceFuzzer.cpp"], + fuzz_config: { + triage_assignee: "waghpawan@google.com", + + // This fuzzer should be used only test fuzzService locally + fuzz_on_haiku_host: false, + fuzz_on_haiku_device: false, + }, +} + +sh_test_host { + name: "fuzz_service_test", + src: "run_fuzz_service_test.sh", + filename: "run_fuzz_service_test.sh", + test_config: "fuzz_service_test_config.xml", + data_bins: [ + "test_service_fuzzer_should_crash", + ], + required: [ + "test_service_fuzzer_should_crash", + ], + target: { + linux_bionic: { + enabled: false, + }, + darwin: { + enabled: false, + }, + }, + test_suites: ["general-tests"], +} diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/ITestService.aidl b/libs/binder/tests/parcel_fuzzer/test_fuzzer/ITestService.aidl new file mode 100644 index 0000000000..3eadc02387 --- /dev/null +++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/ITestService.aidl @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +interface ITestService { + + void setIntData(int input); + + void setCharData(char input); + + void setBooleanData(boolean input); +}
\ No newline at end of file diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/TestServiceFuzzer.cpp b/libs/binder/tests/parcel_fuzzer/test_fuzzer/TestServiceFuzzer.cpp new file mode 100644 index 0000000000..8907ea0c54 --- /dev/null +++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/TestServiceFuzzer.cpp @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <BnTestService.h> +#include <fuzzbinder/libbinder_driver.h> + +#include <log/log.h> + +using android::fuzzService; +using android::sp; +using android::binder::Status; + +namespace android { +// This service is to verify that fuzzService is functioning properly +class TestService : public BnTestService { +public: + Status setIntData(int /*input*/) { + LOG_ALWAYS_FATAL("Expected crash in setIntData"); + return Status::ok(); + } + + Status setCharData(char16_t /*input*/) { + LOG_ALWAYS_FATAL("Expected crash in setCharData"); + return Status::ok(); + } + + Status setBooleanData(bool /*input*/) { + LOG_ALWAYS_FATAL("Expected crash in setBooleanData"); + return Status::ok(); + } +}; +} // namespace android + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + auto service = sp<android::TestService>::make(); + fuzzService(service, FuzzedDataProvider(data, size)); + return 0; +} diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/fuzz_service_test_config.xml b/libs/binder/tests/parcel_fuzzer/test_fuzzer/fuzz_service_test_config.xml new file mode 100644 index 0000000000..19eb33a635 --- /dev/null +++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/fuzz_service_test_config.xml @@ -0,0 +1,22 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2023 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<configuration description="Runs fuzzService test"> + <option name="null-device" value="true" /> + <test class="com.android.tradefed.testtype.binary.ExecutableHostTest" > + <option name="binary" value="run_fuzz_service_test.sh"/> + <option name="relative-path-execution" value="true" /> + </test> +</configuration> diff --git a/libs/binder/tests/parcel_fuzzer/test_fuzzer/run_fuzz_service_test.sh b/libs/binder/tests/parcel_fuzzer/test_fuzzer/run_fuzz_service_test.sh new file mode 100644 index 0000000000..cec52fd6e7 --- /dev/null +++ b/libs/binder/tests/parcel_fuzzer/test_fuzzer/run_fuzz_service_test.sh @@ -0,0 +1,42 @@ +#!/bin/bash +# Copyright (C) 2023 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +color_success=$'\E'"[0;32m" +color_failed=$'\E'"[0;31m" +color_reset=$'\E'"[00m" + +FUZZER_NAME=test_service_fuzzer_should_crash +FUZZER_OUT=fuzzer-output + +if [ ! -f "$FUZZER_NAME" ] +then + echo -e "${color_failed}Binary $FUZZER_NAME does not exist" + echo "${color_reset}" + exit 1 +fi + +echo "INFO: Running fuzzer : test_service_fuzzer_should_crash" + +./test_service_fuzzer_should_crash -max_total_time=30 &>${FUZZER_OUT} + +echo "INFO: Searching fuzzer output for expected crashes" +if grep -q "Expected crash in set" ${FUZZER_OUT}; +then + echo -e "${color_success}Success: Found expected crash. fuzzService test successful!" +else + echo -e "${color_failed}Failed: Unable to find successful fuzzing output from test_service_fuzzer_should_crash" + echo "${color_reset}" + exit 1 +fi diff --git a/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp b/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp index 910c9dc25c..a6fd487fe5 100644 --- a/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp +++ b/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp @@ -51,8 +51,10 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { sp<RpcSession> session = RpcSession::make(); session->setMaxIncomingThreads(1); status_t status; - for (size_t tries = 0; tries < 5; tries++) { - usleep(10000); + + // b/274084938 - ASAN may be slow, wait a while + for (size_t tries = 0; tries < 50; tries++) { + usleep(100000); status = session->setupUnixDomainClient(addr.c_str()); if (status == OK) break; } diff --git a/libs/binder/trusty/RpcServerTrusty.cpp b/libs/binder/trusty/RpcServerTrusty.cpp index 68b000849c..8f643230d1 100644 --- a/libs/binder/trusty/RpcServerTrusty.cpp +++ b/libs/binder/trusty/RpcServerTrusty.cpp @@ -67,7 +67,7 @@ RpcServerTrusty::RpcServerTrusty(std::unique_ptr<RpcTransportCtx> ctx, std::stri // TODO(b/266741352): follow-up to prevent needing this in the future // Trusty needs to be set to the latest stable version that is in prebuilts there. - mRpcServer->setProtocolVersion(0); + LOG_ALWAYS_FATAL_IF(!mRpcServer->setProtocolVersion(0)); if (mPortAcl) { // Initialize the array of pointers to uuids. diff --git a/libs/binder/trusty/RpcTransportTipcTrusty.cpp b/libs/binder/trusty/RpcTransportTipcTrusty.cpp index d249b2e1eb..692f82d6cd 100644 --- a/libs/binder/trusty/RpcTransportTipcTrusty.cpp +++ b/libs/binder/trusty/RpcTransportTipcTrusty.cpp @@ -29,8 +29,6 @@ namespace android { -namespace { - // RpcTransport for Trusty. class RpcTransportTipcTrusty : public RpcTransport { public: @@ -282,8 +280,6 @@ public: std::vector<uint8_t> getCertificate(RpcCertificateFormat) const override { return {}; } }; -} // namespace - std::unique_ptr<RpcTransportCtx> RpcTransportCtxFactoryTipcTrusty::newServerCtx() const { return std::make_unique<RpcTransportCtxTipcTrusty>(); } diff --git a/libs/binder/trusty/include/binder/RpcServerTrusty.h b/libs/binder/trusty/include/binder/RpcServerTrusty.h index 6678eb8fec..8924b3679f 100644 --- a/libs/binder/trusty/include/binder/RpcServerTrusty.h +++ b/libs/binder/trusty/include/binder/RpcServerTrusty.h @@ -59,14 +59,17 @@ public: size_t msgMaxSize, std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory = nullptr); - void setProtocolVersion(uint32_t version) { mRpcServer->setProtocolVersion(version); } + [[nodiscard]] bool setProtocolVersion(uint32_t version) { + return mRpcServer->setProtocolVersion(version); + } void setSupportedFileDescriptorTransportModes( const std::vector<RpcSession::FileDescriptorTransportMode>& modes) { mRpcServer->setSupportedFileDescriptorTransportModes(modes); } void setRootObject(const sp<IBinder>& binder) { mRpcServer->setRootObject(binder); } void setRootObjectWeak(const wp<IBinder>& binder) { mRpcServer->setRootObjectWeak(binder); } - void setPerSessionRootObject(std::function<sp<IBinder>(const void*, size_t)>&& object) { + void setPerSessionRootObject( + std::function<sp<IBinder>(wp<RpcSession> session, const void*, size_t)>&& object) { mRpcServer->setPerSessionRootObject(std::move(object)); } sp<IBinder> getRootObject() { return mRpcServer->getRootObject(); } diff --git a/libs/cputimeinstate/cputimeinstate.cpp b/libs/cputimeinstate/cputimeinstate.cpp index 706704ad34..4a7bd36202 100644 --- a/libs/cputimeinstate/cputimeinstate.cpp +++ b/libs/cputimeinstate/cputimeinstate.cpp @@ -55,6 +55,7 @@ static uint32_t gNPolicies = 0; static uint32_t gNCpus = 0; static std::vector<std::vector<uint32_t>> gPolicyFreqs; static std::vector<std::vector<uint32_t>> gPolicyCpus; +static std::vector<uint32_t> gCpuIndexMap; static std::set<uint32_t> gAllFreqs; static unique_fd gTisTotalMapFd; static unique_fd gTisMapFd; @@ -108,7 +109,7 @@ static bool initGlobals() { free(dirlist[i]); } free(dirlist); - + uint32_t max_cpu_number = 0; for (const auto &policy : policyFileNames) { std::vector<uint32_t> freqs; for (const auto &name : {"available", "boost"}) { @@ -127,8 +128,19 @@ static bool initGlobals() { std::string path = StringPrintf("%s/%s/%s", basepath, policy.c_str(), "related_cpus"); auto cpus = readNumbersFromFile(path); if (!cpus) return false; + for (auto cpu : *cpus) { + if(cpu > max_cpu_number) + max_cpu_number = cpu; + } gPolicyCpus.emplace_back(*cpus); } + gCpuIndexMap = std::vector<uint32_t>(max_cpu_number+1, -1); + uint32_t cpuorder = 0; + for (const auto &cpuList : gPolicyCpus) { + for (auto cpu : cpuList) { + gCpuIndexMap[cpu] = cpuorder++; + } + } gTisTotalMapFd = unique_fd{bpf_obj_get(BPF_FS_PATH "map_timeInState_total_time_in_state_map")}; @@ -277,7 +289,7 @@ std::optional<std::vector<std::vector<uint64_t>>> getTotalCpuFreqTimes() { for (uint32_t policyIdx = 0; policyIdx < gNPolicies; ++policyIdx) { if (freqIdx >= gPolicyFreqs[policyIdx].size()) continue; for (const auto &cpu : gPolicyCpus[policyIdx]) { - out[policyIdx][freqIdx] += vals[cpu]; + out[policyIdx][freqIdx] += vals[gCpuIndexMap[cpu]]; } } } @@ -316,7 +328,8 @@ std::optional<std::vector<std::vector<uint64_t>>> getUidCpuFreqTimes(uint32_t ui auto end = nextOffset < gPolicyFreqs[j].size() ? begin + FREQS_PER_ENTRY : out[j].end(); for (const auto &cpu : gPolicyCpus[j]) { - std::transform(begin, end, std::begin(vals[cpu].ar), begin, std::plus<uint64_t>()); + std::transform(begin, end, std::begin(vals[gCpuIndexMap[cpu]].ar), begin, + std::plus<uint64_t>()); } } } @@ -382,7 +395,8 @@ getUidsUpdatedCpuFreqTimes(uint64_t *lastUpdate) { auto end = nextOffset < gPolicyFreqs[i].size() ? begin + FREQS_PER_ENTRY : map[key.uid][i].end(); for (const auto &cpu : gPolicyCpus[i]) { - std::transform(begin, end, std::begin(vals[cpu].ar), begin, std::plus<uint64_t>()); + std::transform(begin, end, std::begin(vals[gCpuIndexMap[cpu]].ar), begin, + std::plus<uint64_t>()); } } prevKey = key; @@ -437,8 +451,8 @@ std::optional<concurrent_time_t> getUidConcurrentTimes(uint32_t uid, bool retry) : ret.policy[policy].end(); for (const auto &cpu : gPolicyCpus[policy]) { - std::transform(policyBegin, policyEnd, std::begin(vals[cpu].policy), policyBegin, - std::plus<uint64_t>()); + std::transform(policyBegin, policyEnd, std::begin(vals[gCpuIndexMap[cpu]].policy), + policyBegin, std::plus<uint64_t>()); } } } @@ -506,8 +520,8 @@ std::optional<std::unordered_map<uint32_t, concurrent_time_t>> getUidsUpdatedCon : ret[key.uid].policy[policy].end(); for (const auto &cpu : gPolicyCpus[policy]) { - std::transform(policyBegin, policyEnd, std::begin(vals[cpu].policy), policyBegin, - std::plus<uint64_t>()); + std::transform(policyBegin, policyEnd, std::begin(vals[gCpuIndexMap[cpu]].policy), + policyBegin, std::plus<uint64_t>()); } } } while (prevKey = key, !getNextMapKey(gConcurrentMapFd, &prevKey, &key)); @@ -640,7 +654,7 @@ getAggregatedTaskCpuFreqTimes(pid_t tgid, const std::vector<uint16_t> &aggregati auto end = nextOffset < gPolicyFreqs[j].size() ? begin + FREQS_PER_ENTRY : map[key.aggregation_key][j].end(); for (const auto &cpu : gPolicyCpus[j]) { - std::transform(begin, end, std::begin(vals[cpu].ar), begin, + std::transform(begin, end, std::begin(vals[gCpuIndexMap[cpu]].ar), begin, std::plus<uint64_t>()); } } diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 2ac11746c3..0fe6f24842 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -214,6 +214,7 @@ cc_library_shared { shared_libs: [ "libbinder", + "libGLESv2", ], export_shared_lib_headers: [ @@ -326,7 +327,6 @@ cc_defaults { "libbase", "libcutils", "libEGL", - "libGLESv2", "libhidlbase", "liblog", "libnativewindow", diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index 9eb1a9f526..f93468036f 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -615,7 +615,8 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou BQ_LOGV("dequeueBuffer: returning slot=%d/%" PRIu64 " buf=%p flags=%#x", *outSlot, mSlots[*outSlot].mFrameNumber, - mSlots[*outSlot].mGraphicBuffer->handle, returnFlags); + mSlots[*outSlot].mGraphicBuffer != nullptr ? + mSlots[*outSlot].mGraphicBuffer->handle : nullptr, returnFlags); if (outBufferAge) { *outBufferAge = mCore->mBufferAge; diff --git a/libs/gui/OWNERS b/libs/gui/OWNERS index 05b55337ab..826a418aab 100644 --- a/libs/gui/OWNERS +++ b/libs/gui/OWNERS @@ -1,12 +1,7 @@ -adyabr@google.com -alecmouri@google.com -chaviw@google.com chrisforbes@google.com jreck@google.com -lpy@google.com -pdwilliams@google.com -racarr@google.com -vishnun@google.com + +file:/services/surfaceflinger/OWNERS per-file EndToEndNativeInputTest.cpp = svv@google.com diff --git a/libs/sensor/ISensorServer.cpp b/libs/sensor/ISensorServer.cpp index a6cacad374..93c95b98c5 100644 --- a/libs/sensor/ISensorServer.cpp +++ b/libs/sensor/ISensorServer.cpp @@ -66,7 +66,11 @@ public: v.setCapacity(n); while (n) { n--; - reply.read(s); + if(reply.read(s) != OK) { + ALOGE("Failed to read reply from getSensorList"); + v.clear(); + break; + } v.add(s); } return v; @@ -84,7 +88,11 @@ public: v.setCapacity(n); while (n) { n--; - reply.read(s); + if(reply.read(s) != OK) { + ALOGE("Failed to read reply from getDynamicSensorList"); + v.clear(); + break; + } v.add(s); } return v; diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index ec0ced8663..b865c4d5d6 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -632,7 +632,13 @@ bool Sensor::unflattenString8(void const*& buffer, size_t& size, String8& output return false; } outputString8.setTo(static_cast<char const*>(buffer), len); + + if (size < FlattenableUtils::align<4>(len)) { + ALOGE("Malformed Sensor String8 field. Should be in a 4-byte aligned buffer but is not."); + return false; + } FlattenableUtils::advance(buffer, size, FlattenableUtils::align<4>(len)); + return true; } diff --git a/libs/sensor/SensorManager.cpp b/libs/sensor/SensorManager.cpp index 0ba9704263..40061cde61 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -92,6 +92,16 @@ SensorManager& SensorManager::getInstanceForPackage(const String16& packageName) return *sensorManager; } +void SensorManager::removeInstanceForPackage(const String16& packageName) { + Mutex::Autolock _l(sLock); + auto iterator = sPackageInstances.find(packageName); + if (iterator != sPackageInstances.end()) { + SensorManager* sensorManager = iterator->second; + delete sensorManager; + sPackageInstances.erase(iterator); + } +} + SensorManager::SensorManager(const String16& opPackageName) : mSensorList(nullptr), mOpPackageName(opPackageName), mDirectConnectionHandle(1) { Mutex::Autolock _l(mLock); @@ -166,6 +176,11 @@ status_t SensorManager::assertStateLocked() { mSensors = mSensorServer->getSensorList(mOpPackageName); size_t count = mSensors.size(); + if (count == 0) { + ALOGE("Failed to get Sensor list"); + mSensorServer.clear(); + return UNKNOWN_ERROR; + } mSensorList = static_cast<Sensor const**>(malloc(count * sizeof(Sensor*))); LOG_ALWAYS_FATAL_IF(mSensorList == nullptr, "mSensorList NULL"); diff --git a/libs/sensor/include/sensor/SensorManager.h b/libs/sensor/include/sensor/SensorManager.h index 8d0a8a45d9..7c9d604ff7 100644 --- a/libs/sensor/include/sensor/SensorManager.h +++ b/libs/sensor/include/sensor/SensorManager.h @@ -54,6 +54,7 @@ class SensorManager : public ASensorManager { public: static SensorManager& getInstanceForPackage(const String16& packageName); + static void removeInstanceForPackage(const String16& packageName); ~SensorManager(); ssize_t getSensorList(Sensor const* const** list); diff --git a/opengl/libs/EGL/Loader.cpp b/opengl/libs/EGL/Loader.cpp index dd14bcfb55..6ea400721d 100644 --- a/opengl/libs/EGL/Loader.cpp +++ b/opengl/libs/EGL/Loader.cpp @@ -21,6 +21,7 @@ #include <android-base/properties.h> #include <android/dlext.h> +#include <cutils/properties.h> #include <dirent.h> #include <dlfcn.h> #include <graphicsenv/GraphicsEnv.h> @@ -236,29 +237,22 @@ void* Loader::open(egl_connection_t* cnx) LOG_ALWAYS_FATAL("couldn't find an OpenGL ES implementation from %s", android::GraphicsEnv::getInstance().getDriverPath().c_str()); } - // Finally, try to load system driver. If ANGLE is the system driver - // (i.e. we are forcing the legacy system driver instead of ANGLE), use - // the driver suffix that was passed down from above. - if (shouldForceLegacyDriver) { - std::string suffix = android::GraphicsEnv::getInstance().getLegacySuffix(); - hnd = attempt_to_load_system_driver(cnx, suffix.c_str(), true); - } else { - // Start by searching for the library name appended by the system - // properties of the GLES userspace driver in both locations. - // i.e.: - // libGLES_${prop}.so, or: - // libEGL_${prop}.so, libGLESv1_CM_${prop}.so, libGLESv2_${prop}.so - for (auto key : HAL_SUBNAME_KEY_PROPERTIES) { - auto prop = base::GetProperty(key, ""); - if (prop.empty()) { - continue; - } - hnd = attempt_to_load_system_driver(cnx, prop.c_str(), true); - if (hnd) { - break; - } else if (strcmp(key, DRIVER_SUFFIX_PROPERTY) == 0) { - failToLoadFromDriverSuffixProperty = true; - } + // Finally, try to load system driver. + // Start by searching for the library name appended by the system + // properties of the GLES userspace driver in both locations. + // i.e.: + // libGLES_${prop}.so, or: + // libEGL_${prop}.so, libGLESv1_CM_${prop}.so, libGLESv2_${prop}.so + for (auto key : HAL_SUBNAME_KEY_PROPERTIES) { + auto prop = base::GetProperty(key, ""); + if (prop.empty()) { + continue; + } + hnd = attempt_to_load_system_driver(cnx, prop.c_str(), true); + if (hnd) { + break; + } else if (strcmp(key, DRIVER_SUFFIX_PROPERTY) == 0) { + failToLoadFromDriverSuffixProperty = true; } } } @@ -272,7 +266,10 @@ void* Loader::open(egl_connection_t* cnx) hnd = attempt_to_load_system_driver(cnx, nullptr, true); } - if (!hnd && !failToLoadFromDriverSuffixProperty) { + if (!hnd && !failToLoadFromDriverSuffixProperty && + property_get_int32("ro.vendor.api_level", 0) < __ANDROID_API_U__) { + // Still can't find the graphics drivers with the exact name. This time try to use wildcard + // matching if the device is launched before Android 14. hnd = attempt_to_load_system_driver(cnx, nullptr, false); } diff --git a/opengl/libs/EGL/egl_display.cpp b/opengl/libs/EGL/egl_display.cpp index c2c856e22a..6593c1bb16 100644 --- a/opengl/libs/EGL/egl_display.cpp +++ b/opengl/libs/EGL/egl_display.cpp @@ -326,10 +326,10 @@ EGLBoolean egl_display_t::initialize(EGLint* major, EGLint* minor) { // device's present timestamps are reliable (which may not be the case on emulators). if (cnx->useAngle) { if (android::base::GetBoolProperty("service.sf.present_timestamp", false)) { - mExtensionString.append("EGL_ANDROID_get_frame_timestamps"); + mExtensionString.append("EGL_ANDROID_get_frame_timestamps "); } } else { - mExtensionString.append("EGL_ANDROID_get_frame_timestamps"); + mExtensionString.append("EGL_ANDROID_get_frame_timestamps "); } hasColorSpaceSupport = findExtension(disp.queryString.extensions, "EGL_KHR_gl_colorspace"); diff --git a/services/gpuservice/tests/fuzzers/Android.bp b/services/gpuservice/tests/fuzzers/Android.bp new file mode 100644 index 0000000000..6bcc5e8601 --- /dev/null +++ b/services/gpuservice/tests/fuzzers/Android.bp @@ -0,0 +1,26 @@ +package { + default_applicable_licenses: ["frameworks_native_license"], +} + +cc_fuzz { + name: "gpu_service_fuzzer", + defaults: [ + "service_fuzzer_defaults", + "fuzzer_disable_leaks", + ], + static_libs: [ + "liblog", + ], + fuzz_config: { + cc: [ + "paulthomson@google.com", + "pbaiget@google.com", + ], + triage_assignee: "waghpawan@google.com", + }, + include_dirs: ["frameworks/native/services/gpuservice/"], + srcs: ["GpuServiceFuzzer.cpp"], + shared_libs: [ + "libgpuservice", + ], +} diff --git a/services/gpuservice/tests/fuzzers/GpuServiceFuzzer.cpp b/services/gpuservice/tests/fuzzers/GpuServiceFuzzer.cpp new file mode 100644 index 0000000000..c2574a3fd3 --- /dev/null +++ b/services/gpuservice/tests/fuzzers/GpuServiceFuzzer.cpp @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <fuzzbinder/libbinder_driver.h> + +#include "GpuService.h" + +using ::android::fuzzService; +using ::android::GpuService; +using ::android::sp; + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + sp<GpuService> gpuService = new GpuService(); + fuzzService(gpuService, FuzzedDataProvider(data, size)); + return 0; +} diff --git a/services/sensorservice/aidl/fuzzer/Android.bp b/services/sensorservice/aidl/fuzzer/Android.bp index 0d6e476e70..cb586c6b69 100644 --- a/services/sensorservice/aidl/fuzzer/Android.bp +++ b/services/sensorservice/aidl/fuzzer/Android.bp @@ -11,6 +11,7 @@ cc_fuzz { name: "libsensorserviceaidl_fuzzer", defaults: [ "service_fuzzer_defaults", + "fuzzer_disable_leaks", ], host_supported: true, static_libs: [ diff --git a/services/sensorservice/hidl/SensorManager.cpp b/services/sensorservice/hidl/SensorManager.cpp index 938060063f..0a4e68412d 100644 --- a/services/sensorservice/hidl/SensorManager.cpp +++ b/services/sensorservice/hidl/SensorManager.cpp @@ -60,6 +60,9 @@ SensorManager::~SensorManager() { if (mPollThread.joinable()) { mPollThread.join(); } + + ::android::SensorManager::removeInstanceForPackage( + String16(ISensorManager::descriptor)); } // Methods from ::android::frameworks::sensorservice::V1_0::ISensorManager follow. |