diff options
39 files changed, 1119 insertions, 150 deletions
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index ef2fa4dff7..fa7cb64f3a 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -505,8 +505,9 @@ Status ServiceManager::addService(const std::string& name, const sp<IBinder>& bi return Status::fromExceptionCode(Status::EX_SECURITY, "App UIDs cannot add services."); } - if (!mAccess->canAdd(ctx, name)) { - return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied."); + std::optional<std::string> accessorName; + if (auto status = canAddService(ctx, name, &accessorName); !status.isOk()) { + return status; } if (binder == nullptr) { @@ -888,8 +889,9 @@ Status ServiceManager::registerClientCallback(const std::string& name, const sp< } auto ctx = mAccess->getCallingContext(); - if (!mAccess->canAdd(ctx, name)) { - return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied."); + std::optional<std::string> accessorName; + if (auto status = canAddService(ctx, name, &accessorName); !status.isOk()) { + return status; } auto serviceIt = mNameToService.find(name); @@ -1051,8 +1053,9 @@ Status ServiceManager::tryUnregisterService(const std::string& name, const sp<IB } auto ctx = mAccess->getCallingContext(); - if (!mAccess->canAdd(ctx, name)) { - return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied."); + std::optional<std::string> accessorName; + if (auto status = canAddService(ctx, name, &accessorName); !status.isOk()) { + return status; } auto serviceIt = mNameToService.find(name); @@ -1110,6 +1113,23 @@ Status ServiceManager::tryUnregisterService(const std::string& name, const sp<IB return Status::ok(); } +Status ServiceManager::canAddService(const Access::CallingContext& ctx, const std::string& name, + std::optional<std::string>* accessor) { + if (!mAccess->canAdd(ctx, name)) { + return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied for service."); + } +#ifndef VENDORSERVICEMANAGER + *accessor = getVintfAccessorName(name); +#endif + if (accessor->has_value()) { + if (!mAccess->canAdd(ctx, accessor->value())) { + return Status::fromExceptionCode(Status::EX_SECURITY, + "SELinux denied for the accessor of the service."); + } + } + return Status::ok(); +} + Status ServiceManager::canFindService(const Access::CallingContext& ctx, const std::string& name, std::optional<std::string>* accessor) { if (!mAccess->canFind(ctx, name)) { diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h index 0d666c6bce..c92141b393 100644 --- a/cmds/servicemanager/ServiceManager.h +++ b/cmds/servicemanager/ServiceManager.h @@ -115,6 +115,8 @@ private: os::Service tryGetService(const std::string& name, bool startIfNotFound); sp<IBinder> tryGetBinder(const std::string& name, bool startIfNotFound); + binder::Status canAddService(const Access::CallingContext& ctx, const std::string& name, + std::optional<std::string>* accessor); binder::Status canFindService(const Access::CallingContext& ctx, const std::string& name, std::optional<std::string>* accessor); diff --git a/include/input/OWNERS b/include/input/OWNERS index c88bfe97ca..21d208f577 100644 --- a/include/input/OWNERS +++ b/include/input/OWNERS @@ -1 +1,2 @@ +# Bug component: 136048 include platform/frameworks/base:/INPUT_OWNERS diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index c57c9cdd62..53bd08d420 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -143,6 +143,22 @@ status_t IBinder::getExtension(sp<IBinder>* out) { return reply.readNullableStrongBinder(out); } +status_t IBinder::addFrozenStateChangeCallback(const wp<FrozenStateChangeCallback>& callback) { + BpBinder* proxy = this->remoteBinder(); + if (proxy != nullptr) { + return proxy->addFrozenStateChangeCallback(callback); + } + return INVALID_OPERATION; +} + +status_t IBinder::removeFrozenStateChangeCallback(const wp<FrozenStateChangeCallback>& callback) { + BpBinder* proxy = this->remoteBinder(); + if (proxy != nullptr) { + return proxy->removeFrozenStateChangeCallback(callback); + } + return INVALID_OPERATION; +} + status_t IBinder::getDebugPid(pid_t* out) { BBinder* local = this->localBinder(); if (local != nullptr) { diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp index 6594aa6309..eae844ca03 100644 --- a/libs/binder/BpBinder.cpp +++ b/libs/binder/BpBinder.cpp @@ -160,11 +160,12 @@ void BpBinder::ObjectManager::kill() // --------------------------------------------------------------------------- -sp<BpBinder> BpBinder::create(int32_t handle) { +sp<BpBinder> BpBinder::create(int32_t handle, std::function<void()>* postTask) { if constexpr (!kEnableKernelIpc) { LOG_ALWAYS_FATAL("Binder kernel driver disabled at build time"); return nullptr; } + LOG_ALWAYS_FATAL_IF(postTask == nullptr, "BAD STATE"); int32_t trackedUid = -1; if (sCountByUidEnabled) { @@ -183,7 +184,11 @@ sp<BpBinder> BpBinder::create(int32_t handle) { ALOGE("Still too many binder proxy objects sent to uid %d from uid %d (%d proxies " "held)", getuid(), trackedUid, trackedValue); - if (sLimitCallback) sLimitCallback(trackedUid); + + if (sLimitCallback) { + *postTask = [=]() { sLimitCallback(trackedUid); }; + } + sLastLimitCallbackMap[trackedUid] = trackedValue; } } else { @@ -197,7 +202,11 @@ sp<BpBinder> BpBinder::create(int32_t handle) { ALOGE("Too many binder proxy objects sent to uid %d from uid %d (%d proxies held)", getuid(), trackedUid, trackedValue); sTrackingMap[trackedUid] |= LIMIT_REACHED_MASK; - if (sLimitCallback) sLimitCallback(trackedUid); + + if (sLimitCallback) { + *postTask = [=]() { sLimitCallback(trackedUid); }; + } + sLastLimitCallbackMap[trackedUid] = trackedValue & COUNTING_VALUE_MASK; if (sBinderProxyThrottleCreate) { ALOGI("Throttling binder proxy creates from uid %d in uid %d until binder proxy" @@ -557,6 +566,123 @@ void BpBinder::sendObituary() } } +status_t BpBinder::addFrozenStateChangeCallback(const wp<FrozenStateChangeCallback>& callback) { + LOG_ALWAYS_FATAL_IF(isRpcBinder(), + "addFrozenStateChangeCallback() is not supported for RPC Binder."); + LOG_ALWAYS_FATAL_IF(!kEnableKernelIpc, "Binder kernel driver disabled at build time"); + LOG_ALWAYS_FATAL_IF(ProcessState::self()->getThreadPoolMaxTotalThreadCount() == 0, + "addFrozenStateChangeCallback on %s but there are no threads " + "(yet?) listening to incoming transactions. See " + "ProcessState::startThreadPool " + "and ProcessState::setThreadPoolMaxThreadCount. Generally you should " + "setup the binder threadpool before other initialization steps.", + String8(getInterfaceDescriptor()).c_str()); + LOG_ALWAYS_FATAL_IF(callback == nullptr, + "addFrozenStateChangeCallback(): callback must be non-NULL"); + + const sp<FrozenStateChangeCallback> strongCallback = callback.promote(); + if (strongCallback == nullptr) { + return BAD_VALUE; + } + + { + RpcMutexUniqueLock _l(mLock); + if (!mFrozen) { + ALOGV("Requesting freeze notification: %p handle %d\n", this, binderHandle()); + IPCThreadState* self = IPCThreadState::self(); + status_t status = self->addFrozenStateChangeCallback(binderHandle(), this); + if (status != NO_ERROR) { + // Avoids logspam if kernel does not support freeze + // notification. + if (status != INVALID_OPERATION) { + ALOGE("IPCThreadState.addFrozenStateChangeCallback " + "failed with %s. %p handle %d\n", + statusToString(status).c_str(), this, binderHandle()); + } + return status; + } + mFrozen = std::make_unique<FrozenStateChange>(); + if (!mFrozen) { + std::ignore = + IPCThreadState::self()->removeFrozenStateChangeCallback(binderHandle(), + this); + return NO_MEMORY; + } + } + if (mFrozen->initialStateReceived) { + strongCallback->onStateChanged(wp<BpBinder>::fromExisting(this), + mFrozen->isFrozen + ? FrozenStateChangeCallback::State::FROZEN + : FrozenStateChangeCallback::State::UNFROZEN); + } + ssize_t res = mFrozen->callbacks.add(callback); + if (res < 0) { + return res; + } + return NO_ERROR; + } +} + +status_t BpBinder::removeFrozenStateChangeCallback(const wp<FrozenStateChangeCallback>& callback) { + LOG_ALWAYS_FATAL_IF(isRpcBinder(), + "removeFrozenStateChangeCallback() is not supported for RPC Binder."); + LOG_ALWAYS_FATAL_IF(!kEnableKernelIpc, "Binder kernel driver disabled at build time"); + + RpcMutexUniqueLock _l(mLock); + + const size_t N = mFrozen ? mFrozen->callbacks.size() : 0; + for (size_t i = 0; i < N; i++) { + if (mFrozen->callbacks.itemAt(i) == callback) { + mFrozen->callbacks.removeAt(i); + if (mFrozen->callbacks.size() == 0) { + ALOGV("Clearing freeze notification: %p handle %d\n", this, binderHandle()); + status_t status = + IPCThreadState::self()->removeFrozenStateChangeCallback(binderHandle(), + this); + if (status != NO_ERROR) { + ALOGE("Unexpected error from " + "IPCThreadState.removeFrozenStateChangeCallback: %s. " + "%p handle %d\n", + statusToString(status).c_str(), this, binderHandle()); + } + mFrozen.reset(); + } + return NO_ERROR; + } + } + + return NAME_NOT_FOUND; +} + +void BpBinder::onFrozenStateChanged(bool isFrozen) { + LOG_ALWAYS_FATAL_IF(isRpcBinder(), "onFrozenStateChanged is not supported for RPC Binder."); + LOG_ALWAYS_FATAL_IF(!kEnableKernelIpc, "Binder kernel driver disabled at build time"); + + ALOGV("Sending frozen state change notification for proxy %p handle %d, isFrozen=%s\n", this, + binderHandle(), isFrozen ? "true" : "false"); + + RpcMutexUniqueLock _l(mLock); + if (!mFrozen) { + return; + } + bool stateChanged = !mFrozen->initialStateReceived || mFrozen->isFrozen != isFrozen; + if (stateChanged) { + mFrozen->isFrozen = isFrozen; + mFrozen->initialStateReceived = true; + for (size_t i = 0; i < mFrozen->callbacks.size();) { + sp<FrozenStateChangeCallback> callback = mFrozen->callbacks.itemAt(i).promote(); + if (callback != nullptr) { + callback->onStateChanged(wp<BpBinder>::fromExisting(this), + isFrozen ? FrozenStateChangeCallback::State::FROZEN + : FrozenStateChangeCallback::State::UNFROZEN); + i++; + } else { + mFrozen->callbacks.removeItemsAt(i); + } + } + } +} + void BpBinder::reportOneDeath(const Obituary& obit) { sp<DeathRecipient> recipient = obit.recipient.promote(); @@ -686,6 +812,10 @@ void BpBinder::onLastStrongRef(const void* /*id*/) { if (ipc) ipc->clearDeathNotification(binderHandle(), this); mObituaries = nullptr; } + if (mFrozen != nullptr) { + std::ignore = IPCThreadState::self()->removeFrozenStateChangeCallback(binderHandle(), this); + mFrozen.reset(); + } mLock.unlock(); if (obits != nullptr) { diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 984c93d370..1d26d8543d 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -89,26 +89,33 @@ static const char* kReturnStrings[] = { "BR_FROZEN_REPLY", "BR_ONEWAY_SPAM_SUSPECT", "BR_TRANSACTION_PENDING_FROZEN", + "BR_FROZEN_BINDER", + "BR_CLEAR_FREEZE_NOTIFICATION_DONE", }; -static const char *kCommandStrings[] = { - "BC_TRANSACTION", - "BC_REPLY", - "BC_ACQUIRE_RESULT", - "BC_FREE_BUFFER", - "BC_INCREFS", - "BC_ACQUIRE", - "BC_RELEASE", - "BC_DECREFS", - "BC_INCREFS_DONE", - "BC_ACQUIRE_DONE", - "BC_ATTEMPT_ACQUIRE", - "BC_REGISTER_LOOPER", - "BC_ENTER_LOOPER", - "BC_EXIT_LOOPER", - "BC_REQUEST_DEATH_NOTIFICATION", - "BC_CLEAR_DEATH_NOTIFICATION", - "BC_DEAD_BINDER_DONE" +static const char* kCommandStrings[] = { + "BC_TRANSACTION", + "BC_REPLY", + "BC_ACQUIRE_RESULT", + "BC_FREE_BUFFER", + "BC_INCREFS", + "BC_ACQUIRE", + "BC_RELEASE", + "BC_DECREFS", + "BC_INCREFS_DONE", + "BC_ACQUIRE_DONE", + "BC_ATTEMPT_ACQUIRE", + "BC_REGISTER_LOOPER", + "BC_ENTER_LOOPER", + "BC_EXIT_LOOPER", + "BC_REQUEST_DEATH_NOTIFICATION", + "BC_CLEAR_DEATH_NOTIFICATION", + "BC_DEAD_BINDER_DONE", + "BC_TRANSACTION_SG", + "BC_REPLY_SG", + "BC_REQUEST_FREEZE_NOTIFICATION", + "BC_CLEAR_FREEZE_NOTIFICATION", + "BC_FREEZE_NOTIFICATION_DONE", }; static const int64_t kWorkSourcePropagatedBitIndex = 32; @@ -203,6 +210,18 @@ static const void* printReturnCommand(std::ostream& out, const void* _cmd) { out << ": death cookie " << (void*)(uint64_t)c; } break; + case BR_FROZEN_BINDER: { + const int32_t c = *cmd++; + const int32_t h = *cmd++; + const int32_t isFrozen = *cmd++; + out << ": freeze cookie " << (void*)(uint64_t)c << " isFrozen: " << isFrozen; + } break; + + case BR_CLEAR_FREEZE_NOTIFICATION_DONE: { + const int32_t c = *cmd++; + out << ": freeze cookie " << (void*)(uint64_t)c; + } break; + default: // no details to show for: BR_OK, BR_DEAD_REPLY, // BR_TRANSACTION_COMPLETE, BR_FINISHED @@ -270,11 +289,23 @@ static const void* printCommand(std::ostream& out, const void* _cmd) { out << ": handle=" << h << " (death cookie " << (void*)(uint64_t)c << ")"; } break; + case BC_REQUEST_FREEZE_NOTIFICATION: + case BC_CLEAR_FREEZE_NOTIFICATION: { + const int32_t h = *cmd++; + const int32_t c = *cmd++; + out << ": handle=" << h << " (freeze cookie " << (void*)(uint64_t)c << ")"; + } break; + case BC_DEAD_BINDER_DONE: { const int32_t c = *cmd++; out << ": death cookie " << (void*)(uint64_t)c; } break; + case BC_FREEZE_NOTIFICATION_DONE: { + const int32_t c = *cmd++; + out << ": freeze cookie " << (void*)(uint64_t)c; + } break; + default: // no details to show for: BC_REGISTER_LOOPER, BC_ENTER_LOOPER, // BC_EXIT_LOOPER @@ -953,6 +984,33 @@ status_t IPCThreadState::clearDeathNotification(int32_t handle, BpBinder* proxy) return NO_ERROR; } +status_t IPCThreadState::addFrozenStateChangeCallback(int32_t handle, BpBinder* proxy) { + static bool isSupported = + ProcessState::isDriverFeatureEnabled(ProcessState::DriverFeature::FREEZE_NOTIFICATION); + if (!isSupported) { + return INVALID_OPERATION; + } + proxy->getWeakRefs()->incWeak(proxy); + mOut.writeInt32(BC_REQUEST_FREEZE_NOTIFICATION); + mOut.writeInt32((int32_t)handle); + mOut.writePointer((uintptr_t)proxy); + flushCommands(); + return NO_ERROR; +} + +status_t IPCThreadState::removeFrozenStateChangeCallback(int32_t handle, BpBinder* proxy) { + static bool isSupported = + ProcessState::isDriverFeatureEnabled(ProcessState::DriverFeature::FREEZE_NOTIFICATION); + if (!isSupported) { + return INVALID_OPERATION; + } + mOut.writeInt32(BC_CLEAR_FREEZE_NOTIFICATION); + mOut.writeInt32((int32_t)handle); + mOut.writePointer((uintptr_t)proxy); + flushCommands(); + return NO_ERROR; +} + IPCThreadState::IPCThreadState() : mProcess(ProcessState::self()), mServingStackPointer(nullptr), @@ -1487,6 +1545,26 @@ status_t IPCThreadState::executeCommand(int32_t cmd) proxy->getWeakRefs()->decWeak(proxy); } break; + case BR_FROZEN_BINDER: { + const struct binder_frozen_state_info* data = + reinterpret_cast<const struct binder_frozen_state_info*>( + mIn.readInplace(sizeof(struct binder_frozen_state_info))); + if (data == nullptr) { + result = UNKNOWN_ERROR; + break; + } + BpBinder* proxy = (BpBinder*)data->cookie; + bool isFrozen = mIn.readInt32() > 0; + proxy->getPrivateAccessor().onFrozenStateChanged(data->is_frozen); + mOut.writeInt32(BC_FREEZE_NOTIFICATION_DONE); + mOut.writePointer(data->cookie); + } break; + + case BR_CLEAR_FREEZE_NOTIFICATION_DONE: { + BpBinder* proxy = (BpBinder*)mIn.readPointer(); + proxy->getWeakRefs()->decWeak(proxy); + } break; + case BR_FINISHED: result = TIMED_OUT; break; diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index 083b68d65d..c55dd9de1b 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -252,8 +252,11 @@ bool checkPermission(const String16& permission, pid_t pid, uid_t uid, bool logP } } +#endif //__ANDROID_VNDK__ + void* openDeclaredPassthroughHal(const String16& interface, const String16& instance, int flag) { -#if defined(__ANDROID__) && !defined(__ANDROID_RECOVERY__) && !defined(__ANDROID_NATIVE_BRIDGE__) +#if defined(__ANDROID__) && !defined(__ANDROID_VENDOR__) && !defined(__ANDROID_RECOVERY__) && \ + !defined(__ANDROID_NATIVE_BRIDGE__) sp<IServiceManager> sm = defaultServiceManager(); String16 name = interface + String16("/") + instance; if (!sm->isDeclared(name)) { @@ -273,8 +276,6 @@ void* openDeclaredPassthroughHal(const String16& interface, const String16& inst #endif } -#endif //__ANDROID_VNDK__ - // ---------------------------------------------------------------------- CppBackendShim::CppBackendShim(const sp<BackendUnifiedServiceManager>& impl) diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp index a42ede29a2..5e7f1510bc 100644 --- a/libs/binder/ProcessState.cpp +++ b/libs/binder/ProcessState.cpp @@ -24,7 +24,6 @@ #include <binder/IPCThreadState.h> #include <binder/IServiceManager.h> #include <binder/Stability.h> -#include <cutils/atomic.h> #include <utils/AndroidThreads.h> #include <utils/String8.h> #include <utils/Thread.h> @@ -57,6 +56,25 @@ const char* kDefaultDriver = "/dev/binder"; // ------------------------------------------------------------------------- +namespace { +bool readDriverFeatureFile(const char* filename) { + int fd = open(filename, O_RDONLY | O_CLOEXEC); + char on; + if (fd == -1) { + ALOGE_IF(errno != ENOENT, "%s: cannot open %s: %s", __func__, filename, strerror(errno)); + return false; + } + if (read(fd, &on, sizeof(on)) == -1) { + ALOGE("%s: error reading to %s: %s", __func__, filename, strerror(errno)); + close(fd); + return false; + } + close(fd); + return on == '1'; +} + +} // namespace + namespace android { using namespace android::binder::impl; @@ -311,6 +329,7 @@ extern sp<BBinder> the_context_object; sp<IBinder> ProcessState::getStrongProxyForHandle(int32_t handle) { sp<IBinder> result; + std::function<void()> postTask; std::unique_lock<std::mutex> _l(mLock); @@ -358,7 +377,7 @@ sp<IBinder> ProcessState::getStrongProxyForHandle(int32_t handle) return nullptr; } - sp<BpBinder> b = BpBinder::PrivateAccessor::create(handle); + sp<BpBinder> b = BpBinder::PrivateAccessor::create(handle, &postTask); e->binder = b.get(); if (b) e->refs = b->getWeakRefs(); result = b; @@ -371,6 +390,10 @@ sp<IBinder> ProcessState::getStrongProxyForHandle(int32_t handle) } } + _l.unlock(); + + if (postTask) postTask(); + return result; } @@ -387,7 +410,7 @@ void ProcessState::expungeHandle(int32_t handle, IBinder* binder) } String8 ProcessState::makeBinderThreadName() { - int32_t s = android_atomic_add(1, &mThreadPoolSeq); + int32_t s = mThreadPoolSeq.fetch_add(1, std::memory_order_release); pid_t pid = getpid(); std::string_view driverName = mDriverName.c_str(); @@ -429,8 +452,17 @@ status_t ProcessState::setThreadPoolMaxThreadCount(size_t maxThreads) { } size_t ProcessState::getThreadPoolMaxTotalThreadCount() const { + // Need to read `mKernelStartedThreads` before `mThreadPoolStarted` (with + // non-relaxed memory ordering) to avoid a race like the following: + // + // thread A: if (mThreadPoolStarted) { // evaluates false + // thread B: mThreadPoolStarted = true; + // thread B: mKernelStartedThreads++; + // thread A: size_t kernelStarted = mKernelStartedThreads; + // thread A: LOG_ALWAYS_FATAL_IF(kernelStarted != 0, ...); + size_t kernelStarted = mKernelStartedThreads; + if (mThreadPoolStarted) { - size_t kernelStarted = mKernelStartedThreads; size_t max = mMaxThreads; size_t current = mCurrentThreads; @@ -460,7 +492,6 @@ size_t ProcessState::getThreadPoolMaxTotalThreadCount() const { // must not be initialized or maybe has poll thread setup, we // currently don't track this in libbinder - size_t kernelStarted = mKernelStartedThreads; LOG_ALWAYS_FATAL_IF(kernelStarted != 0, "Expecting 0 kernel started threads but have %zu", kernelStarted); return mCurrentThreads; @@ -472,27 +503,20 @@ bool ProcessState::isThreadPoolStarted() const { #define DRIVER_FEATURES_PATH "/dev/binderfs/features/" bool ProcessState::isDriverFeatureEnabled(const DriverFeature feature) { - static const char* const names[] = { - [static_cast<int>(DriverFeature::ONEWAY_SPAM_DETECTION)] = - DRIVER_FEATURES_PATH "oneway_spam_detection", - [static_cast<int>(DriverFeature::EXTENDED_ERROR)] = - DRIVER_FEATURES_PATH "extended_error", - }; - int fd = open(names[static_cast<int>(feature)], O_RDONLY | O_CLOEXEC); - char on; - if (fd == -1) { - ALOGE_IF(errno != ENOENT, "%s: cannot open %s: %s", __func__, - names[static_cast<int>(feature)], strerror(errno)); - return false; + // Use static variable to cache the results. + if (feature == DriverFeature::ONEWAY_SPAM_DETECTION) { + static bool enabled = readDriverFeatureFile(DRIVER_FEATURES_PATH "oneway_spam_detection"); + return enabled; } - if (read(fd, &on, sizeof(on)) == -1) { - ALOGE("%s: error reading to %s: %s", __func__, - names[static_cast<int>(feature)], strerror(errno)); - close(fd); - return false; + if (feature == DriverFeature::EXTENDED_ERROR) { + static bool enabled = readDriverFeatureFile(DRIVER_FEATURES_PATH "extended_error"); + return enabled; } - close(fd); - return on == '1'; + if (feature == DriverFeature::FREEZE_NOTIFICATION) { + static bool enabled = readDriverFeatureFile(DRIVER_FEATURES_PATH "freeze_notification"); + return enabled; + } + return false; } status_t ProcessState::enableOnewaySpamDetection(bool enable) { @@ -577,7 +601,7 @@ ProcessState::ProcessState(const char* driver) #ifdef __ANDROID__ LOG_ALWAYS_FATAL_IF(!opened.ok(), "Binder driver '%s' could not be opened. Error: %s. Terminating.", - error.c_str(), driver); + driver, error.c_str()); #endif if (opened.ok()) { diff --git a/libs/binder/binder_module.h b/libs/binder/binder_module.h index b3a2d9ec28..65cdcd7735 100644 --- a/libs/binder/binder_module.h +++ b/libs/binder/binder_module.h @@ -32,4 +32,34 @@ #include <linux/android/binder.h> #include <sys/ioctl.h> +struct binder_frozen_state_info { + binder_uintptr_t cookie; + __u32 is_frozen; +}; + +#ifndef BR_FROZEN_BINDER +// Temporary definition of BR_FROZEN_BINDER until UAPI binder.h includes it. +#define BR_FROZEN_BINDER _IOR('r', 21, struct binder_frozen_state_info) +#endif // BR_FROZEN_BINDER + +#ifndef BR_CLEAR_FREEZE_NOTIFICATION_DONE +// Temporary definition of BR_CLEAR_FREEZE_NOTIFICATION_DONE until UAPI binder.h includes it. +#define BR_CLEAR_FREEZE_NOTIFICATION_DONE _IOR('r', 22, binder_uintptr_t) +#endif // BR_CLEAR_FREEZE_NOTIFICATION_DONE + +#ifndef BC_REQUEST_FREEZE_NOTIFICATION +// Temporary definition of BC_REQUEST_FREEZE_NOTIFICATION until UAPI binder.h includes it. +#define BC_REQUEST_FREEZE_NOTIFICATION _IOW('c', 19, struct binder_handle_cookie) +#endif // BC_REQUEST_FREEZE_NOTIFICATION + +#ifndef BC_CLEAR_FREEZE_NOTIFICATION +// Temporary definition of BC_CLEAR_FREEZE_NOTIFICATION until UAPI binder.h includes it. +#define BC_CLEAR_FREEZE_NOTIFICATION _IOW('c', 20, struct binder_handle_cookie) +#endif // BC_CLEAR_FREEZE_NOTIFICATION + +#ifndef BC_FREEZE_NOTIFICATION_DONE +// Temporary definition of BC_FREEZE_NOTIFICATION_DONE until UAPI binder.h includes it. +#define BC_FREEZE_NOTIFICATION_DONE _IOW('c', 21, binder_uintptr_t) +#endif // BC_FREEZE_NOTIFICATION_DONE + #endif // _BINDER_MODULE_H_ diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h index d7f74c4152..7518044ce6 100644 --- a/libs/binder/include/binder/BpBinder.h +++ b/libs/binder/include/binder/BpBinder.h @@ -29,6 +29,7 @@ // --------------------------------------------------------------------------- namespace android { +class IPCThreadState; class RpcSession; class RpcState; namespace internal { @@ -66,6 +67,12 @@ public: void* cookie = nullptr, uint32_t flags = 0, wp<DeathRecipient>* outRecipient = nullptr); + [[nodiscard]] status_t addFrozenStateChangeCallback( + const wp<FrozenStateChangeCallback>& recipient); + + [[nodiscard]] status_t removeFrozenStateChangeCallback( + const wp<FrozenStateChangeCallback>& recipient); + LIBBINDER_EXPORTED virtual void* attachObject(const void* objectID, void* object, void* cleanupCookie, object_cleanup_func func) final; @@ -75,7 +82,6 @@ public: LIBBINDER_EXPORTED sp<IBinder> lookupOrCreateWeak(const void* objectID, IBinder::object_make_func make, const void* makeArgs); - LIBBINDER_EXPORTED virtual BpBinder* remoteBinder(); LIBBINDER_EXPORTED void sendObituary(); @@ -132,9 +138,14 @@ public: friend class ::android::ProcessState; friend class ::android::RpcSession; friend class ::android::RpcState; - explicit PrivateAccessor(const BpBinder* binder) : mBinder(binder) {} + friend class ::android::IPCThreadState; + explicit PrivateAccessor(const BpBinder* binder) + : mBinder(binder), mMutableBinder(nullptr) {} + explicit PrivateAccessor(BpBinder* binder) : mBinder(binder), mMutableBinder(binder) {} - static sp<BpBinder> create(int32_t handle) { return BpBinder::create(handle); } + static sp<BpBinder> create(int32_t handle, std::function<void()>* postTask) { + return BpBinder::create(handle, postTask); + } static sp<BpBinder> create(const sp<RpcSession>& session, uint64_t address) { return BpBinder::create(session, address); } @@ -146,17 +157,22 @@ public: uint64_t rpcAddress() const { return mBinder->rpcAddress(); } const sp<RpcSession>& rpcSession() const { return mBinder->rpcSession(); } + void onFrozenStateChanged(bool isFrozen) { mMutableBinder->onFrozenStateChanged(isFrozen); } const BpBinder* mBinder; + BpBinder* mMutableBinder; }; + LIBBINDER_EXPORTED const PrivateAccessor getPrivateAccessor() const { return PrivateAccessor(this); } + PrivateAccessor getPrivateAccessor() { return PrivateAccessor(this); } + private: friend PrivateAccessor; friend class sp<BpBinder>; - static sp<BpBinder> create(int32_t handle); + static sp<BpBinder> create(int32_t handle, std::function<void()>* postTask); static sp<BpBinder> create(const sp<RpcSession>& session, uint64_t address); struct BinderHandle { @@ -192,6 +208,14 @@ private: uint32_t flags; }; + void onFrozenStateChanged(bool isFrozen); + + struct FrozenStateChange { + bool isFrozen = false; + Vector<wp<FrozenStateChangeCallback>> callbacks; + bool initialStateReceived = false; + }; + void reportOneDeath(const Obituary& obit); bool isDescriptorCached() const; @@ -199,6 +223,7 @@ private: volatile int32_t mAlive; volatile int32_t mObitsSent; Vector<Obituary>* mObituaries; + std::unique_ptr<FrozenStateChange> mFrozen; ObjectManager mObjects; mutable String16 mDescriptorCache; int32_t mTrackedUid; diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h index 4eb1c082e2..1ed7c91de6 100644 --- a/libs/binder/include/binder/IBinder.h +++ b/libs/binder/include/binder/IBinder.h @@ -202,9 +202,18 @@ public: virtual void binderDied(const wp<IBinder>& who) = 0; }; - #if defined(__clang__) - #pragma clang diagnostic pop - #endif + class FrozenStateChangeCallback : public virtual RefBase { + public: + enum class State { + FROZEN, + UNFROZEN, + }; + virtual void onStateChanged(const wp<IBinder>& who, State state) = 0; + }; + +#if defined(__clang__) +#pragma clang diagnostic pop +#endif /** * Register the @a recipient for a notification if this binder @@ -253,6 +262,48 @@ public: uint32_t flags = 0, wp<DeathRecipient>* outRecipient = nullptr) = 0; + /** + * addFrozenStateChangeCallback provides a callback mechanism to notify + * about process frozen/unfrozen events. Upon registration and any + * subsequent state changes, the callback is invoked with the latest process + * frozen state. + * + * If the listener process (the one using this API) is itself frozen, state + * change events might be combined into a single one with the latest state. + * (meaning 'frozen, unfrozen' might just be 'unfrozen'). This single event + * would then be delivered when the listener process becomes unfrozen. + * Similarly, if an event happens before the previous event is consumed, + * they might be combined. This means the callback might not be called for + * every single state change, so don't rely on this API to count how many + * times the state has changed. + * + * @note When all references to the binder are dropped, the callback is + * automatically removed. So, you must hold onto a binder in order to + * receive notifications about it. + * + * @note You will only receive freeze notifications for remote binders, as + * local binders by definition can't be frozen without you being frozen as + * well. Trying to use this function on a local binder will result in an + * INVALID_OPERATION code being returned and nothing happening. + * + * @note This binder always holds a weak reference to the callback. + * + * @note You will only receive a weak reference to the binder object. You + * should not try to promote this to a strong reference. (Nor should you + * need to, as there is nothing useful you can directly do with it now that + * it has passed on.) + */ + [[nodiscard]] status_t addFrozenStateChangeCallback( + const wp<FrozenStateChangeCallback>& callback); + + /** + * Remove a previously registered freeze callback. + * The @a callback will no longer be called if this object + * changes its frozen state. + */ + [[nodiscard]] status_t removeFrozenStateChangeCallback( + const wp<FrozenStateChangeCallback>& callback); + virtual bool checkSubclass(const void* subclassID) const; typedef void (*object_cleanup_func)(const void* id, void* obj, void* cleanupCookie); diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h index 09ab442c7d..9ef4e694dd 100644 --- a/libs/binder/include/binder/IPCThreadState.h +++ b/libs/binder/include/binder/IPCThreadState.h @@ -174,6 +174,8 @@ public: LIBBINDER_EXPORTED static void expungeHandle(int32_t handle, IBinder* binder); LIBBINDER_EXPORTED status_t requestDeathNotification(int32_t handle, BpBinder* proxy); LIBBINDER_EXPORTED status_t clearDeathNotification(int32_t handle, BpBinder* proxy); + [[nodiscard]] status_t addFrozenStateChangeCallback(int32_t handle, BpBinder* proxy); + [[nodiscard]] status_t removeFrozenStateChangeCallback(int32_t handle, BpBinder* proxy); LIBBINDER_EXPORTED static void shutdown(); @@ -210,13 +212,14 @@ private: IPCThreadState(); ~IPCThreadState(); - status_t sendReply(const Parcel& reply, uint32_t flags); - status_t waitForResponse(Parcel* reply, status_t* acquireResult = nullptr); - status_t talkWithDriver(bool doReceive = true); - status_t writeTransactionData(int32_t cmd, uint32_t binderFlags, int32_t handle, uint32_t code, - const Parcel& data, status_t* statusBuffer); - status_t getAndExecuteCommand(); - status_t executeCommand(int32_t command); + [[nodiscard]] status_t sendReply(const Parcel& reply, uint32_t flags); + [[nodiscard]] status_t waitForResponse(Parcel* reply, status_t* acquireResult = nullptr); + [[nodiscard]] status_t talkWithDriver(bool doReceive = true); + [[nodiscard]] status_t writeTransactionData(int32_t cmd, uint32_t binderFlags, int32_t handle, + uint32_t code, const Parcel& data, + status_t* statusBuffer); + [[nodiscard]] status_t getAndExecuteCommand(); + [[nodiscard]] status_t executeCommand(int32_t command); void processPendingDerefs(); void processPostWriteDerefs(); diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h index 021bd58e21..21bfd42fcc 100644 --- a/libs/binder/include/binder/ProcessState.h +++ b/libs/binder/include/binder/ProcessState.h @@ -133,6 +133,7 @@ public: enum class DriverFeature { ONEWAY_SPAM_DETECTION, EXTENDED_ERROR, + FREEZE_NOTIFICATION, }; // Determine whether a feature is supported by the binder driver. LIBBINDER_EXPORTED static bool isDriverFeatureEnabled(const DriverFeature feature); @@ -188,8 +189,8 @@ private: Vector<handle_entry> mHandleToObject; bool mForked; - bool mThreadPoolStarted; - volatile int32_t mThreadPoolSeq; + std::atomic_bool mThreadPoolStarted; + std::atomic_int32_t mThreadPoolSeq; CallRestriction mCallRestriction; }; diff --git a/libs/binder/ndk/Android.bp b/libs/binder/ndk/Android.bp index 26c228d9a4..4e02aceb86 100644 --- a/libs/binder/ndk/Android.bp +++ b/libs/binder/ndk/Android.bp @@ -255,6 +255,9 @@ ndk_headers { "include_cpp/android/*.h", ], license: "NOTICE", + // These are intentionally not C. It's a mistake that they're in the NDK. + // See the bug above. + skip_verification: true, } ndk_library { diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs index 33dfe19fe9..7f70396882 100644 --- a/libs/binder/rust/src/parcel/parcelable.rs +++ b/libs/binder/rust/src/parcel/parcelable.rs @@ -1333,7 +1333,7 @@ mod tests { let vec = Vec::<u8>::deserialize(parcel.borrowed_ref()).unwrap(); assert_eq!(vec, [-128i8 as u8, 127, 42, -117i8 as u8]); - let u16s = [u16::max_value(), 12_345, 42, 117]; + let u16s = [u16::MAX, 12_345, 42, 117]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. @@ -1348,7 +1348,7 @@ mod tests { } assert_eq!(parcel.read::<u32>().unwrap(), 4); // 4 items - assert_eq!(parcel.read::<u32>().unwrap(), 0xffff); // u16::max_value() + assert_eq!(parcel.read::<u32>().unwrap(), 0xffff); // u16::MAX assert_eq!(parcel.read::<u32>().unwrap(), 12345); // 12,345 assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42 assert_eq!(parcel.read::<u32>().unwrap(), 117); // 117 @@ -1361,9 +1361,9 @@ mod tests { let vec = Vec::<u16>::deserialize(parcel.borrowed_ref()).unwrap(); - assert_eq!(vec, [u16::max_value(), 12_345, 42, 117]); + assert_eq!(vec, [u16::MAX, 12_345, 42, 117]); - let i16s = [i16::max_value(), i16::min_value(), 42, -117]; + let i16s = [i16::MAX, i16::MIN, 42, -117]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. @@ -1378,8 +1378,8 @@ mod tests { } assert_eq!(parcel.read::<u32>().unwrap(), 4); // 4 items - assert_eq!(parcel.read::<u32>().unwrap(), 0x7fff); // i16::max_value() - assert_eq!(parcel.read::<u32>().unwrap(), 0x8000); // i16::min_value() + assert_eq!(parcel.read::<u32>().unwrap(), 0x7fff); // i16::MAX + assert_eq!(parcel.read::<u32>().unwrap(), 0x8000); // i16::MIN assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42 assert_eq!(parcel.read::<u32>().unwrap(), 0xff8b); // -117 @@ -1391,9 +1391,9 @@ mod tests { let vec = Vec::<i16>::deserialize(parcel.borrowed_ref()).unwrap(); - assert_eq!(vec, [i16::max_value(), i16::min_value(), 42, -117]); + assert_eq!(vec, [i16::MAX, i16::MIN, 42, -117]); - let u32s = [u32::max_value(), 12_345, 42, 117]; + let u32s = [u32::MAX, 12_345, 42, 117]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. @@ -1408,7 +1408,7 @@ mod tests { } assert_eq!(parcel.read::<u32>().unwrap(), 4); // 4 items - assert_eq!(parcel.read::<u32>().unwrap(), 0xffffffff); // u32::max_value() + assert_eq!(parcel.read::<u32>().unwrap(), 0xffffffff); // u32::MAX assert_eq!(parcel.read::<u32>().unwrap(), 12345); // 12,345 assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42 assert_eq!(parcel.read::<u32>().unwrap(), 117); // 117 @@ -1421,9 +1421,9 @@ mod tests { let vec = Vec::<u32>::deserialize(parcel.borrowed_ref()).unwrap(); - assert_eq!(vec, [u32::max_value(), 12_345, 42, 117]); + assert_eq!(vec, [u32::MAX, 12_345, 42, 117]); - let i32s = [i32::max_value(), i32::min_value(), 42, -117]; + let i32s = [i32::MAX, i32::MIN, 42, -117]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. @@ -1438,8 +1438,8 @@ mod tests { } assert_eq!(parcel.read::<u32>().unwrap(), 4); // 4 items - assert_eq!(parcel.read::<u32>().unwrap(), 0x7fffffff); // i32::max_value() - assert_eq!(parcel.read::<u32>().unwrap(), 0x80000000); // i32::min_value() + assert_eq!(parcel.read::<u32>().unwrap(), 0x7fffffff); // i32::MAX + assert_eq!(parcel.read::<u32>().unwrap(), 0x80000000); // i32::MIN assert_eq!(parcel.read::<u32>().unwrap(), 42); // 42 assert_eq!(parcel.read::<u32>().unwrap(), 0xffffff8b); // -117 @@ -1451,9 +1451,9 @@ mod tests { let vec = Vec::<i32>::deserialize(parcel.borrowed_ref()).unwrap(); - assert_eq!(vec, [i32::max_value(), i32::min_value(), 42, -117]); + assert_eq!(vec, [i32::MAX, i32::MIN, 42, -117]); - let u64s = [u64::max_value(), 12_345, 42, 117]; + let u64s = [u64::MAX, 12_345, 42, 117]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. @@ -1469,9 +1469,9 @@ mod tests { let vec = Vec::<u64>::deserialize(parcel.borrowed_ref()).unwrap(); - assert_eq!(vec, [u64::max_value(), 12_345, 42, 117]); + assert_eq!(vec, [u64::MAX, 12_345, 42, 117]); - let i64s = [i64::max_value(), i64::min_value(), 42, -117]; + let i64s = [i64::MAX, i64::MIN, 42, -117]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. @@ -1487,9 +1487,9 @@ mod tests { let vec = Vec::<i64>::deserialize(parcel.borrowed_ref()).unwrap(); - assert_eq!(vec, [i64::max_value(), i64::min_value(), 42, -117]); + assert_eq!(vec, [i64::MAX, i64::MIN, 42, -117]); - let f32s = [std::f32::NAN, std::f32::INFINITY, 1.23456789, std::f32::EPSILON]; + let f32s = [f32::NAN, f32::INFINITY, 1.23456789, f32::EPSILON]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. @@ -1509,7 +1509,7 @@ mod tests { assert!(vec[0].is_nan()); assert_eq!(vec[1..], f32s[1..]); - let f64s = [std::f64::NAN, std::f64::INFINITY, 1.234567890123456789, std::f64::EPSILON]; + let f64s = [f64::NAN, f64::INFINITY, 1.234567890123456789, f64::EPSILON]; // SAFETY: start is less than the current size of the parcel data buffer, because we haven't // made it any shorter since we got the position. diff --git a/libs/binder/rust/tests/serialization.rs b/libs/binder/rust/tests/serialization.rs index 2b6c282530..a902e96739 100644 --- a/libs/binder/rust/tests/serialization.rs +++ b/libs/binder/rust/tests/serialization.rs @@ -124,7 +124,7 @@ fn on_transact( bindings::Transaction_TEST_BYTE => { assert_eq!(parcel.read::<i8>()?, 0); assert_eq!(parcel.read::<i8>()?, 1); - assert_eq!(parcel.read::<i8>()?, i8::max_value()); + assert_eq!(parcel.read::<i8>()?, i8::MAX); // SAFETY: Just reading an extern constant. assert_eq!(parcel.read::<Vec<i8>>()?, unsafe { bindings::TESTDATA_I8 }); // SAFETY: Just reading an extern constant. @@ -133,7 +133,7 @@ fn on_transact( reply.write(&0i8)?; reply.write(&1i8)?; - reply.write(&i8::max_value())?; + reply.write(&i8::MAX)?; // SAFETY: Just reading an extern constant. reply.write(&unsafe { bindings::TESTDATA_I8 }[..])?; // SAFETY: Just reading an extern constant. @@ -143,14 +143,14 @@ fn on_transact( bindings::Transaction_TEST_U16 => { assert_eq!(parcel.read::<u16>()?, 0); assert_eq!(parcel.read::<u16>()?, 1); - assert_eq!(parcel.read::<u16>()?, u16::max_value()); + assert_eq!(parcel.read::<u16>()?, u16::MAX); // SAFETY: Just reading an extern constant. assert_eq!(parcel.read::<Vec<u16>>()?, unsafe { bindings::TESTDATA_CHARS }); assert_eq!(parcel.read::<Option<Vec<u16>>>()?, None); reply.write(&0u16)?; reply.write(&1u16)?; - reply.write(&u16::max_value())?; + reply.write(&u16::MAX)?; // SAFETY: Just reading an extern constant. reply.write(&unsafe { bindings::TESTDATA_CHARS }[..])?; reply.write(&(None as Option<Vec<u16>>))?; @@ -158,14 +158,14 @@ fn on_transact( bindings::Transaction_TEST_I32 => { assert_eq!(parcel.read::<i32>()?, 0); assert_eq!(parcel.read::<i32>()?, 1); - assert_eq!(parcel.read::<i32>()?, i32::max_value()); + assert_eq!(parcel.read::<i32>()?, i32::MAX); // SAFETY: Just reading an extern constant. assert_eq!(parcel.read::<Vec<i32>>()?, unsafe { bindings::TESTDATA_I32 }); assert_eq!(parcel.read::<Option<Vec<i32>>>()?, None); reply.write(&0i32)?; reply.write(&1i32)?; - reply.write(&i32::max_value())?; + reply.write(&i32::MAX)?; // SAFETY: Just reading an extern constant. reply.write(&unsafe { bindings::TESTDATA_I32 }[..])?; reply.write(&(None as Option<Vec<i32>>))?; @@ -173,14 +173,14 @@ fn on_transact( bindings::Transaction_TEST_I64 => { assert_eq!(parcel.read::<i64>()?, 0); assert_eq!(parcel.read::<i64>()?, 1); - assert_eq!(parcel.read::<i64>()?, i64::max_value()); + assert_eq!(parcel.read::<i64>()?, i64::MAX); // SAFETY: Just reading an extern constant. assert_eq!(parcel.read::<Vec<i64>>()?, unsafe { bindings::TESTDATA_I64 }); assert_eq!(parcel.read::<Option<Vec<i64>>>()?, None); reply.write(&0i64)?; reply.write(&1i64)?; - reply.write(&i64::max_value())?; + reply.write(&i64::MAX)?; // SAFETY: Just reading an extern constant. reply.write(&unsafe { bindings::TESTDATA_I64 }[..])?; reply.write(&(None as Option<Vec<i64>>))?; @@ -188,14 +188,14 @@ fn on_transact( bindings::Transaction_TEST_U64 => { assert_eq!(parcel.read::<u64>()?, 0); assert_eq!(parcel.read::<u64>()?, 1); - assert_eq!(parcel.read::<u64>()?, u64::max_value()); + assert_eq!(parcel.read::<u64>()?, u64::MAX); // SAFETY: Just reading an extern constant. assert_eq!(parcel.read::<Vec<u64>>()?, unsafe { bindings::TESTDATA_U64 }); assert_eq!(parcel.read::<Option<Vec<u64>>>()?, None); reply.write(&0u64)?; reply.write(&1u64)?; - reply.write(&u64::max_value())?; + reply.write(&u64::MAX)?; // SAFETY: Just reading an extern constant. reply.write(&unsafe { bindings::TESTDATA_U64 }[..])?; reply.write(&(None as Option<Vec<u64>>))?; diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index dd50fbdc38..21c32acb0a 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -42,6 +42,9 @@ cc_test { defaults: ["binder_test_defaults"], header_libs: ["libbinder_headers"], srcs: ["binderDriverInterfaceTest.cpp"], + shared_libs: [ + "libbinder", + ], test_suites: [ "device-tests", "vts", @@ -127,6 +130,7 @@ cc_test { "libbase", "libbinder", "liblog", + "libprocessgroup", "libutils", ], static_libs: [ diff --git a/libs/binder/tests/binderDriverInterfaceTest.cpp b/libs/binder/tests/binderDriverInterfaceTest.cpp index 7be4f21cee..af8286008f 100644 --- a/libs/binder/tests/binderDriverInterfaceTest.cpp +++ b/libs/binder/tests/binderDriverInterfaceTest.cpp @@ -20,12 +20,15 @@ #include <stdlib.h> #include <binder/IBinder.h> +#include <binder/ProcessState.h> #include <gtest/gtest.h> #include <linux/android/binder.h> #include <poll.h> #include <sys/ioctl.h> #include <sys/mman.h> +#include "../binder_module.h" + #define BINDER_DEV_NAME "/dev/binder" testing::Environment* binder_env; @@ -365,6 +368,251 @@ TEST_F(BinderDriverInterfaceTest, RequestDeathNotification) { binderTestReadEmpty(); } +TEST_F(BinderDriverInterfaceTest, RequestFrozenNotification) { + if (!android::ProcessState::isDriverFeatureEnabled( + android::ProcessState::DriverFeature::FREEZE_NOTIFICATION)) { + GTEST_SKIP() << "Skipping test for kernels that support freeze notification"; + return; + } + binder_uintptr_t cookie = 1234; + struct { + uint32_t cmd0; + uint32_t arg0; + uint32_t cmd1; + struct binder_handle_cookie arg1; + } __attribute__((packed)) bc1 = { + .cmd0 = BC_INCREFS, + .arg0 = 0, + .cmd1 = BC_REQUEST_FREEZE_NOTIFICATION, + .arg1 = + { + .handle = 0, + .cookie = cookie, + }, + }; + struct { + uint32_t cmd0; + // Expecting a BR_FROZEN_BINDER since BC_REQUEST_FREEZE_NOTIFICATION + // above should lead to an immediate notification of the current state. + uint32_t cmd1; + struct binder_frozen_state_info arg1; + uint32_t pad[16]; + } __attribute__((packed)) br1; + struct { + uint32_t cmd2; + binder_uintptr_t arg2; + uint32_t cmd3; + struct binder_handle_cookie arg3; + uint32_t cmd4; + uint32_t arg4; + } __attribute__((packed)) bc2 = { + // Tell kernel that userspace has done handling BR_FROZEN_BINDER. + .cmd2 = BC_FREEZE_NOTIFICATION_DONE, + .arg2 = cookie, + .cmd3 = BC_CLEAR_FREEZE_NOTIFICATION, + .arg3 = + { + .handle = 0, + .cookie = cookie, + }, + .cmd4 = BC_DECREFS, + .arg4 = 0, + }; + struct { + uint32_t cmd2; + uint32_t cmd3; + binder_uintptr_t arg3; + uint32_t pad[16]; + } __attribute__((packed)) br2; + + struct binder_write_read bwr1 = binder_write_read(); + bwr1.write_buffer = (uintptr_t)&bc1; + bwr1.write_size = sizeof(bc1); + bwr1.read_buffer = (uintptr_t)&br1; + bwr1.read_size = sizeof(br1); + binderTestIoctl(BINDER_WRITE_READ, &bwr1); + EXPECT_EQ(sizeof(bc1), bwr1.write_consumed); + EXPECT_EQ(sizeof(br1) - sizeof(br1.pad), bwr1.read_consumed); + EXPECT_EQ(BR_NOOP, br1.cmd0); + ASSERT_EQ(BR_FROZEN_BINDER, br1.cmd1); + EXPECT_FALSE(br1.arg1.is_frozen); + + struct binder_write_read bwr2 = binder_write_read(); + bwr2.write_buffer = (uintptr_t)&bc2; + bwr2.write_size = sizeof(bc2); + bwr2.read_buffer = (uintptr_t)&br2; + bwr2.read_size = sizeof(br2); + binderTestIoctl(BINDER_WRITE_READ, &bwr2); + EXPECT_EQ(sizeof(bc2), bwr2.write_consumed); + EXPECT_EQ(sizeof(br2) - sizeof(br2.pad), bwr2.read_consumed); + EXPECT_EQ(BR_NOOP, br2.cmd2); + EXPECT_EQ(BR_CLEAR_FREEZE_NOTIFICATION_DONE, br2.cmd3); + EXPECT_EQ(cookie, br2.arg3); + + binderTestReadEmpty(); +} + +TEST_F(BinderDriverInterfaceTest, OverwritePendingFrozenNotification) { + if (!android::ProcessState::isDriverFeatureEnabled( + android::ProcessState::DriverFeature::FREEZE_NOTIFICATION)) { + GTEST_SKIP() << "Skipping test for kernels that support freeze notification"; + return; + } + binder_uintptr_t cookie = 1234; + struct { + uint32_t cmd0; + uint32_t arg0; + uint32_t cmd1; + struct binder_handle_cookie arg1; + uint32_t cmd2; + struct binder_handle_cookie arg2; + uint32_t cmd3; + uint32_t arg3; + } __attribute__((packed)) bc = { + .cmd0 = BC_INCREFS, + .arg0 = 0, + .cmd1 = BC_REQUEST_FREEZE_NOTIFICATION, + // This BC_REQUEST_FREEZE_NOTIFICATION should lead to a pending + // frozen notification inserted into the queue. + .arg1 = + { + .handle = 0, + .cookie = cookie, + }, + // Send BC_CLEAR_FREEZE_NOTIFICATION before the above frozen + // notification has a chance of being sent. The notification should + // be overwritten. Userspace is expected to only receive + // BR_CLEAR_FREEZE_NOTIFICATION_DONE. + .cmd2 = BC_CLEAR_FREEZE_NOTIFICATION, + .arg2 = + { + .handle = 0, + .cookie = cookie, + }, + .cmd3 = BC_DECREFS, + .arg3 = 0, + }; + struct { + uint32_t cmd0; + uint32_t cmd1; + binder_uintptr_t arg1; + uint32_t pad[16]; + } __attribute__((packed)) br; + struct binder_write_read bwr = binder_write_read(); + + bwr.write_buffer = (uintptr_t)&bc; + bwr.write_size = sizeof(bc); + bwr.read_buffer = (uintptr_t)&br; + bwr.read_size = sizeof(br); + + binderTestIoctl(BINDER_WRITE_READ, &bwr); + EXPECT_EQ(sizeof(bc), bwr.write_consumed); + EXPECT_EQ(sizeof(br) - sizeof(br.pad), bwr.read_consumed); + EXPECT_EQ(BR_NOOP, br.cmd0); + EXPECT_EQ(BR_CLEAR_FREEZE_NOTIFICATION_DONE, br.cmd1); + EXPECT_EQ(cookie, br.arg1); + binderTestReadEmpty(); +} + +TEST_F(BinderDriverInterfaceTest, ResendFrozenNotification) { + if (!android::ProcessState::isDriverFeatureEnabled( + android::ProcessState::DriverFeature::FREEZE_NOTIFICATION)) { + GTEST_SKIP() << "Skipping test for kernels that support freeze notification"; + return; + } + binder_uintptr_t cookie = 1234; + struct { + uint32_t cmd0; + uint32_t arg0; + uint32_t cmd1; + struct binder_handle_cookie arg1; + } __attribute__((packed)) bc1 = { + .cmd0 = BC_INCREFS, + .arg0 = 0, + .cmd1 = BC_REQUEST_FREEZE_NOTIFICATION, + .arg1 = + { + .handle = 0, + .cookie = cookie, + }, + }; + struct { + uint32_t cmd0; + uint32_t cmd1; + struct binder_frozen_state_info arg1; + uint32_t pad[16]; + } __attribute__((packed)) br1; + struct { + uint32_t cmd2; + struct binder_handle_cookie arg2; + } __attribute__((packed)) bc2 = { + // Clear the notification before acknowledging the in-flight + // BR_FROZEN_BINDER. Kernel should hold off sending + // BR_CLEAR_FREEZE_NOTIFICATION_DONE until the acknowledgement + // reaches kernel. + .cmd2 = BC_CLEAR_FREEZE_NOTIFICATION, + .arg2 = + { + .handle = 0, + .cookie = cookie, + }, + }; + struct { + uint32_t pad[16]; + } __attribute__((packed)) br2; + struct { + uint32_t cmd3; + binder_uintptr_t arg3; + uint32_t cmd4; + uint32_t arg4; + } __attribute__((packed)) bc3 = { + // Send the acknowledgement. Now the kernel should send out + // BR_CLEAR_FREEZE_NOTIFICATION_DONE. + .cmd3 = BC_FREEZE_NOTIFICATION_DONE, + .arg3 = cookie, + .cmd4 = BC_DECREFS, + .arg4 = 0, + }; + struct { + uint32_t cmd2; + uint32_t cmd3; + binder_uintptr_t arg3; + uint32_t pad[16]; + } __attribute__((packed)) br3; + + struct binder_write_read bwr1 = binder_write_read(); + bwr1.write_buffer = (uintptr_t)&bc1; + bwr1.write_size = sizeof(bc1); + bwr1.read_buffer = (uintptr_t)&br1; + bwr1.read_size = sizeof(br1); + binderTestIoctl(BINDER_WRITE_READ, &bwr1); + EXPECT_EQ(sizeof(bc1), bwr1.write_consumed); + EXPECT_EQ(sizeof(br1) - sizeof(br1.pad), bwr1.read_consumed); + EXPECT_EQ(BR_NOOP, br1.cmd0); + ASSERT_EQ(BR_FROZEN_BINDER, br1.cmd1); + EXPECT_FALSE(br1.arg1.is_frozen); + + struct binder_write_read bwr2 = binder_write_read(); + bwr2.write_buffer = (uintptr_t)&bc2; + bwr2.write_size = sizeof(bc2); + bwr2.read_buffer = (uintptr_t)&br2; + bwr2.read_size = sizeof(br2); + binderTestIoctlSuccessOrError(BINDER_WRITE_READ, &bwr2, EAGAIN); + binderTestReadEmpty(); + + struct binder_write_read bwr3 = binder_write_read(); + bwr3.write_buffer = (uintptr_t)&bc3; + bwr3.write_size = sizeof(bc3); + bwr3.read_buffer = (uintptr_t)&br3; + bwr3.read_size = sizeof(br3); + binderTestIoctl(BINDER_WRITE_READ, &bwr3); + EXPECT_EQ(sizeof(bc3), bwr3.write_consumed); + EXPECT_EQ(sizeof(br3) - sizeof(br3.pad), bwr3.read_consumed); + EXPECT_EQ(BR_CLEAR_FREEZE_NOTIFICATION_DONE, br3.cmd3); + EXPECT_EQ(cookie, br3.arg3); + binderTestReadEmpty(); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 9b1ba01146..bcab6decca 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -27,6 +27,7 @@ #include <gmock/gmock.h> #include <gtest/gtest.h> +#include <android-base/logging.h> #include <android-base/properties.h> #include <android-base/result-gmock.h> #include <binder/Binder.h> @@ -39,6 +40,8 @@ #include <binder/RpcSession.h> #include <binder/Status.h> #include <binder/unique_fd.h> +#include <input/BlockingQueue.h> +#include <processgroup/processgroup.h> #include <utils/Flattenable.h> #include <linux/sched.h> @@ -57,6 +60,7 @@ using namespace std::chrono_literals; using android::base::testing::HasValue; using android::binder::Status; using android::binder::unique_fd; +using std::chrono_literals::operator""ms; using testing::ExplainMatchResult; using testing::Matcher; using testing::Not; @@ -115,6 +119,8 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_NOP_TRANSACTION_WAIT, BINDER_LIB_TEST_GETPID, BINDER_LIB_TEST_GETUID, + BINDER_LIB_TEST_LISTEN_FOR_FROZEN_STATE_CHANGE, + BINDER_LIB_TEST_CONSUME_STATE_CHANGE_EVENTS, BINDER_LIB_TEST_ECHO_VECTOR, BINDER_LIB_TEST_GET_NON_BLOCKING_FD, BINDER_LIB_TEST_REJECT_OBJECTS, @@ -247,6 +253,43 @@ class BinderLibTestEnv : public ::testing::Environment { sp<IBinder> m_server; }; +class TestFrozenStateChangeCallback : public IBinder::FrozenStateChangeCallback { +public: + BlockingQueue<std::pair<const wp<IBinder>, State>> events; + + virtual void onStateChanged(const wp<IBinder>& who, State state) { + events.push(std::make_pair(who, state)); + } + + void ensureFrozenEventReceived() { + auto event = events.popWithTimeout(500ms); + ASSERT_TRUE(event.has_value()); + EXPECT_EQ(State::FROZEN, event->second); // isFrozen should be true + EXPECT_EQ(0u, events.size()); + } + + void ensureUnfrozenEventReceived() { + auto event = events.popWithTimeout(500ms); + ASSERT_TRUE(event.has_value()); + EXPECT_EQ(State::UNFROZEN, event->second); // isFrozen should be false + EXPECT_EQ(0u, events.size()); + } + + std::vector<bool> getAllAndClear() { + std::vector<bool> results; + while (true) { + auto event = events.popWithTimeout(0ms); + if (!event.has_value()) { + break; + } + results.push_back(event->second == State::FROZEN); + } + return results; + } + + sp<IBinder> binder; +}; + class BinderLibTest : public ::testing::Test { public: virtual void SetUp() { @@ -291,6 +334,51 @@ class BinderLibTest : public ::testing::Test { EXPECT_EQ(1, ret); } + bool checkFreezeSupport() { + std::ifstream freezer_file("/sys/fs/cgroup/uid_0/cgroup.freeze"); + // Pass test on devices where the cgroup v2 freezer is not supported + if (freezer_file.fail()) { + return false; + } + return IPCThreadState::self()->freeze(getpid(), false, 0) == NO_ERROR; + } + + bool checkFreezeAndNotificationSupport() { + if (!checkFreezeSupport()) { + return false; + } + return ProcessState::isDriverFeatureEnabled( + ProcessState::DriverFeature::FREEZE_NOTIFICATION); + } + + bool getBinderPid(int32_t* pid, sp<IBinder> server) { + Parcel data, replypid; + if (server->transact(BINDER_LIB_TEST_GETPID, data, &replypid) != NO_ERROR) { + ALOGE("BINDER_LIB_TEST_GETPID failed"); + return false; + } + *pid = replypid.readInt32(); + if (*pid <= 0) { + ALOGE("pid should be greater than zero"); + return false; + } + return true; + } + + void freezeProcess(int32_t pid) { + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, true, 1000)); + } + + void unfreezeProcess(int32_t pid) { + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, false, 0)); + } + + void removeCallbackAndValidateNoEvent(sp<IBinder> binder, + sp<TestFrozenStateChangeCallback> callback) { + EXPECT_THAT(binder->removeFrozenStateChangeCallback(callback), StatusEq(NO_ERROR)); + EXPECT_EQ(0u, callback->events.size()); + } + sp<IBinder> m_server; }; @@ -516,29 +604,18 @@ TEST_F(BinderLibTest, NopTransactionClear) { } TEST_F(BinderLibTest, Freeze) { - Parcel data, reply, replypid; - std::ifstream freezer_file("/sys/fs/cgroup/uid_0/cgroup.freeze"); - - // Pass test on devices where the cgroup v2 freezer is not supported - if (freezer_file.fail()) { - GTEST_SKIP(); + if (!checkFreezeSupport()) { + GTEST_SKIP() << "Skipping test for kernels that do not support proceess freezing"; return; } - + Parcel data, reply, replypid; EXPECT_THAT(m_server->transact(BINDER_LIB_TEST_GETPID, data, &replypid), StatusEq(NO_ERROR)); int32_t pid = replypid.readInt32(); for (int i = 0; i < 10; i++) { EXPECT_EQ(NO_ERROR, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION_WAIT, data, &reply, TF_ONE_WAY)); } - // Pass test on devices where BINDER_FREEZE ioctl is not supported - int ret = IPCThreadState::self()->freeze(pid, false, 0); - if (ret == -EINVAL) { - GTEST_SKIP(); - return; - } - EXPECT_EQ(NO_ERROR, ret); - + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, false, 0)); EXPECT_EQ(-EAGAIN, IPCThreadState::self()->freeze(pid, true, 0)); // b/268232063 - succeeds ~0.08% of the time @@ -835,6 +912,199 @@ TEST_F(BinderLibTest, DeathNotificationThread) EXPECT_THAT(callback->getResult(), StatusEq(NO_ERROR)); } +TEST_F(BinderLibTest, ReturnErrorIfKernelDoesNotSupportFreezeNotification) { + if (ProcessState::isDriverFeatureEnabled(ProcessState::DriverFeature::FREEZE_NOTIFICATION)) { + GTEST_SKIP() << "Skipping test for kernels that support FREEZE_NOTIFICATION"; + return; + } + sp<TestFrozenStateChangeCallback> callback = sp<TestFrozenStateChangeCallback>::make(); + sp<IBinder> binder = addServer(); + ASSERT_NE(nullptr, binder); + ASSERT_EQ(nullptr, binder->localBinder()); + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback), StatusEq(INVALID_OPERATION)); +} + +TEST_F(BinderLibTest, FrozenStateChangeNotificatiion) { + if (!checkFreezeAndNotificationSupport()) { + GTEST_SKIP() << "Skipping test for kernels that do not support FREEZE_NOTIFICATION"; + return; + } + sp<TestFrozenStateChangeCallback> callback = sp<TestFrozenStateChangeCallback>::make(); + sp<IBinder> binder = addServer(); + ASSERT_NE(nullptr, binder); + int32_t pid; + ASSERT_TRUE(getBinderPid(&pid, binder)); + + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback), StatusEq(NO_ERROR)); + // Expect current state (unfrozen) to be delivered immediately. + callback->ensureUnfrozenEventReceived(); + // Check that the process hasn't died otherwise there's a risk of freezing + // the wrong process. + EXPECT_EQ(OK, binder->pingBinder()); + freezeProcess(pid); + callback->ensureFrozenEventReceived(); + unfreezeProcess(pid); + callback->ensureUnfrozenEventReceived(); + removeCallbackAndValidateNoEvent(binder, callback); +} + +TEST_F(BinderLibTest, AddFrozenCallbackWhenFrozen) { + if (!checkFreezeAndNotificationSupport()) { + GTEST_SKIP() << "Skipping test for kernels that do not support FREEZE_NOTIFICATION"; + return; + } + sp<TestFrozenStateChangeCallback> callback = sp<TestFrozenStateChangeCallback>::make(); + sp<IBinder> binder = addServer(); + ASSERT_NE(nullptr, binder); + int32_t pid; + ASSERT_TRUE(getBinderPid(&pid, binder)); + + // Check that the process hasn't died otherwise there's a risk of freezing + // the wrong process. + EXPECT_EQ(OK, binder->pingBinder()); + freezeProcess(pid); + // Add the callback while the target process is frozen. + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback), StatusEq(NO_ERROR)); + callback->ensureFrozenEventReceived(); + unfreezeProcess(pid); + callback->ensureUnfrozenEventReceived(); + removeCallbackAndValidateNoEvent(binder, callback); + + // Check that the process hasn't died otherwise there's a risk of freezing + // the wrong process. + EXPECT_EQ(OK, binder->pingBinder()); + freezeProcess(pid); + unfreezeProcess(pid); + // Make sure no callback happens since the listener has been removed. + EXPECT_EQ(0u, callback->events.size()); +} + +TEST_F(BinderLibTest, NoFrozenNotificationAfterCallbackRemoval) { + if (!checkFreezeAndNotificationSupport()) { + GTEST_SKIP() << "Skipping test for kernels that do not support FREEZE_NOTIFICATION"; + return; + } + sp<TestFrozenStateChangeCallback> callback = sp<TestFrozenStateChangeCallback>::make(); + sp<IBinder> binder = addServer(); + ASSERT_NE(nullptr, binder); + int32_t pid; + ASSERT_TRUE(getBinderPid(&pid, binder)); + + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback), StatusEq(NO_ERROR)); + callback->ensureUnfrozenEventReceived(); + removeCallbackAndValidateNoEvent(binder, callback); + + // Make sure no callback happens after the listener is removed. + freezeProcess(pid); + unfreezeProcess(pid); + EXPECT_EQ(0u, callback->events.size()); +} + +TEST_F(BinderLibTest, MultipleFrozenStateChangeCallbacks) { + if (!checkFreezeAndNotificationSupport()) { + GTEST_SKIP() << "Skipping test for kernels that do not support FREEZE_NOTIFICATION"; + return; + } + sp<TestFrozenStateChangeCallback> callback1 = sp<TestFrozenStateChangeCallback>::make(); + sp<TestFrozenStateChangeCallback> callback2 = sp<TestFrozenStateChangeCallback>::make(); + sp<IBinder> binder = addServer(); + ASSERT_NE(nullptr, binder); + int32_t pid; + ASSERT_TRUE(getBinderPid(&pid, binder)); + + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback1), StatusEq(NO_ERROR)); + // Expect current state (unfrozen) to be delivered immediately. + callback1->ensureUnfrozenEventReceived(); + + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback2), StatusEq(NO_ERROR)); + // Expect current state (unfrozen) to be delivered immediately. + callback2->ensureUnfrozenEventReceived(); + + freezeProcess(pid); + callback1->ensureFrozenEventReceived(); + callback2->ensureFrozenEventReceived(); + + removeCallbackAndValidateNoEvent(binder, callback1); + unfreezeProcess(pid); + EXPECT_EQ(0u, callback1->events.size()); + callback2->ensureUnfrozenEventReceived(); + removeCallbackAndValidateNoEvent(binder, callback2); + + freezeProcess(pid); + EXPECT_EQ(0u, callback2->events.size()); +} + +TEST_F(BinderLibTest, RemoveThenAddFrozenStateChangeCallbacks) { + if (!checkFreezeAndNotificationSupport()) { + GTEST_SKIP() << "Skipping test for kernels that do not support FREEZE_NOTIFICATION"; + return; + } + sp<TestFrozenStateChangeCallback> callback = sp<TestFrozenStateChangeCallback>::make(); + sp<IBinder> binder = addServer(); + ASSERT_NE(nullptr, binder); + int32_t pid; + ASSERT_TRUE(getBinderPid(&pid, binder)); + + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback), StatusEq(NO_ERROR)); + // Expect current state (unfrozen) to be delivered immediately. + callback->ensureUnfrozenEventReceived(); + removeCallbackAndValidateNoEvent(binder, callback); + + EXPECT_THAT(binder->addFrozenStateChangeCallback(callback), StatusEq(NO_ERROR)); + callback->ensureUnfrozenEventReceived(); +} + +TEST_F(BinderLibTest, CoalesceFreezeCallbacksWhenListenerIsFrozen) { + if (!checkFreezeAndNotificationSupport()) { + GTEST_SKIP() << "Skipping test for kernels that do not support FREEZE_NOTIFICATION"; + return; + } + sp<IBinder> binder = addServer(); + sp<IBinder> listener = addServer(); + ASSERT_NE(nullptr, binder); + ASSERT_NE(nullptr, listener); + int32_t pid, listenerPid; + ASSERT_TRUE(getBinderPid(&pid, binder)); + ASSERT_TRUE(getBinderPid(&listenerPid, listener)); + + // Ask the listener process to register for state change callbacks. + { + Parcel data, reply; + data.writeStrongBinder(binder); + ASSERT_THAT(listener->transact(BINDER_LIB_TEST_LISTEN_FOR_FROZEN_STATE_CHANGE, data, + &reply), + StatusEq(NO_ERROR)); + } + // Freeze the listener process. + freezeProcess(listenerPid); + createProcessGroup(getuid(), listenerPid); + ASSERT_TRUE(SetProcessProfiles(getuid(), listenerPid, {"Frozen"})); + // Repeatedly flip the target process between frozen and unfrozen states. + for (int i = 0; i < 1000; i++) { + usleep(50); + unfreezeProcess(pid); + usleep(50); + freezeProcess(pid); + } + // Unfreeze the listener process. Now it should receive the frozen state + // change notifications. + ASSERT_TRUE(SetProcessProfiles(getuid(), listenerPid, {"Unfrozen"})); + unfreezeProcess(listenerPid); + // Wait for 500ms to give the process enough time to wake up and handle + // notifications. + usleep(500 * 1000); + { + std::vector<bool> events; + Parcel data, reply; + ASSERT_THAT(listener->transact(BINDER_LIB_TEST_CONSUME_STATE_CHANGE_EVENTS, data, &reply), + StatusEq(NO_ERROR)); + reply.readBoolVector(&events); + // There should only be one single state change notifications delievered. + ASSERT_EQ(1u, events.size()); + EXPECT_TRUE(events[0]); + } +} + TEST_F(BinderLibTest, PassFile) { int ret; int pipefd[2]; @@ -1981,6 +2251,26 @@ public: reply->writeInt32(param.sched_priority); return NO_ERROR; } + case BINDER_LIB_TEST_LISTEN_FOR_FROZEN_STATE_CHANGE: { + sp<IBinder> binder = data.readStrongBinder(); + frozenStateChangeCallback = sp<TestFrozenStateChangeCallback>::make(); + // Hold an strong pointer to the binder object so it doesn't go + // away. + frozenStateChangeCallback->binder = binder; + int ret = binder->addFrozenStateChangeCallback(frozenStateChangeCallback); + if (ret != NO_ERROR) { + return ret; + } + auto event = frozenStateChangeCallback->events.popWithTimeout(10ms); + if (!event.has_value()) { + return NOT_ENOUGH_DATA; + } + return NO_ERROR; + } + case BINDER_LIB_TEST_CONSUME_STATE_CHANGE_EVENTS: { + reply->writeBoolVector(frozenStateChangeCallback->getAllAndClear()); + return NO_ERROR; + } case BINDER_LIB_TEST_ECHO_VECTOR: { std::vector<uint64_t> vector; auto err = data.readUint64Vector(&vector); @@ -2067,6 +2357,7 @@ private: sp<IBinder> m_callback; bool m_exitOnDestroy; std::mutex m_blockMutex; + sp<TestFrozenStateChangeCallback> frozenStateChangeCallback; }; int run_server(int index, int readypipefd, bool usePoll) diff --git a/libs/binder/tests/binderRecordReplayTest.cpp b/libs/binder/tests/binderRecordReplayTest.cpp index b975fad2c9..f867b42b1c 100644 --- a/libs/binder/tests/binderRecordReplayTest.cpp +++ b/libs/binder/tests/binderRecordReplayTest.cpp @@ -99,12 +99,12 @@ public: GENERATE_GETTER_SETTER(SingleDataParcelableArray, std::vector<SingleDataParcelable>); Status setFileDescriptor(unique_fd input) { - mFd = std::move(unique_fd(dup(input))); + mFd = unique_fd(dup(input)); return Status::ok(); } Status getFileDescriptor(unique_fd* output) { - *output = std::move(unique_fd(dup(mFd))); + *output = unique_fd(dup(mFd)); return Status::ok(); } unique_fd mFd; @@ -117,7 +117,7 @@ std::vector<uint8_t> retrieveData(borrowed_fd fd) { std::vector<uint8_t> buffer(fdStat.st_size); auto readResult = android::base::ReadFully(fd, buffer.data(), fdStat.st_size); EXPECT_TRUE(readResult != 0); - return std::move(buffer); + return buffer; } void replayFuzzService(const sp<BpBinder>& binder, const RecordedTransaction& transaction) { @@ -387,8 +387,8 @@ TEST_F(BinderRecordReplayTest, ReplayFd) { // When fds are replayed, it will be replaced by /dev/null..reading from it should yield // null data - recordReplay(&IBinderRecordReplayTest::setFileDescriptor, std::move(unique_fd(dup(saved))), - &IBinderRecordReplayTest::getFileDescriptor, std::move(unique_fd(dup(changed)))); + recordReplay(&IBinderRecordReplayTest::setFileDescriptor, unique_fd(dup(saved)), + &IBinderRecordReplayTest::getFileDescriptor, unique_fd(dup(changed))); } int main(int argc, char** argv) { diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index cd78e82322..3038de9a41 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -1384,8 +1384,8 @@ TEST(BinderRpc, Java) { sp<IServiceManager> sm = defaultServiceManager(); ASSERT_NE(nullptr, sm); // Any Java service with non-empty getInterfaceDescriptor() would do. - // Let's pick batteryproperties. - auto binder = sm->checkService(String16("batteryproperties")); + // Let's pick activity. + auto binder = sm->checkService(String16("activity")); ASSERT_NE(nullptr, binder); auto descriptor = binder->getInterfaceDescriptor(); ASSERT_GE(descriptor.size(), 0u); diff --git a/libs/binder/tests/binderRpcUniversalTests.cpp b/libs/binder/tests/binderRpcUniversalTests.cpp index f4807806ad..2cec243c38 100644 --- a/libs/binder/tests/binderRpcUniversalTests.cpp +++ b/libs/binder/tests/binderRpcUniversalTests.cpp @@ -323,6 +323,22 @@ TEST_P(BinderRpc, CannotSendSocketBinderOverRegularBinder) { // END TESTS FOR LIMITATIONS OF SOCKET BINDER +class TestFrozenStateChangeCallback : public IBinder::FrozenStateChangeCallback { +public: + virtual void onStateChanged(const wp<IBinder>&, State) {} +}; + +TEST_P(BinderRpc, RpcBinderShouldFailOnFrozenStateCallbacks) { + auto proc = createRpcTestSocketServerProcess({}); + + sp<IBinder> a; + sp<TestFrozenStateChangeCallback> callback = sp<TestFrozenStateChangeCallback>::make(); + EXPECT_OK(proc.rootIface->alwaysGiveMeTheSameBinder(&a)); + EXPECT_DEATH_IF_SUPPORTED( + { std::ignore = a->addFrozenStateChangeCallback(callback); }, + "addFrozenStateChangeCallback\\(\\) is not supported for RPC Binder."); +} + TEST_P(BinderRpc, RepeatRootObject) { auto proc = createRpcTestSocketServerProcess({}); diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp index fd9777a916..0ed8a554ac 100644 --- a/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp +++ b/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp @@ -55,7 +55,7 @@ std::vector<uint8_t> reverseBytes(T min, T max, T val) { offset += CHAR_BIT; } - return std::move(reverseData); + return reverseData; } template <typename T> diff --git a/libs/math/include/math/TVecHelpers.h b/libs/math/include/math/TVecHelpers.h index 0dac662e97..7278d2d3e1 100644 --- a/libs/math/include/math/TVecHelpers.h +++ b/libs/math/include/math/TVecHelpers.h @@ -620,15 +620,10 @@ public: } // namespace details } // namespace android -namespace std { - template<template<typename T> class VECTOR, typename T> - struct hash<VECTOR<T>> { - static constexpr bool IS_VECTOR = - std::is_base_of<android::details::TVecUnaryOperators<VECTOR, T>, VECTOR<T>>::value; - - typename std::enable_if<IS_VECTOR, size_t>::type - operator()(const VECTOR<T>& v) const { - return v.hash(); - } - }; -} +#define TVECHELPERS_STD_HASH(VECTOR) \ + template <typename T> \ + struct std::hash<VECTOR<T>> { \ + size_t operator()(const VECTOR<T>& v) const { \ + return v.hash(); \ + } \ + } diff --git a/libs/math/include/math/mat2.h b/libs/math/include/math/mat2.h index 3e6cd4c794..24c2bad420 100644 --- a/libs/math/include/math/mat2.h +++ b/libs/math/include/math/mat2.h @@ -373,5 +373,7 @@ typedef details::TMat22<float> mat2f; // ---------------------------------------------------------------------------------------- } // namespace android +TVECHELPERS_STD_HASH(android::details::TMat22); + #undef PURE #undef CONSTEXPR diff --git a/libs/math/include/math/mat3.h b/libs/math/include/math/mat3.h index 5c8a9b2573..4647a60917 100644 --- a/libs/math/include/math/mat3.h +++ b/libs/math/include/math/mat3.h @@ -436,5 +436,7 @@ typedef details::TMat33<float> mat3f; // ---------------------------------------------------------------------------------------- } // namespace android +TVECHELPERS_STD_HASH(android::details::TMat33); + #undef PURE #undef CONSTEXPR diff --git a/libs/math/include/math/mat4.h b/libs/math/include/math/mat4.h index c630d972b2..c9e118ab34 100644 --- a/libs/math/include/math/mat4.h +++ b/libs/math/include/math/mat4.h @@ -590,5 +590,7 @@ typedef details::TMat44<float> mat4f; // ---------------------------------------------------------------------------------------- } // namespace android +TVECHELPERS_STD_HASH(android::details::TMat44); + #undef PURE #undef CONSTEXPR diff --git a/libs/math/include/math/quat.h b/libs/math/include/math/quat.h index 07573c5ecf..43c8038cc1 100644 --- a/libs/math/include/math/quat.h +++ b/libs/math/include/math/quat.h @@ -187,6 +187,8 @@ constexpr inline quatd operator"" _kd(unsigned long long v) { // NOLINT // ---------------------------------------------------------------------------------------- } // namespace android +TVECHELPERS_STD_HASH(android::details::TQuaternion); + #pragma clang diagnostic pop #undef PURE diff --git a/libs/math/include/math/vec2.h b/libs/math/include/math/vec2.h index e0adb7f6cc..909c77eb28 100644 --- a/libs/math/include/math/vec2.h +++ b/libs/math/include/math/vec2.h @@ -122,4 +122,6 @@ typedef details::TVec2<bool> bool2; // ---------------------------------------------------------------------------------------- } // namespace android +TVECHELPERS_STD_HASH(android::details::TVec2); + #pragma clang diagnostic pop diff --git a/libs/math/include/math/vec3.h b/libs/math/include/math/vec3.h index 21fb684efc..ff2b3e4f54 100644 --- a/libs/math/include/math/vec3.h +++ b/libs/math/include/math/vec3.h @@ -128,4 +128,6 @@ typedef details::TVec3<bool> bool3; // ---------------------------------------------------------------------------------------- } // namespace android +TVECHELPERS_STD_HASH(android::details::TVec3); + #pragma clang diagnostic pop diff --git a/libs/math/include/math/vec4.h b/libs/math/include/math/vec4.h index 1e279fe3f2..16509c91a1 100644 --- a/libs/math/include/math/vec4.h +++ b/libs/math/include/math/vec4.h @@ -125,4 +125,6 @@ typedef details::TVec4<bool> bool4; // ---------------------------------------------------------------------------------------- } // namespace android +TVECHELPERS_STD_HASH(android::details::TVec4); + #pragma clang diagnostic pop diff --git a/opengl/Android.bp b/opengl/Android.bp index 4454f36b67..37dc9314e2 100644 --- a/opengl/Android.bp +++ b/opengl/Android.bp @@ -30,6 +30,10 @@ ndk_headers { to: "", srcs: ["include/EGL/**/*.h"], license: "include/EGL/NOTICE", + // eglext.h is not self-contained. Safe to skip C-compat verification + // though since upstream also cares about C compatibility, and the header is + // auto-generated anyway. + skip_verification: true, } ndk_headers { @@ -38,6 +42,10 @@ ndk_headers { to: "", srcs: ["include/GLES/**/*.h"], license: "include/GLES/NOTICE", + // glext.h is not self-contained. Safe to skip C-compat verification + // though since upstream also cares about C compatibility, and the header is + // auto-generated anyway. + skip_verification: true, } ndk_headers { @@ -46,6 +54,10 @@ ndk_headers { to: "", srcs: ["include/GLES2/**/*.h"], license: "include/GLES2/NOTICE", + // gl2ext.h is not self-contained. Safe to skip C-compat verification + // though since upstream also cares about C compatibility, and the header is + // auto-generated anyway. + skip_verification: true, } ndk_headers { diff --git a/opengl/libs/EGL/Loader.cpp b/opengl/libs/EGL/Loader.cpp index af0bcffc1f..3297fe037e 100644 --- a/opengl/libs/EGL/Loader.cpp +++ b/opengl/libs/EGL/Loader.cpp @@ -262,7 +262,7 @@ void* Loader::open(egl_connection_t* cnx) { hnd = attempt_to_load_updated_driver(cnx); // If updated driver apk is set but fail to load, abort here. - LOG_ALWAYS_FATAL_IF(android::GraphicsEnv::getInstance().getDriverNamespace(), + LOG_ALWAYS_FATAL_IF(android::GraphicsEnv::getInstance().getDriverNamespace() && !hnd, "couldn't find an OpenGL ES implementation from %s", android::GraphicsEnv::getInstance().getDriverPath().c_str()); } diff --git a/services/surfaceflinger/CompositionEngine/Android.bp b/services/surfaceflinger/CompositionEngine/Android.bp index dad945c964..05af4ed153 100644 --- a/services/surfaceflinger/CompositionEngine/Android.bp +++ b/services/surfaceflinger/CompositionEngine/Android.bp @@ -49,6 +49,7 @@ cc_defaults { "libtonemap", "libaidlcommonsupport", "libprocessgroup", + "libprocessgroup_util", "libcgrouprc", "libjsoncpp", "libcgrouprc_format", diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp index 12ab2c284a..be95913b89 100644 --- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp +++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp @@ -209,7 +209,7 @@ sp<GraphicBuffer> allocateClearSlotBuffer() { if (!buffer || buffer->initCheck() != ::android::OK) { return nullptr; } - return std::move(buffer); + return buffer; } } // anonymous namespace diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp index 2dc89b55ba..3752d5e0a5 100644 --- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp +++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp @@ -62,12 +62,12 @@ perfetto::protos::TransactionState TransactionProtoParser::toProto(const Transac proto.mutable_layer_changes()->Reserve(static_cast<int32_t>(t.states.size())); for (auto& layerState : t.states) { - proto.mutable_layer_changes()->Add(std::move(toProto(layerState))); + proto.mutable_layer_changes()->Add(toProto(layerState)); } proto.mutable_display_changes()->Reserve(static_cast<int32_t>(t.displays.size())); for (auto& displayState : t.displays) { - proto.mutable_display_changes()->Add(std::move(toProto(displayState))); + proto.mutable_display_changes()->Add(toProto(displayState)); } proto.mutable_merged_transaction_ids()->Reserve( diff --git a/services/surfaceflinger/Tracing/TransactionTracing.cpp b/services/surfaceflinger/Tracing/TransactionTracing.cpp index 696f3489bf..bc9f8094f2 100644 --- a/services/surfaceflinger/Tracing/TransactionTracing.cpp +++ b/services/surfaceflinger/Tracing/TransactionTracing.cpp @@ -244,7 +244,7 @@ void TransactionTracing::addEntry(const std::vector<CommittedUpdates>& committed static_cast<int32_t>(update.createdLayers.size())); for (const auto& args : update.createdLayers) { - entryProto.mutable_added_layers()->Add(std::move(mProtoParser.toProto(args))); + entryProto.mutable_added_layers()->Add(mProtoParser.toProto(args)); } entryProto.mutable_destroyed_layers()->Reserve( @@ -276,7 +276,7 @@ void TransactionTracing::addEntry(const std::vector<CommittedUpdates>& committed static_cast<int32_t>(update.displayInfos.size())); for (auto& [layerStack, displayInfo] : update.displayInfos) { entryProto.mutable_displays()->Add( - std::move(mProtoParser.toProto(displayInfo, layerStack.id))); + mProtoParser.toProto(displayInfo, layerStack.id)); } } diff --git a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp index fb4ef70450..7bf167498b 100644 --- a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp @@ -131,14 +131,14 @@ protected: // add layers and add some layer transaction { frontend::Update update; - update.layerCreationArgs.emplace_back(std::move( + update.layerCreationArgs.emplace_back( getLayerCreationArgs(mParentLayerId, /*parentId=*/UNASSIGNED_LAYER_ID, /*layerIdToMirror=*/UNASSIGNED_LAYER_ID, /*flags=*/123, - /*addToRoot=*/true))); - update.layerCreationArgs.emplace_back(std::move( + /*addToRoot=*/true)); + update.layerCreationArgs.emplace_back( getLayerCreationArgs(mChildLayerId, mParentLayerId, /*layerIdToMirror=*/UNASSIGNED_LAYER_ID, /*flags=*/456, - /*addToRoot=*/true))); + /*addToRoot=*/true)); TransactionState transaction; transaction.id = 50; ResolvedComposerState layerState; diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 81fd1185b6..e9204ab65a 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -339,10 +339,13 @@ void Hal::UnloadBuiltinDriver() { ALOGD("Unload builtin Vulkan driver."); - // Close the opened device - int err = hal_.dev_->common.close( - const_cast<struct hw_device_t*>(&hal_.dev_->common)); - ALOG_ASSERT(!err, "hw_device_t::close() failed."); + if (hal_.dev_->common.close != nullptr) + { + // Close the opened device + int err = hal_.dev_->common.close( + const_cast<struct hw_device_t*>(&hal_.dev_->common)); + ALOG_ASSERT(!err, "hw_device_t::close() failed."); + } // Close the opened shared library in the hw_module_t android_unload_sphal_library(hal_.dev_->common.module->dso); |