Skip to content

Commit 26640c9

Browse files
committed
InputVerifier: verify button events and states (attempt 2)
Check the following things: * BUTTON_PRESS and _RELEASE actions have a single, valid action button * No redundant BUTTON_PRESS or _RELEASE actions (i.e. for buttons that are already pressed or released) * Button state remains consistent throughout the stream, i.e.: * Buttons are only added to the state by BUTTON_PRESS (though DOWN events can have "pending" buttons on them) * Buttons are only removed from the state by BUTTON_RELEASE * When a DOWN event has pending buttons in its state, it is immediately followed by a BUTTON_PRESS for each one We could also verify that press and release events for primary, secondary, and tertiary buttons are accompanied by down and up events. However, I couldn't find any documentation that states which buttons should result in down or up events, so I haven't implemented this for now. This is the second attempt to land this change, due to the original causing test failures. Change I5c259c13d433d3010d2cf9c6fe01e08ba5990ef7 fixes the failures. v2 also adds a separate flag for button state verification, as it is actually used in production to check events injected by assistive technologies, whether or not the flag that I previously thought was guarding it is enabled. Test: connect a mouse and a touchpad, enable the verifier, play around with the buttons, and check that any issues found by the verifier appear to be legitimate. (I found b/391298464 , and checked that the verifier caught a button problem with a partial fix to b/372571823 .) Test: atest --host libinput_rust_tests Test: atest --host frameworks/native/libs/input/tests/InputVerifier_test.cpp Test: atest --host \ frameworks/native/services/inputflinger/tests/InputDispatcher_test.cpp Bug: 392870542 Flag: com.android.input.flags.enable_button_state_verification Change-Id: I46f489b26df8785563e41e58135b6b5de4ff62a2
1 parent 03ec059 commit 26640c9

15 files changed

Lines changed: 856 additions & 53 deletions

File tree

