Do not call 'setEnabled' before mapper is configured

The mappers have a specific lifecycle:
1. constructor
2. configure(0)
3. reset
4. use it

However, currently, this could be broken because the 'reset' function is
getting invoked before the first configure(0).

If a mapper's 'configure(0)' method isn't called, then there will be
uninitialized variables inside.
Specifically, in TouchInputMapper, this will mean that:
a. mPointerUsage may be set to something like "STYLUS".
b. mPointerSimple::down or mPointerSimple::hovering may be set to true

The above combination could cause a crash, because it would try to
access mPointerController, which isn't yet initialized.

This is a speculative fix, because we can't reproduce the crash, since
it relies on a specific state of the uninitialized variables.

Ideally, we would simply eliminate these possibilities by either using
the constructor (and calling "configure" there), or providing some
default values.

To keep the fix simple, in this CL we just avoid calling 'setEnabled'
too early.

Bug: 255739891
Bug: 255839467
Test: atest inputflinger_tests
Change-Id: I44038c5ce5bfdd5ac4c2933e0dc4fa714c5cf260

2 files changed, 16 insertions, 2 deletions

diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp
index 5291776638..e6ab8722f6 100644
--- a/services/inputflinger/reader/InputDevice.cpp
+++ b/services/inputflinger/reader/InputDevice.cpp
@@ -313,7 +313,10 @@ std::list<NotifyArgs> InputDevice::configure(nsecs_t when, const InputReaderConf
             }
         }
 
-        if (!changes || (changes & InputReaderConfiguration::CHANGE_ENABLED_STATE)) {
+        if (changes & InputReaderConfiguration::CHANGE_ENABLED_STATE) {
+            // Do not execute this code on the first configure, because 'setEnabled' would call
+            // InputMapper::reset, and you can't reset a mapper before it has been configured.
+            // The mappers are configured for the first time at the bottom of this function.
             auto it = config->disabledDevices.find(mId);
             bool enabled = it == config->disabledDevices.end();
             out += setEnabled(enabled, when);
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index bc70584af8..a8b0fe9c9a 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -39,6 +39,7 @@
 #include <gtest/gtest.h>
 #include <gui/constants.h>
 
+#include <thread>
 #include "android/hardware/input/InputDeviceCountryCode.h"
 #include "input/DisplayViewport.h"
 #include "input/Input.h"
@@ -141,6 +142,16 @@ static void assertAxisNotPresent(MultiTouchInputMapper& mapper, int axis) {
     ASSERT_EQ(nullptr, motionRange);
 }
 
+[[maybe_unused]] static void dumpReader(InputReader& reader) {
+    std::string dump;
+    reader.dump(dump);
+    std::istringstream iss(dump);
+    for (std::string line; std::getline(iss, line);) {
+        ALOGE("%s", line.c_str());
+        std::this_thread::sleep_for(std::chrono::milliseconds(1));
+    }
+}
+
 // --- FakePointerController ---
 
 class FakePointerController : public PointerControllerInterface {
@@ -740,7 +751,7 @@ private:
     status_t getAbsoluteAxisInfo(int32_t deviceId, int axis,
                                  RawAbsoluteAxisInfo* outAxisInfo) const override {
         Device* device = getDevice(deviceId);
-        if (device && device->enabled) {
+        if (device) {
             ssize_t index = device->absoluteAxes.indexOfKey(axis);
             if (index >= 0) {
                 *outAxisInfo = device->absoluteAxes.valueAt(index);