From 21e96e646fb1022589df69e09a2ec8183cd0edfb Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Thu, 27 Oct 2022 10:23:37 -0700 Subject: 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 --- services/inputflinger/reader/InputDevice.cpp | 5 ++++- services/inputflinger/tests/InputReader_test.cpp | 13 ++++++++++++- 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 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 #include +#include #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); -- cgit v1.2.3-59-g8ed1b