include/android/input.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,7 @@ enum {
849849
* Refer to the documentation on the MotionEvent class for descriptions of each button.
850850
*/
851851
enum {
852+
// LINT.IfChange(AMOTION_EVENT_BUTTON)
852853
/** primary */
853854
AMOTION_EVENT_BUTTON_PRIMARY = 1 << 0,
854855
/** secondary */
@@ -861,6 +862,7 @@ enum {
861862
AMOTION_EVENT_BUTTON_FORWARD = 1 << 4,
862863
AMOTION_EVENT_BUTTON_STYLUS_PRIMARY = 1 << 5,
863864
AMOTION_EVENT_BUTTON_STYLUS_SECONDARY = 1 << 6,
865+
// LINT.ThenChange(/frameworks/native/libs/input/rust/input.rs)
864866
};
865867

866868
/**

include/input/InputVerifier.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,10 @@ class InputVerifier {
4747
InputVerifier(const std::string& name);
4848

4949
android::base::Result<void> processMovement(int32_t deviceId, int32_t source, int32_t action,
50-
uint32_t pointerCount,
50+
int32_t actionButton, uint32_t pointerCount,
5151
const PointerProperties* pointerProperties,
52-
const PointerCoords* pointerCoords, int32_t flags);
52+
const PointerCoords* pointerCoords, int32_t flags,
53+
int32_t buttonState);
5354

5455
void resetDevice(int32_t deviceId);
5556

libs/input/Android.bp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,13 @@ rust_bindgen {
133133
"--allowlist-var=AMOTION_EVENT_ACTION_POINTER_DOWN",
134134
"--allowlist-var=AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT",
135135
"--allowlist-var=AMOTION_EVENT_ACTION_UP",
136+
"--allowlist-var=AMOTION_EVENT_BUTTON_BACK",
137+
"--allowlist-var=AMOTION_EVENT_BUTTON_FORWARD",
138+
"--allowlist-var=AMOTION_EVENT_BUTTON_PRIMARY",
139+
"--allowlist-var=AMOTION_EVENT_BUTTON_SECONDARY",
140+
"--allowlist-var=AMOTION_EVENT_BUTTON_STYLUS_PRIMARY",
141+
"--allowlist-var=AMOTION_EVENT_BUTTON_STYLUS_SECONDARY",
142+
"--allowlist-var=AMOTION_EVENT_BUTTON_TERTIARY",
136143
"--allowlist-var=MAX_POINTER_ID",
137144
"--verbose",
138145
],

libs/input/InputTransport.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -651,9 +651,9 @@ status_t InputPublisher::publishMotionEvent(
651651
const status_t status = mChannel->sendMessage(&msg);
652652

653653
if (status == OK && verifyEvents()) {
654-
Result<void> result =
655-
mInputVerifier.processMovement(deviceId, source, action, pointerCount,
656-
pointerProperties, pointerCoords, flags);
654+
Result<void> result = mInputVerifier.processMovement(deviceId, source, action, actionButton,
655+
pointerCount, pointerProperties,
656+
pointerCoords, flags, buttonState);
657657
if (!result.ok()) {
658658
LOG(ERROR) << "Bad stream: " << result.error();
659659
return BAD_VALUE;

libs/input/InputVerifier.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#define LOG_TAG "InputVerifier"
1818

1919
#include <android-base/logging.h>
20+
#include <com_android_input_flags.h>
2021
#include <input/InputVerifier.h>
2122
#include "input_cxx_bridge.rs.h"
2223

@@ -26,25 +27,33 @@ using android::input::RustPointerProperties;
2627

2728
using DeviceId = int32_t;
2829

30+
namespace input_flags = com::android::input::flags;
31+
2932
namespace android {
3033

3134
// --- InputVerifier ---
3235

3336
InputVerifier::InputVerifier(const std::string& name)
34-
: mVerifier(android::input::verifier::create(rust::String::lossy(name))){};
37+
: mVerifier(
38+
android::input::verifier::create(rust::String::lossy(name),
39+
input_flags::enable_button_state_verification())) {
40+
}
3541

3642
Result<void> InputVerifier::processMovement(DeviceId deviceId, int32_t source, int32_t action,
37-
uint32_t pointerCount,
43+
int32_t actionButton, uint32_t pointerCount,
3844
const PointerProperties* pointerProperties,
39-
const PointerCoords* pointerCoords, int32_t flags) {
45+
const PointerCoords* pointerCoords, int32_t flags,
46+
int32_t buttonState) {
4047
std::vector<RustPointerProperties> rpp;
4148
for (size_t i = 0; i < pointerCount; i++) {
4249
rpp.emplace_back(RustPointerProperties{.id = pointerProperties[i].id});
4350
}
4451
rust::Slice<const RustPointerProperties> properties{rpp.data(), rpp.size()};
4552
rust::String errorMessage =
4653
android::input::verifier::process_movement(*mVerifier, deviceId, source, action,
47-
properties, static_cast<uint32_t>(flags));
54+
actionButton, properties,
55+
static_cast<uint32_t>(flags),
56+
static_cast<uint32_t>(buttonState));
4857
if (errorMessage.empty()) {
4958
return {};
5059
} else {

libs/input/input_flags.aconfig

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@ flag {
1515
bug: "271455682"
1616
}
1717

18+
flag {
19+
name: "enable_button_state_verification"
20+
namespace: "input"
21+
description: "Set to true to enable crashing whenever bad inbound events are going into InputDispatcher"
22+
bug: "392870542"
23+
metadata {
24+
purpose: PURPOSE_BUGFIX
25+
}
26+
}
27+
1828
flag {
1929
name: "remove_input_channel_from_windowstate"
2030
namespace: "input"

libs/input/rust/input.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ bitflags! {
101101

102102
/// A rust enum representation of a MotionEvent action.
103103
#[repr(u32)]
104+
#[derive(Eq, PartialEq)]
104105
pub enum MotionAction {
105106
/// ACTION_DOWN
106107
Down = input_bindgen::AMOTION_EVENT_ACTION_DOWN,
@@ -193,6 +194,27 @@ impl MotionAction {
193194
}
194195
}
195196

197+
bitflags! {
198+
/// MotionEvent buttons.
199+
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
200+
pub struct MotionButton: u32 {
201+
/// Primary button (e.g. the left mouse button)
202+
const Primary = input_bindgen::AMOTION_EVENT_BUTTON_PRIMARY;
203+
/// Secondary button (e.g. the right mouse button)
204+
const Secondary = input_bindgen::AMOTION_EVENT_BUTTON_SECONDARY;
205+
/// Tertiary button (e.g. the middle mouse button)
206+
const Tertiary = input_bindgen::AMOTION_EVENT_BUTTON_TERTIARY;
207+
/// Back button
208+
const Back = input_bindgen::AMOTION_EVENT_BUTTON_BACK;
209+
/// Forward button
210+
const Forward = input_bindgen::AMOTION_EVENT_BUTTON_FORWARD;
211+
/// Primary stylus button
212+
const StylusPrimary = input_bindgen::AMOTION_EVENT_BUTTON_STYLUS_PRIMARY;
213+
/// Secondary stylus button
214+
const StylusSecondary = input_bindgen::AMOTION_EVENT_BUTTON_STYLUS_SECONDARY;
215+
}
216+
}
217+
196218
bitflags! {
197219
/// MotionEvent flags.
198220
/// The source of truth for the flag definitions are the MotionEventFlag AIDL enum.

0 commit comments

Comments
 (0)