Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a4c502a3 authored by Siarhei Vishniakou's avatar Siarhei Vishniakou
Browse files

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
parent 79e26e17

services/inputflinger/reader/EventHub.cpp

+17 −8
Original line number Diff line number Diff line
@@ -1771,7 +1771,17 @@ void EventHub::unregisterVideoDeviceFromEpollLocked(const TouchVideoDevice& vide 
    }

}



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 @@ status_t EventHub::openDeviceLocked(const std::string& devicePath) { 
    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 @@ status_t EventHub::openDeviceLocked(const std::string& devicePath) { 
        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 @@ status_t EventHub::openDeviceLocked(const std::string& devicePath) { 
    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 @@ status_t EventHub::openDeviceLocked(const std::string& devicePath) { 
    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 @@ status_t EventHub::openDeviceLocked(const std::string& devicePath) { 
    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 @@ status_t EventHub::openDeviceLocked(const std::string& devicePath) { 
    }



    if (registerDeviceForEpollLocked(*device) != OK) {

        return -1;

        return;

    }



    device->configureFd();
@@ -2056,7 +2066,6 @@ status_t EventHub::openDeviceLocked(const std::string& devicePath) { 
          toString(mBuiltInKeyboardId == deviceId));



    addDeviceLocked(std::move(device));

    return OK;

}



void EventHub::openVideoDeviceLocked(const std::string& devicePath) {

services/inputflinger/reader/include/EventHub.h

+4 −1
Original line number Diff line number Diff line
@@ -567,7 +567,10 @@ private: 
        bool configureLightsLocked();

    };



    status_t openDeviceLocked(const std::string& devicePath);

    /**

     * Create a new device for the provided path.

     */

    void openDeviceLocked(const std::string& devicePath);

    void openVideoDeviceLocked(const std::string& devicePath);

    /**

     * Try to associate a video device with an input device. If the association succeeds,