diff options
author | Jeevan Prasath <sivasaj@google.com> | 2023-03-01 19:55:46 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2023-03-01 19:55:46 +0000 |
commit | c185e37030ce74039788a014a38e39509c27c3ff (patch) | |
tree | ca958d527f8d11476819dd3d93defbae464d17e9 | |
parent | f8ce875c134b1e6ce08171c2cb4b4f3232a90b08 (diff) | |
parent | 55cc7345a9f21992bf39133a55be6e71eb11bee3 (diff) | |
download | native-c185e37030ce74039788a014a38e39509c27c3ff.tar.gz |
Merge "EventHub: Prevent duplicate descriptors" into android12L-devandroid12L-dev
-rw-r--r-- | services/inputflinger/reader/EventHub.cpp | 36 | ||||
-rw-r--r-- | services/inputflinger/reader/InputReader.cpp | 25 | ||||
-rw-r--r-- | services/inputflinger/reader/include/EventHub.h | 4 | ||||
-rw-r--r-- | services/inputflinger/tests/EventHub_test.cpp | 14 |
4 files changed, 65 insertions, 14 deletions
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index 1c76dbb386..b87964932b 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -1241,7 +1241,8 @@ static std::string generateDescriptor(InputDeviceIdentifier& identifier) { if (!identifier.uniqueId.empty()) { rawDescriptor += "uniqueId:"; rawDescriptor += identifier.uniqueId; - } else if (identifier.nonce != 0) { + } + if (identifier.nonce != 0) { rawDescriptor += StringPrintf("nonce:%04x", identifier.nonce); } @@ -1269,16 +1270,20 @@ void EventHub::assignDescriptorLocked(InputDeviceIdentifier& identifier) { // of Android. In practice we sometimes get devices that cannot be uniquely // identified. In this case we enforce uniqueness between connected devices. // Ideally, we also want the descriptor to be short and relatively opaque. + // Note that we explicitly do not use the path or location for external devices + // as their path or location will change as they are plugged/unplugged or moved + // to different ports. We do fallback to using name and location in the case of + // internal devices which are detected by the vendor and product being 0 in + // generateDescriptor. If two identical descriptors are detected we will fallback + // to using a 'nonce' and incrementing it until the new descriptor no longer has + // a match with any existing descriptors. identifier.nonce = 0; std::string rawDescriptor = generateDescriptor(identifier); - if (identifier.uniqueId.empty()) { - // If it didn't have a unique id check for conflicts and enforce - // uniqueness if necessary. - while (getDeviceByDescriptorLocked(identifier.descriptor) != nullptr) { - identifier.nonce++; - rawDescriptor = generateDescriptor(identifier); - } + // Enforce that the generated descriptor is unique. + while (hasDeviceWithDescriptorLocked(identifier.descriptor)) { + identifier.nonce++; + rawDescriptor = generateDescriptor(identifier); } ALOGV("Created descriptor: raw=%s, cooked=%s", rawDescriptor.c_str(), identifier.descriptor.c_str()); @@ -1353,13 +1358,22 @@ std::vector<int32_t> EventHub::getVibratorIds(int32_t deviceId) { return vibrators; } -EventHub::Device* EventHub::getDeviceByDescriptorLocked(const std::string& descriptor) const { +/** + * Checks both mDevices and mOpeningDevices for a device with the descriptor passed. + */ +bool EventHub::hasDeviceWithDescriptorLocked(const std::string& descriptor) const { + for (const auto& device : mOpeningDevices) { + if (descriptor == device->identifier.descriptor) { + return true; + } + } + for (const auto& [id, device] : mDevices) { if (descriptor == device->identifier.descriptor) { - return device.get(); + return true; } } - return nullptr; + return false; } EventHub::Device* EventHub::getDeviceLocked(int32_t deviceId) const { diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 5120860503..fc398a2513 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -38,6 +38,26 @@ using android::base::StringPrintf; namespace android { +/** + * Determines if the identifiers passed are a sub-devices. Sub-devices are physical devices + * that expose multiple input device paths such a keyboard that also has a touchpad input. + * These are separate devices with unique descriptors in EventHub, but InputReader should + * create a single InputDevice for them. + * Sub-devices are detected by the following criteria: + * 1. The vendor, product, bus, version, and unique id match + * 2. The location matches. The location is used to distinguish a single device with multiple + * inputs versus the same device plugged into multiple ports. + */ + +static bool isSubDevice(const InputDeviceIdentifier& identifier1, + const InputDeviceIdentifier& identifier2) { + return (identifier1.vendor == identifier2.vendor && + identifier1.product == identifier2.product && identifier1.bus == identifier2.bus && + identifier1.version == identifier2.version && + identifier1.uniqueId == identifier2.uniqueId && + identifier1.location == identifier2.location); +} + // --- InputReader --- InputReader::InputReader(std::shared_ptr<EventHubInterface> eventHub, @@ -277,8 +297,9 @@ void InputReader::removeDeviceLocked(nsecs_t when, int32_t eventHubId) { std::shared_ptr<InputDevice> InputReader::createDeviceLocked( int32_t eventHubId, const InputDeviceIdentifier& identifier) { auto deviceIt = std::find_if(mDevices.begin(), mDevices.end(), [identifier](auto& devicePair) { - return devicePair.second->getDescriptor().size() && identifier.descriptor.size() && - devicePair.second->getDescriptor() == identifier.descriptor; + const InputDeviceIdentifier identifier2 = + devicePair.second->getDeviceInfo().getIdentifier(); + return isSubDevice(identifier, identifier2); }); std::shared_ptr<InputDevice> device; diff --git a/services/inputflinger/reader/include/EventHub.h b/services/inputflinger/reader/include/EventHub.h index 1016a2c409..1a39a5cc4d 100644 --- a/services/inputflinger/reader/include/EventHub.h +++ b/services/inputflinger/reader/include/EventHub.h @@ -643,7 +643,6 @@ private: void scanDevicesLocked() REQUIRES(mLock); status_t readNotifyLocked() REQUIRES(mLock); - Device* getDeviceByDescriptorLocked(const std::string& descriptor) const REQUIRES(mLock); Device* getDeviceLocked(int32_t deviceId) const REQUIRES(mLock); Device* getDeviceByPathLocked(const std::string& devicePath) const REQUIRES(mLock); /** @@ -653,6 +652,9 @@ private: Device* getDeviceByFdLocked(int fd) const REQUIRES(mLock); int32_t getNextControllerNumberLocked(const std::string& name) REQUIRES(mLock); + + bool hasDeviceWithDescriptorLocked(const std::string& descriptor) const REQUIRES(mLock); + void releaseControllerNumberLocked(int32_t num) REQUIRES(mLock); void reportDeviceAddedForStatisticsLocked(const InputDeviceIdentifier& identifier, Flags<InputDeviceClass> classes) REQUIRES(mLock); diff --git a/services/inputflinger/tests/EventHub_test.cpp b/services/inputflinger/tests/EventHub_test.cpp index ef68a84fdb..6ef6e4498c 100644 --- a/services/inputflinger/tests/EventHub_test.cpp +++ b/services/inputflinger/tests/EventHub_test.cpp @@ -180,6 +180,20 @@ void EventHubTest::assertNoMoreEvents() { } /** + * Ensure that two identical devices get assigned unique descriptors from EventHub. + */ +TEST_F(EventHubTest, DevicesWithMatchingUniqueIdsAreUnique) { + std::unique_ptr<UinputHomeKey> keyboard2 = createUinputDevice<UinputHomeKey>(); + int32_t deviceId2; + ASSERT_NO_FATAL_FAILURE(deviceId2 = waitForDeviceCreation()); + + ASSERT_NE(mEventHub->getDeviceIdentifier(mDeviceId).descriptor, + mEventHub->getDeviceIdentifier(deviceId2).descriptor); + keyboard2.reset(); + waitForDeviceClose(deviceId2); +} + +/** * Ensure that input_events are generated with monotonic clock. * That means input_event should receive a timestamp that is in the future of the time * before the event was sent. |