Prevent duplicate devices from getting registered
In EventHub, we have the following race currently present:
1. Phone is booted
2. EventHub is created. First, mNeedToScanDevices is set to true in the initializer list.
Then, in constructor body, inotify gets set up.
3. Around the same time, the touch driver is probing.
Based on the logs, steps 2. and 3. happen within a millisecond of each other.
4. Since inotify is already registered, we are getting an inotify callback in EventHub.
5. EventHub::getEvents is executed.
scanDevicesLocked happens first and finds the /dev/input/event3 node that is now present.
6. The new device gets added
7. The pending inotify is read. The device is added again.
So this is a race between scanDir and registration of inotify. If we register inotify
after scanDir, we run the risk of missing the device, if it happens to appear between those
two tasks. If we register before (as we do now), we run the risk of double-registering a device, which is surfaced by this bug.
In the inotify interface, there does not appear to be any api to get
callbacks for the nodes that are already present. As a result, we put a
simple patch to prevent this race.
Bug: 178139688
Test: manually verified by adding and reviewing the logs on device
Test: verified using a proof-of-concept test in patchset 4
Change-Id: I1bbe7574dfe84c79c112e538f14f5a0dcde98dcc
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp
index d0a9ca7..1a7fcd5 100644
--- a/services/inputflinger/reader/EventHub.cpp
+++ b/services/inputflinger/reader/EventHub.cpp
@@ -1771,7 +1771,17 @@
}
}
-status_t EventHub::openDeviceLocked(const std::string& devicePath) {
+void EventHub::openDeviceLocked(const std::string& devicePath) {
+ // If an input device happens to register around the time when EventHub's constructor runs, it
+ // is possible that the same input event node (for example, /dev/input/event3) will be noticed
+ // in both 'inotify' callback and also in the 'scanDirLocked' pass. To prevent duplicate devices
+ // from getting registered, ensure that this path is not already covered by an existing device.
+ for (const auto& [deviceId, device] : mDevices) {
+ if (device->path == devicePath) {
+ return; // device was already registered
+ }
+ }
+
char buffer[80];
ALOGV("Opening device: %s", devicePath.c_str());
@@ -1779,7 +1789,7 @@
int fd = open(devicePath.c_str(), O_RDWR | O_CLOEXEC | O_NONBLOCK);
if (fd < 0) {
ALOGE("could not open %s, %s\n", devicePath.c_str(), strerror(errno));
- return -1;
+ return;
}
InputDeviceIdentifier identifier;
@@ -1798,7 +1808,7 @@
if (identifier.name == item) {
ALOGI("ignoring event id %s driver %s\n", devicePath.c_str(), item.c_str());
close(fd);
- return -1;
+ return;
}
}
@@ -1807,7 +1817,7 @@
if (ioctl(fd, EVIOCGVERSION, &driverVersion)) {
ALOGE("could not get driver version for %s, %s\n", devicePath.c_str(), strerror(errno));
close(fd);
- return -1;
+ return;
}
// Get device identifier.
@@ -1815,7 +1825,7 @@
if (ioctl(fd, EVIOCGID, &inputId)) {
ALOGE("could not get device input id for %s, %s\n", devicePath.c_str(), strerror(errno));
close(fd);
- return -1;
+ return;
}
identifier.bus = inputId.bustype;
identifier.product = inputId.product;
@@ -2013,7 +2023,7 @@
if (device->classes == Flags<InputDeviceClass>(0)) {
ALOGV("Dropping device: id=%d, path='%s', name='%s'", deviceId, devicePath.c_str(),
device->identifier.name.c_str());
- return -1;
+ return;
}
// Classify InputDeviceClass::BATTERY.
@@ -2043,7 +2053,7 @@
}
if (registerDeviceForEpollLocked(*device) != OK) {
- return -1;
+ return;
}
device->configureFd();
@@ -2056,7 +2066,6 @@
toString(mBuiltInKeyboardId == deviceId));
addDeviceLocked(std::move(device));
- return OK;
}
void EventHub::openVideoDeviceLocked(const std::string& devicePath) {