diff options
28 files changed, 519 insertions, 74 deletions
diff --git a/cmds/dumpstate/TEST_MAPPING b/cmds/dumpstate/TEST_MAPPING index 839a2c3023..649a13ee1e 100644 --- a/cmds/dumpstate/TEST_MAPPING +++ b/cmds/dumpstate/TEST_MAPPING @@ -9,15 +9,15 @@ ] }, { - "name": "dumpstate_smoke_test" - }, - { "name": "dumpstate_test" } ], "postsubmit": [ { "name": "BugreportManagerTestCases" + }, + { + "name": "dumpstate_smoke_test" } ], "imports": [ diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index 2c8adc7126..faf67fd92f 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -125,8 +125,6 @@ static std::once_flag flag; namespace { -constexpr const char* kDump = "android.permission.DUMP"; - static binder::Status ok() { return binder::Status::ok(); } @@ -150,19 +148,6 @@ static binder::Status error(uint32_t code, const std::string& msg) { return binder::Status::fromServiceSpecificError(code, String8(msg.c_str())); } -binder::Status checkPermission(const char* permission) { - pid_t pid; - uid_t uid; - - if (checkCallingPermission(String16(permission), reinterpret_cast<int32_t*>(&pid), - reinterpret_cast<int32_t*>(&uid))) { - return ok(); - } else { - return exception(binder::Status::EX_SECURITY, - StringPrintf("UID %d / PID %d lacks permission %s", uid, pid, permission)); - } -} - binder::Status checkUid(uid_t expectedUid) { uid_t uid = IPCThreadState::self()->getCallingUid(); if (uid == expectedUid || uid == AID_ROOT) { @@ -400,13 +385,7 @@ status_t InstalldNativeService::start() { return android::OK; } -status_t InstalldNativeService::dump(int fd, const Vector<String16> & /* args */) { - const binder::Status dump_permission = checkPermission(kDump); - if (!dump_permission.isOk()) { - dprintf(fd, "%s\n", dump_permission.toString8().c_str()); - return PERMISSION_DENIED; - } - +status_t InstalldNativeService::dump(int fd, const Vector<String16>& /* args */) { { std::lock_guard<std::recursive_mutex> lock(mMountsLock); dprintf(fd, "Storage mounts:\n"); diff --git a/cmds/servicemanager/ServiceManagerFuzzer.cpp b/cmds/servicemanager/ServiceManagerFuzzer.cpp index 39f8522f84..bc48fa920e 100644 --- a/cmds/servicemanager/ServiceManagerFuzzer.cpp +++ b/cmds/servicemanager/ServiceManagerFuzzer.cpp @@ -26,9 +26,15 @@ using ::android::ServiceManager; using ::android::sp; extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + FuzzedDataProvider provider(data, size); + + // Adding this random abort to check bug pipeline. + bool shouldAbort = provider.ConsumeBool(); + if (shouldAbort) abort(); + auto accessPtr = std::make_unique<Access>(); auto serviceManager = sp<ServiceManager>::make(std::move(accessPtr)); - fuzzService(serviceManager, FuzzedDataProvider(data, size)); + fuzzService(serviceManager, std::move(provider)); return 0; } diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index 4ff629d3e5..df965ab65f 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -76,6 +76,7 @@ cc_defaults { srcs: [ "Binder.cpp", + "BinderRecordReplay.cpp", "BpBinder.cpp", "Debug.cpp", "FdTrigger.cpp", @@ -148,7 +149,10 @@ cc_defaults { }, debuggable: { - cflags: ["-DBINDER_RPC_DEV_SERVERS"], + cflags: [ + "-DBINDER_RPC_DEV_SERVERS", + "-DBINDER_ENABLE_RECORDING", + ], }, }, @@ -486,6 +490,9 @@ aidl_interface { java: { enabled: false, }, + cpp: { + gen_trace: false, + }, }, } diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index 402995717c..481d704f1c 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -21,6 +21,7 @@ #include <android-base/logging.h> #include <android-base/unique_fd.h> +#include <binder/BinderRecordReplay.h> #include <binder/BpBinder.h> #include <binder/IInterface.h> #include <binder/IPCThreadState.h> @@ -28,7 +29,9 @@ #include <binder/IShellCallback.h> #include <binder/Parcel.h> #include <binder/RpcServer.h> +#include <cutils/compiler.h> #include <private/android_filesystem_config.h> +#include <pthread.h> #include <utils/misc.h> #include <inttypes.h> @@ -60,6 +63,12 @@ bool kEnableRpcDevServers = true; bool kEnableRpcDevServers = false; #endif +#ifdef BINDER_ENABLE_RECORDING +bool kEnableRecording = true; +#else +bool kEnableRecording = false; +#endif + // Log any reply transactions for which the data exceeds this size #define LOG_REPLIES_OVER_SIZE (300 * 1024) // --------------------------------------------------------------------------- @@ -265,11 +274,13 @@ public: Mutex mLock; std::set<sp<RpcServerLink>> mRpcServerLinks; BpBinder::ObjectManager mObjects; + + android::base::unique_fd mRecordingFd; }; // --------------------------------------------------------------------------- -BBinder::BBinder() : mExtras(nullptr), mStability(0), mParceled(false) {} +BBinder::BBinder() : mExtras(nullptr), mStability(0), mParceled(false), mRecordingOn(false) {} bool BBinder::isBinderAlive() const { @@ -281,6 +292,63 @@ status_t BBinder::pingBinder() return NO_ERROR; } +status_t BBinder::startRecordingTransactions(const Parcel& data) { + if (!kEnableRecording) { + ALOGW("Binder recording disallowed because recording is not enabled"); + return INVALID_OPERATION; + } + if (!kEnableKernelIpc) { + ALOGW("Binder recording disallowed because kernel binder is not enabled"); + return INVALID_OPERATION; + } + uid_t uid = IPCThreadState::self()->getCallingUid(); + if (uid != AID_ROOT) { + ALOGE("Binder recording not allowed because client %" PRIu32 " is not root", uid); + return PERMISSION_DENIED; + } + Extras* e = getOrCreateExtras(); + AutoMutex lock(e->mLock); + if (mRecordingOn) { + LOG(INFO) << "Could not start Binder recording. Another is already in progress."; + return INVALID_OPERATION; + } else { + status_t readStatus = data.readUniqueFileDescriptor(&(e->mRecordingFd)); + if (readStatus != OK) { + return readStatus; + } + mRecordingOn = true; + LOG(INFO) << "Started Binder recording."; + return NO_ERROR; + } +} + +status_t BBinder::stopRecordingTransactions() { + if (!kEnableRecording) { + ALOGW("Binder recording disallowed because recording is not enabled"); + return INVALID_OPERATION; + } + if (!kEnableKernelIpc) { + ALOGW("Binder recording disallowed because kernel binder is not enabled"); + return INVALID_OPERATION; + } + uid_t uid = IPCThreadState::self()->getCallingUid(); + if (uid != AID_ROOT) { + ALOGE("Binder recording not allowed because client %" PRIu32 " is not root", uid); + return PERMISSION_DENIED; + } + Extras* e = getOrCreateExtras(); + AutoMutex lock(e->mLock); + if (mRecordingOn) { + e->mRecordingFd.reset(); + mRecordingOn = false; + LOG(INFO) << "Stopped Binder recording."; + return NO_ERROR; + } else { + LOG(INFO) << "Could not stop Binder recording. One is not in progress."; + return INVALID_OPERATION; + } +} + const String16& BBinder::getInterfaceDescriptor() const { static StaticString16 sBBinder(u"BBinder"); @@ -303,6 +371,12 @@ status_t BBinder::transact( case PING_TRANSACTION: err = pingBinder(); break; + case START_RECORDING_TRANSACTION: + err = startRecordingTransactions(data); + break; + case STOP_RECORDING_TRANSACTION: + err = stopRecordingTransactions(); + break; case EXTENSION_TRANSACTION: CHECK(reply != nullptr); err = reply->writeStrongBinder(getExtension()); @@ -329,6 +403,26 @@ status_t BBinder::transact( } } + if (CC_UNLIKELY(kEnableKernelIpc && mRecordingOn && code != START_RECORDING_TRANSACTION)) { + Extras* e = mExtras.load(std::memory_order_acquire); + AutoMutex lock(e->mLock); + if (mRecordingOn) { + Parcel emptyReply; + auto transaction = + android::binder::debug::RecordedTransaction::fromDetails(code, flags, data, + reply ? *reply + : emptyReply, + err); + if (transaction) { + if (status_t err = transaction->dumpToFile(e->mRecordingFd); err != NO_ERROR) { + LOG(INFO) << "Failed to dump RecordedTransaction to file with error " << err; + } + } else { + LOG(INFO) << "Failed to create RecordedTransaction object."; + } + } + } + return err; } diff --git a/libs/binder/BinderRecordReplay.cpp b/libs/binder/BinderRecordReplay.cpp new file mode 100644 index 0000000000..90c02a80b9 --- /dev/null +++ b/libs/binder/BinderRecordReplay.cpp @@ -0,0 +1,185 @@ +/* + * Copyright (C) 2022, The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <android-base/file.h> +#include <android-base/logging.h> +#include <binder/BinderRecordReplay.h> +#include <algorithm> + +using android::Parcel; +using android::base::unique_fd; +using android::binder::debug::RecordedTransaction; + +#define PADDING8(s) ((8 - (s) % 8) % 8) + +static_assert(PADDING8(0) == 0); +static_assert(PADDING8(1) == 7); +static_assert(PADDING8(7) == 1); +static_assert(PADDING8(8) == 0); + +// Transactions are sequentially recorded to the file descriptor in the following format: +// +// RecordedTransaction.TransactionHeader (32 bytes) +// Sent Parcel data (getDataSize() bytes) +// padding (enough bytes to align the reply Parcel data to 8 bytes) +// Reply Parcel data (getReplySize() bytes) +// padding (enough bytes to align the next header to 8 bytes) +// [repeats with next transaction] +// +// Warning: This format is non-stable + +RecordedTransaction::RecordedTransaction(RecordedTransaction&& t) noexcept { + mHeader = {t.getCode(), t.getFlags(), t.getDataSize(), + t.getReplySize(), t.getReturnedStatus(), t.getVersion()}; + mSent.setData(t.getDataParcel().data(), t.getDataSize()); + mReply.setData(t.getReplyParcel().data(), t.getReplySize()); +} + +std::optional<RecordedTransaction> RecordedTransaction::fromDetails(uint32_t code, uint32_t flags, + const Parcel& dataParcel, + const Parcel& replyParcel, + status_t err) { + RecordedTransaction t; + t.mHeader = {code, + flags, + static_cast<uint64_t>(dataParcel.dataSize()), + static_cast<uint64_t>(replyParcel.dataSize()), + static_cast<int32_t>(err), + dataParcel.isForRpc() ? static_cast<uint32_t>(1) : static_cast<uint32_t>(0)}; + + if (t.mSent.setData(dataParcel.data(), t.getDataSize()) != android::NO_ERROR) { + LOG(INFO) << "Failed to set sent parcel data."; + return std::nullopt; + } + + if (t.mReply.setData(replyParcel.data(), t.getReplySize()) != android::NO_ERROR) { + LOG(INFO) << "Failed to set reply parcel data."; + return std::nullopt; + } + + return std::optional<RecordedTransaction>(std::move(t)); +} + +std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd& fd) { + RecordedTransaction t; + if (!android::base::ReadFully(fd, &t.mHeader, sizeof(mHeader))) { + LOG(INFO) << "Failed to read transactionHeader from fd " << fd.get(); + return std::nullopt; + } + if (t.getVersion() != 0) { + LOG(INFO) << "File corrupted: transaction version is not 0."; + return std::nullopt; + } + + std::vector<uint8_t> bytes; + bytes.resize(t.getDataSize()); + if (!android::base::ReadFully(fd, bytes.data(), t.getDataSize())) { + LOG(INFO) << "Failed to read sent parcel data from fd " << fd.get(); + return std::nullopt; + } + if (t.mSent.setData(bytes.data(), t.getDataSize()) != android::NO_ERROR) { + LOG(INFO) << "Failed to set sent parcel data."; + return std::nullopt; + } + + uint8_t padding[7]; + if (!android::base::ReadFully(fd, padding, PADDING8(t.getDataSize()))) { + LOG(INFO) << "Failed to read sent parcel padding from fd " << fd.get(); + return std::nullopt; + } + if (std::any_of(padding, padding + 7, [](uint8_t i) { return i != 0; })) { + LOG(INFO) << "File corrupted: padding isn't 0."; + return std::nullopt; + } + + bytes.resize(t.getReplySize()); + if (!android::base::ReadFully(fd, bytes.data(), t.getReplySize())) { + LOG(INFO) << "Failed to read reply parcel data from fd " << fd.get(); + return std::nullopt; + } + if (t.mReply.setData(bytes.data(), t.getReplySize()) != android::NO_ERROR) { + LOG(INFO) << "Failed to set reply parcel data."; + return std::nullopt; + } + + if (!android::base::ReadFully(fd, padding, PADDING8(t.getReplySize()))) { + LOG(INFO) << "Failed to read parcel padding from fd " << fd.get(); + return std::nullopt; + } + if (std::any_of(padding, padding + 7, [](uint8_t i) { return i != 0; })) { + LOG(INFO) << "File corrupted: padding isn't 0."; + return std::nullopt; + } + + return std::optional<RecordedTransaction>(std::move(t)); +} + +android::status_t RecordedTransaction::dumpToFile(const unique_fd& fd) const { + if (!android::base::WriteFully(fd, &mHeader, sizeof(mHeader))) { + LOG(INFO) << "Failed to write transactionHeader to fd " << fd.get(); + return UNKNOWN_ERROR; + } + if (!android::base::WriteFully(fd, mSent.data(), getDataSize())) { + LOG(INFO) << "Failed to write sent parcel data to fd " << fd.get(); + return UNKNOWN_ERROR; + } + const uint8_t zeros[7] = {0}; + if (!android::base::WriteFully(fd, zeros, PADDING8(getDataSize()))) { + LOG(INFO) << "Failed to write sent parcel padding to fd " << fd.get(); + return UNKNOWN_ERROR; + } + if (!android::base::WriteFully(fd, mReply.data(), getReplySize())) { + LOG(INFO) << "Failed to write reply parcel data to fd " << fd.get(); + return UNKNOWN_ERROR; + } + if (!android::base::WriteFully(fd, zeros, PADDING8(getReplySize()))) { + LOG(INFO) << "Failed to write reply parcel padding to fd " << fd.get(); + return UNKNOWN_ERROR; + } + return NO_ERROR; +} + +uint32_t RecordedTransaction::getCode() const { + return mHeader.code; +} + +uint32_t RecordedTransaction::getFlags() const { + return mHeader.flags; +} + +uint64_t RecordedTransaction::getDataSize() const { + return mHeader.dataSize; +} + +uint64_t RecordedTransaction::getReplySize() const { + return mHeader.replySize; +} + +int32_t RecordedTransaction::getReturnedStatus() const { + return mHeader.statusReturned; +} + +uint32_t RecordedTransaction::getVersion() const { + return mHeader.version; +} + +const Parcel& RecordedTransaction::getDataParcel() const { + return mSent; +} + +const Parcel& RecordedTransaction::getReplyParcel() const { + return mReply; +} diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp index d9b723108e..54d24457d4 100644 --- a/libs/binder/BpBinder.cpp +++ b/libs/binder/BpBinder.cpp @@ -30,6 +30,8 @@ #include "BuildFlags.h" +#include <android-base/file.h> + //#undef ALOGV //#define ALOGV(...) fprintf(stderr, __VA_ARGS__) @@ -299,6 +301,18 @@ status_t BpBinder::pingBinder() return transact(PING_TRANSACTION, data, &reply); } +status_t BpBinder::startRecordingBinder(const android::base::unique_fd& fd) { + Parcel send, reply; + send.writeUniqueFileDescriptor(fd); + return transact(START_RECORDING_TRANSACTION, send, &reply); +} + +status_t BpBinder::stopRecordingBinder() { + Parcel data, reply; + data.markForBinder(sp<BpBinder>::fromExisting(this)); + return transact(STOP_RECORDING_TRANSACTION, data, &reply); +} + status_t BpBinder::dump(int fd, const Vector<String16>& args) { Parcel send; diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index b50cfb3d19..bfcf39ad30 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -1318,6 +1318,13 @@ status_t IPCThreadState::executeCommand(int32_t cmd) LOG_ONEWAY("Sending reply to %d!", mCallingPid); if (error < NO_ERROR) reply.setError(error); + // b/238777741: clear buffer before we send the reply. + // Otherwise, there is a race where the client may + // receive the reply and send another transaction + // here and the space used by this transaction won't + // be freed for the client. + buffer.setDataSize(0); + constexpr uint32_t kForwardReplyFlags = TF_CLEAR_BUF; sendReply(reply, (tr.flags & kForwardReplyFlags)); } else { diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index c0a8d74195..5db3eef392 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#define LOG_TAG "ServiceManager" +#define LOG_TAG "ServiceManagerCppClient" #include <binder/IServiceManager.h> diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h index 88d9ca1d58..08dbd13bf4 100644 --- a/libs/binder/include/binder/Binder.h +++ b/libs/binder/include/binder/Binder.h @@ -105,6 +105,12 @@ public: [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd, const sp<IBinder>& keepAliveBinder); + // Start recording transactions to the unique_fd in data. + // See BinderRecordReplay.h for more details. + [[nodiscard]] status_t startRecordingTransactions(const Parcel& data); + // Stop the current recording. + [[nodiscard]] status_t stopRecordingTransactions(); + protected: virtual ~BBinder(); @@ -131,7 +137,7 @@ private: friend ::android::internal::Stability; int16_t mStability; bool mParceled; - uint8_t mReserved0; + bool mRecordingOn; #ifdef __LP64__ int32_t mReserved1; diff --git a/libs/binder/include/binder/BinderRecordReplay.h b/libs/binder/include/binder/BinderRecordReplay.h new file mode 100644 index 0000000000..25ed5e5ff8 --- /dev/null +++ b/libs/binder/include/binder/BinderRecordReplay.h @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2022, The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <android-base/unique_fd.h> +#include <binder/Parcel.h> +#include <mutex> + +namespace android { + +namespace binder::debug { + +// Warning: Transactions are sequentially recorded to the file descriptor in a +// non-stable format. A detailed description of the recording format can be found in +// BinderRecordReplay.cpp. + +class RecordedTransaction { +public: + // Filled with the first transaction from fd. + static std::optional<RecordedTransaction> fromFile(const android::base::unique_fd& fd); + // Filled with the arguments. + static std::optional<RecordedTransaction> fromDetails(uint32_t code, uint32_t flags, + const Parcel& data, const Parcel& reply, + status_t err); + RecordedTransaction(RecordedTransaction&& t) noexcept; + + [[nodiscard]] status_t dumpToFile(const android::base::unique_fd& fd) const; + + uint32_t getCode() const; + uint32_t getFlags() const; + uint64_t getDataSize() const; + uint64_t getReplySize() const; + int32_t getReturnedStatus() const; + uint32_t getVersion() const; + const Parcel& getDataParcel() const; + const Parcel& getReplyParcel() const; + +private: + RecordedTransaction() = default; + +#pragma clang diagnostic push +#pragma clang diagnostic error "-Wpadded" + struct TransactionHeader { + uint32_t code = 0; + uint32_t flags = 0; + uint64_t dataSize = 0; + uint64_t replySize = 0; + int32_t statusReturned = 0; + uint32_t version = 0; // !0 iff Rpc + }; +#pragma clang diagnostic pop + static_assert(sizeof(TransactionHeader) == 32); + static_assert(sizeof(TransactionHeader) % 8 == 0); + + TransactionHeader mHeader; + Parcel mSent; + Parcel mReply; +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-private-field" + uint8_t mReserved[40]; +#pragma clang diagnostic pop +}; + +} // namespace binder::debug + +} // namespace android diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h index 4172cc511e..57e103d39e 100644 --- a/libs/binder/include/binder/BpBinder.h +++ b/libs/binder/include/binder/BpBinder.h @@ -16,6 +16,7 @@ #pragma once +#include <android-base/unique_fd.h> #include <binder/IBinder.h> #include <utils/Mutex.h> @@ -89,6 +90,12 @@ public: std::optional<int32_t> getDebugBinderHandle() const; + // Start recording transactions to the unique_fd. + // See BinderRecordReplay.h for more details. + status_t startRecordingBinder(const android::base::unique_fd& fd); + // Stop the current recording. + status_t stopRecordingBinder(); + class ObjectManager { public: ObjectManager(); diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h index 83aaca7f95..e75d548bd8 100644 --- a/libs/binder/include/binder/IBinder.h +++ b/libs/binder/include/binder/IBinder.h @@ -56,6 +56,8 @@ public: LAST_CALL_TRANSACTION = 0x00ffffff, PING_TRANSACTION = B_PACK_CHARS('_', 'P', 'N', 'G'), + START_RECORDING_TRANSACTION = B_PACK_CHARS('_', 'S', 'R', 'D'), + STOP_RECORDING_TRANSACTION = B_PACK_CHARS('_', 'E', 'R', 'D'), DUMP_TRANSACTION = B_PACK_CHARS('_', 'D', 'M', 'P'), SHELL_COMMAND_TRANSACTION = B_PACK_CHARS('_', 'C', 'M', 'D'), INTERFACE_TRANSACTION = B_PACK_CHARS('_', 'N', 'T', 'F'), diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index e460d2c965..92d132f37c 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -605,6 +605,7 @@ cc_test { shared_libs: [ "libbinder", "liblog", + "libcutils", "libutils", "libutilscallstack", "libbase", diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp index a2ab8ab302..55a3916112 100644 --- a/libs/binder/tests/binderAllocationLimits.cpp +++ b/libs/binder/tests/binderAllocationLimits.cpp @@ -20,6 +20,7 @@ #include <binder/Parcel.h> #include <binder/RpcServer.h> #include <binder/RpcSession.h> +#include <cutils/trace.h> #include <gtest/gtest.h> #include <utils/CallStack.h> @@ -223,5 +224,10 @@ int main(int argc, char** argv) { return 1; } ::testing::InitGoogleTest(&argc, argv); + + // if tracing is enabled, take in one-time cost + (void)ATRACE_INIT(); + (void)ATRACE_GET_ENABLED_TAGS(); + return RUN_ALL_TESTS(); } diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 5de08bdc00..6e1c8ac16a 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -1161,8 +1161,7 @@ TEST_F(BinderLibTest, VectorSent) { // see ProcessState.cpp BINDER_VM_SIZE = 1MB. // This value is not exposed, but some code in the framework relies on being able to use // buffers near the cap size. -// TODO(b/238777741): why do larger values, like 300K fail sometimes -constexpr size_t kSizeBytesAlmostFull = 100'000; +constexpr size_t kSizeBytesAlmostFull = 950'000; constexpr size_t kSizeBytesOverFull = 1'050'000; TEST_F(BinderLibTest, GargantuanVectorSent) { diff --git a/libs/fakeservicemanager/Android.bp b/libs/fakeservicemanager/Android.bp index 47c0657bbd..29924ff500 100644 --- a/libs/fakeservicemanager/Android.bp +++ b/libs/fakeservicemanager/Android.bp @@ -28,6 +28,7 @@ cc_defaults { cc_library { name: "libfakeservicemanager", defaults: ["fakeservicemanager_defaults"], + export_include_dirs: ["include/fakeservicemanager"], } cc_test_host { @@ -37,4 +38,5 @@ cc_test_host { "test_sm.cpp", ], static_libs: ["libgmock"], + local_include_dirs: ["include/fakeservicemanager"], } diff --git a/libs/fakeservicemanager/ServiceManager.h b/libs/fakeservicemanager/include/fakeservicemanager/ServiceManager.h index e0af5d4ba8..e0af5d4ba8 100644 --- a/libs/fakeservicemanager/ServiceManager.h +++ b/libs/fakeservicemanager/include/fakeservicemanager/ServiceManager.h diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index 13ca9ecd35..4127f7ce10 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -929,6 +929,8 @@ std::ostream& operator<<(std::ostream& out, const MotionEvent& event) { out << ", actionButton=" << std::to_string(event.getActionButton()); } const size_t pointerCount = event.getPointerCount(); + LOG_ALWAYS_FATAL_IF(pointerCount > MAX_POINTERS, "Too many pointers : pointerCount = %zu", + pointerCount); for (size_t i = 0; i < pointerCount; i++) { out << ", id[" << i << "]=" << event.getPointerId(i); float x = event.getX(i); diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp index 3732fee7f2..429760ffe0 100644 --- a/libs/ui/GraphicBuffer.cpp +++ b/libs/ui/GraphicBuffer.cpp @@ -465,7 +465,7 @@ status_t GraphicBuffer::unflatten(void const*& buffer, size_t& size, int const*& if (flattenWordCount == 13) { usage = (uint64_t(buf[12]) << 32) | uint32_t(buf[6]); } else { - usage = uint64_t(usage_deprecated); + usage = uint64_t(ANDROID_NATIVE_UNSIGNED_CAST(usage_deprecated)); } native_handle* h = native_handle_create(static_cast<int>(numFds), static_cast<int>(numInts)); diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index eef20a96d0..20baa42a20 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -1485,25 +1485,35 @@ EventHub::Device* EventHub::getDeviceByFdLocked(int fd) const { } std::optional<int32_t> EventHub::getBatteryCapacity(int32_t deviceId, int32_t batteryId) const { - std::scoped_lock _l(mLock); + std::filesystem::path batteryPath; + { + // Do not read the sysfs node to get the battery state while holding + // the EventHub lock. For some peripheral devices, reading battery state + // can be broken and take 5+ seconds. Holding the lock in this case would + // block all other event processing during this time. For now, we assume this + // call never happens on the InputReader thread and read the sysfs node outside + // the lock to prevent event processing from being blocked by this call. + std::scoped_lock _l(mLock); + + const auto infos = getBatteryInfoLocked(deviceId); + auto it = infos.find(batteryId); + if (it == infos.end()) { + return std::nullopt; + } + batteryPath = it->second.path; + } // release lock - const auto infos = getBatteryInfoLocked(deviceId); - auto it = infos.find(batteryId); - if (it == infos.end()) { - return std::nullopt; - } std::string buffer; // Some devices report battery capacity as an integer through the "capacity" file - if (base::ReadFileToString(it->second.path / BATTERY_NODES.at(InputBatteryClass::CAPACITY), + if (base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::CAPACITY), &buffer)) { return std::stoi(base::Trim(buffer)); } // Other devices report capacity as an enum value POWER_SUPPLY_CAPACITY_LEVEL_XXX // These values are taken from kernel source code include/linux/power_supply.h - if (base::ReadFileToString(it->second.path / - BATTERY_NODES.at(InputBatteryClass::CAPACITY_LEVEL), + if (base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::CAPACITY_LEVEL), &buffer)) { // Remove any white space such as trailing new line const auto levelIt = BATTERY_LEVEL.find(base::Trim(buffer)); @@ -1516,15 +1526,27 @@ std::optional<int32_t> EventHub::getBatteryCapacity(int32_t deviceId, int32_t ba } std::optional<int32_t> EventHub::getBatteryStatus(int32_t deviceId, int32_t batteryId) const { - std::scoped_lock _l(mLock); - const auto infos = getBatteryInfoLocked(deviceId); - auto it = infos.find(batteryId); - if (it == infos.end()) { - return std::nullopt; - } + std::filesystem::path batteryPath; + { + // Do not read the sysfs node to get the battery state while holding + // the EventHub lock. For some peripheral devices, reading battery state + // can be broken and take 5+ seconds. Holding the lock in this case would + // block all other event processing during this time. For now, we assume this + // call never happens on the InputReader thread and read the sysfs node outside + // the lock to prevent event processing from being blocked by this call. + std::scoped_lock _l(mLock); + + const auto infos = getBatteryInfoLocked(deviceId); + auto it = infos.find(batteryId); + if (it == infos.end()) { + return std::nullopt; + } + batteryPath = it->second.path; + } // release lock + std::string buffer; - if (!base::ReadFileToString(it->second.path / BATTERY_NODES.at(InputBatteryClass::STATUS), + if (!base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::STATUS), &buffer)) { ALOGE("Failed to read sysfs battery info: %s", strerror(errno)); return std::nullopt; diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp index ba5083bec3..0d73e072fd 100644 --- a/services/inputflinger/reader/InputDevice.cpp +++ b/services/inputflinger/reader/InputDevice.cpp @@ -547,14 +547,6 @@ void InputDevice::cancelTouch(nsecs_t when, nsecs_t readTime) { for_each_mapper([when, readTime](InputMapper& mapper) { mapper.cancelTouch(when, readTime); }); } -std::optional<int32_t> InputDevice::getBatteryCapacity() { - return mController ? mController->getBatteryCapacity(DEFAULT_BATTERY_ID) : std::nullopt; -} - -std::optional<int32_t> InputDevice::getBatteryStatus() { - return mController ? mController->getBatteryStatus(DEFAULT_BATTERY_ID) : std::nullopt; -} - bool InputDevice::setLightColor(int32_t lightId, int32_t color) { return mController ? mController->setLightColor(lightId, color) : false; } @@ -622,6 +614,10 @@ void InputDevice::updateLedState(bool reset) { for_each_mapper([reset](InputMapper& mapper) { mapper.updateLedState(reset); }); } +std::optional<int32_t> InputDevice::getBatteryEventHubId() const { + return mController ? std::make_optional(mController->getEventHubId()) : std::nullopt; +} + InputDeviceContext::InputDeviceContext(InputDevice& device, int32_t eventHubId) : mDevice(device), mContext(device.getContext()), diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index a378ba84cf..e9a82b4d4d 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -705,23 +705,43 @@ void InputReader::flushSensor(int32_t deviceId, InputDeviceSensorType sensorType } std::optional<int32_t> InputReader::getBatteryCapacity(int32_t deviceId) { - std::scoped_lock _l(mLock); + std::optional<int32_t> eventHubId; + { + // Do not query the battery state while holding the lock. For some peripheral devices, + // reading battery state can be broken and take 5+ seconds. Holding the lock in this case + // would block all other event processing during this time. For now, we assume this + // call never happens on the InputReader thread and get the battery state outside the + // lock to prevent event processing from being blocked by this call. + std::scoped_lock _l(mLock); + InputDevice* device = findInputDeviceLocked(deviceId); + if (!device) return {}; + eventHubId = device->getBatteryEventHubId(); + } // release lock - InputDevice* device = findInputDeviceLocked(deviceId); - if (device) { - return device->getBatteryCapacity(); - } - return std::nullopt; + if (!eventHubId) return {}; + const auto batteryIds = mEventHub->getRawBatteryIds(*eventHubId); + if (batteryIds.empty()) return {}; + return mEventHub->getBatteryCapacity(*eventHubId, batteryIds.front()); } std::optional<int32_t> InputReader::getBatteryStatus(int32_t deviceId) { - std::scoped_lock _l(mLock); + std::optional<int32_t> eventHubId; + { + // Do not query the battery state while holding the lock. For some peripheral devices, + // reading battery state can be broken and take 5+ seconds. Holding the lock in this case + // would block all other event processing during this time. For now, we assume this + // call never happens on the InputReader thread and get the battery state outside the + // lock to prevent event processing from being blocked by this call. + std::scoped_lock _l(mLock); + InputDevice* device = findInputDeviceLocked(deviceId); + if (!device) return {}; + eventHubId = device->getBatteryEventHubId(); + } // release lock - InputDevice* device = findInputDeviceLocked(deviceId); - if (device) { - return device->getBatteryStatus(); - } - return std::nullopt; + if (!eventHubId) return {}; + const auto batteryIds = mEventHub->getRawBatteryIds(*eventHubId); + if (batteryIds.empty()) return {}; + return mEventHub->getBatteryStatus(*eventHubId, batteryIds.front()); } std::vector<InputDeviceLightInfo> InputReader::getLights(int32_t deviceId) { diff --git a/services/inputflinger/reader/controller/PeripheralController.cpp b/services/inputflinger/reader/controller/PeripheralController.cpp index a6934960c9..8065f57524 100644 --- a/services/inputflinger/reader/controller/PeripheralController.cpp +++ b/services/inputflinger/reader/controller/PeripheralController.cpp @@ -524,4 +524,8 @@ std::optional<int32_t> PeripheralController::getLightPlayerId(int32_t lightId) { return light->getLightPlayerId(); } +int32_t PeripheralController::getEventHubId() const { + return getDeviceContext().getEventHubId(); +} + } // namespace android diff --git a/services/inputflinger/reader/controller/PeripheralController.h b/services/inputflinger/reader/controller/PeripheralController.h index b1bc8c732c..ac951ebe2a 100644 --- a/services/inputflinger/reader/controller/PeripheralController.h +++ b/services/inputflinger/reader/controller/PeripheralController.h @@ -31,6 +31,7 @@ public: explicit PeripheralController(InputDeviceContext& deviceContext); ~PeripheralController() override; + int32_t getEventHubId() const override; void populateDeviceInfo(InputDeviceInfo* deviceInfo) override; void dump(std::string& dump) override; bool setLightColor(int32_t lightId, int32_t color) override; @@ -43,6 +44,7 @@ public: private: inline int32_t getDeviceId() { return mDeviceContext.getId(); } inline InputDeviceContext& getDeviceContext() { return mDeviceContext; } + inline InputDeviceContext& getDeviceContext() const { return mDeviceContext; } InputDeviceContext& mDeviceContext; void configureLights(); diff --git a/services/inputflinger/reader/controller/PeripheralControllerInterface.h b/services/inputflinger/reader/controller/PeripheralControllerInterface.h index 7688a431d1..306e36119b 100644 --- a/services/inputflinger/reader/controller/PeripheralControllerInterface.h +++ b/services/inputflinger/reader/controller/PeripheralControllerInterface.h @@ -33,6 +33,8 @@ public: PeripheralControllerInterface() {} virtual ~PeripheralControllerInterface() {} + virtual int32_t getEventHubId() const = 0; + // Interface methods for Battery virtual std::optional<int32_t> getBatteryCapacity(int32_t batteryId) = 0; virtual std::optional<int32_t> getBatteryStatus(int32_t batteryId) = 0; diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h index 728020eadc..b3a24af8a9 100644 --- a/services/inputflinger/reader/include/InputDevice.h +++ b/services/inputflinger/reader/include/InputDevice.h @@ -32,8 +32,6 @@ #include "InputReaderContext.h" namespace android { -// TODO b/180733860 support multiple battery in API and remove this. -constexpr int32_t DEFAULT_BATTERY_ID = 1; class PeripheralController; class PeripheralControllerInterface; @@ -100,8 +98,7 @@ public: void disableSensor(InputDeviceSensorType sensorType); void flushSensor(InputDeviceSensorType sensorType); - std::optional<int32_t> getBatteryCapacity(); - std::optional<int32_t> getBatteryStatus(); + std::optional<int32_t> getBatteryEventHubId() const; bool setLightColor(int32_t lightId, int32_t color); bool setLightPlayerId(int32_t lightId, int32_t playerId); diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index 2ccb35e8cb..db4669967f 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -987,7 +987,9 @@ private: return BATTERY_STATUS; } - const std::vector<int32_t> getRawBatteryIds(int32_t deviceId) { return {}; } + const std::vector<int32_t> getRawBatteryIds(int32_t deviceId) override { + return {DEFAULT_BATTERY}; + } std::optional<RawBatteryInfo> getRawBatteryInfo(int32_t deviceId, int32_t batteryId) { return std::nullopt; @@ -2137,6 +2139,8 @@ public: ~FakePeripheralController() override {} + int32_t getEventHubId() const { return getDeviceContext().getEventHubId(); } + void populateDeviceInfo(InputDeviceInfo* deviceInfo) override {} void dump(std::string& dump) override {} @@ -2170,6 +2174,7 @@ private: InputDeviceContext& mDeviceContext; inline int32_t getDeviceId() { return mDeviceContext.getId(); } inline InputDeviceContext& getDeviceContext() { return mDeviceContext; } + inline InputDeviceContext& getDeviceContext() const { return mDeviceContext; } }; TEST_F(InputReaderTest, BatteryGetCapacity) { |