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

Commit 3218fc08 authored by Siarhei Vishniakou's avatar Siarhei Vishniakou
Browse files

Store coords and properties as vector in args 

Before this CL, NotifyMotionArgs stored PointerCoords and
PointerProperties in a fixed-size array. Upon creation of a new object,
some of that data typically remained uninitialized.

At the same time, the copy assignment operator was defaulted, which
meant that the uninitialized data was getting accessed in order to copy
the object.

The sanitizers identify this as a problem and crash.

To fix this, store these objects inside vectors.

Bug: 271455682
Test: atest inputflinger_tests
Change-Id: I9dba29f75df59a21f8ed7fd0f46fd1f6d45f2eef
parent c16c3d24

services/inputflinger/InputCommonConverter.cpp

+3 −3
Original line number Diff line number Diff line
@@ -289,9 +289,9 @@ static std::vector<common::VideoFrame> convertVideoFrames( 
static void getHalPropertiesAndCoords(const NotifyMotionArgs& args,

                                      std::vector<common::PointerProperties>& outPointerProperties,

                                      std::vector<common::PointerCoords>& outPointerCoords) {

    outPointerProperties.reserve(args.pointerCount);

    outPointerCoords.reserve(args.pointerCount);

    for (size_t i = 0; i < args.pointerCount; i++) {

    outPointerProperties.reserve(args.getPointerCount());

    outPointerCoords.reserve(args.getPointerCount());

    for (size_t i = 0; i < args.getPointerCount(); i++) {

        common::PointerProperties properties;

        properties.id = args.pointerProperties[i].id;

        properties.toolType = getToolType(args.pointerProperties[i].toolType);

services/inputflinger/InputDeviceMetricsCollector.cpp

+2 −2
Original line number Diff line number Diff line
@@ -128,10 +128,10 @@ InputDeviceUsageSource getUsageSourceForKeyArgs(const InputDeviceInfo& info, 
}



std::set<InputDeviceUsageSource> getUsageSourcesForMotionArgs(const NotifyMotionArgs& motionArgs) {

    LOG_ALWAYS_FATAL_IF(motionArgs.pointerCount < 1, "Received motion args without pointers");

    LOG_ALWAYS_FATAL_IF(motionArgs.getPointerCount() < 1, "Received motion args without pointers");

    std::set<InputDeviceUsageSource> sources;



    for (uint32_t i = 0; i < motionArgs.pointerCount; i++) {

    for (uint32_t i = 0; i < motionArgs.getPointerCount(); i++) {

        const auto toolType = motionArgs.pointerProperties[i].toolType;

        if (isFromSource(motionArgs.source, AINPUT_SOURCE_MOUSE)) {

            if (toolType == ToolType::MOUSE) {

services/inputflinger/NotifyArgs.cpp

+10 −53
Original line number Diff line number Diff line
@@ -83,7 +83,6 @@ NotifyMotionArgs::NotifyMotionArgs( 
        buttonState(buttonState),

        classification(classification),

        edgeFlags(edgeFlags),

        pointerCount(pointerCount),

        xPrecision(xPrecision),

        yPrecision(yPrecision),

        xCursorPosition(xCursorPosition),
@@ -92,36 +91,8 @@ NotifyMotionArgs::NotifyMotionArgs( 
        readTime(readTime),

        videoFrames(videoFrames) {

    for (uint32_t i = 0; i < pointerCount; i++) {

        this->pointerProperties[i].copyFrom(pointerProperties[i]);

        this->pointerCoords[i].copyFrom(pointerCoords[i]);

    }

}



NotifyMotionArgs::NotifyMotionArgs(const NotifyMotionArgs& other)

      : id(other.id),

        eventTime(other.eventTime),

        deviceId(other.deviceId),

        source(other.source),

        displayId(other.displayId),

        policyFlags(other.policyFlags),

        action(other.action),

        actionButton(other.actionButton),

        flags(other.flags),

        metaState(other.metaState),

        buttonState(other.buttonState),

        classification(other.classification),

        edgeFlags(other.edgeFlags),

        pointerCount(other.pointerCount),

        xPrecision(other.xPrecision),

        yPrecision(other.yPrecision),

        xCursorPosition(other.xCursorPosition),

        yCursorPosition(other.yCursorPosition),

        downTime(other.downTime),

        readTime(other.readTime),

        videoFrames(other.videoFrames) {

    for (uint32_t i = 0; i < pointerCount; i++) {

        pointerProperties[i].copyFrom(other.pointerProperties[i]);

        pointerCoords[i].copyFrom(other.pointerCoords[i]);

        this->pointerProperties.push_back(pointerProperties[i]);

        this->pointerCoords.push_back(pointerCoords[i]);

    }

}
@@ -130,35 +101,22 @@ static inline bool isCursorPositionEqual(float lhs, float rhs) { 
}



bool NotifyMotionArgs::operator==(const NotifyMotionArgs& rhs) const {

    bool equal = id == rhs.id && eventTime == rhs.eventTime && readTime == rhs.readTime &&

    return id == rhs.id && eventTime == rhs.eventTime && readTime == rhs.readTime &&

            deviceId == rhs.deviceId && source == rhs.source && displayId == rhs.displayId &&

            policyFlags == rhs.policyFlags && action == rhs.action &&

            actionButton == rhs.actionButton && flags == rhs.flags && metaState == rhs.metaState &&

            buttonState == rhs.buttonState && classification == rhs.classification &&

            edgeFlags == rhs.edgeFlags &&

            pointerCount == rhs.pointerCount

            // PointerProperties and PointerCoords are compared separately below

            && xPrecision == rhs.xPrecision && yPrecision == rhs.yPrecision &&

            edgeFlags == rhs.edgeFlags && pointerProperties == rhs.pointerProperties &&

            pointerCoords == rhs.pointerCoords && xPrecision == rhs.xPrecision &&

            yPrecision == rhs.yPrecision &&

            isCursorPositionEqual(xCursorPosition, rhs.xCursorPosition) &&

            isCursorPositionEqual(yCursorPosition, rhs.yCursorPosition) &&

            downTime == rhs.downTime && videoFrames == rhs.videoFrames;

    if (!equal) {

        return false;

    }



    for (size_t i = 0; i < pointerCount; i++) {

        equal = pointerProperties[i] == rhs.pointerProperties[i] &&

                pointerCoords[i] == rhs.pointerCoords[i];

        if (!equal) {

            return false;

        }

    }

    return true;

}



std::string NotifyMotionArgs::dump() const {

    std::string coords;

    for (uint32_t i = 0; i < pointerCount; i++) {

    for (uint32_t i = 0; i < getPointerCount(); i++) {

        if (!coords.empty()) {

            coords += ", ";

        }
@@ -181,11 +139,10 @@ std::string NotifyMotionArgs::dump() const { 
        coords += "}";

    }

    return StringPrintf("NotifyMotionArgs(id=%" PRId32 ", eventTime=%" PRId64 ", deviceId=%" PRId32

                        ", source=%s, action=%s, pointerCount=%" PRIu32

                        " pointers=%s, flags=0x%08x)",

                        ", source=%s, action=%s, pointerCount=%zu pointers=%s, flags=0x%08x)",

                        id, eventTime, deviceId, inputEventSourceToString(source).c_str(),

                        MotionEvent::actionToString(action).c_str(), pointerCount, coords.c_str(),

                        flags);

                        MotionEvent::actionToString(action).c_str(), getPointerCount(),

                        coords.c_str(), flags);

}



// --- NotifySwitchArgs ---

services/inputflinger/PreferStylusOverTouchBlocker.cpp

+1 −1
Original line number Diff line number Diff line
@@ -22,7 +22,7 @@ namespace android { 
static std::pair<bool, bool> checkToolType(const NotifyMotionArgs& args) {

    bool hasStylus = false;

    bool hasTouch = false;

    for (size_t i = 0; i < args.pointerCount; i++) {

    for (size_t i = 0; i < args.getPointerCount(); i++) {

        // Make sure we are canceling stylus pointers

        const ToolType toolType = args.pointerProperties[i].toolType;

        if (isStylusToolType(toolType)) {

services/inputflinger/UnwantedInteractionBlocker.cpp

+18 −17
Original line number Diff line number Diff line
@@ -117,7 +117,7 @@ static int getLinuxToolCode(ToolType toolType) { 
}



static int32_t getActionUpForPointerId(const NotifyMotionArgs& args, int32_t pointerId) {

    for (size_t i = 0; i < args.pointerCount; i++) {

    for (size_t i = 0; i < args.getPointerCount(); i++) {

        if (pointerId == args.pointerProperties[i].id) {

            return AMOTION_EVENT_ACTION_POINTER_UP |

                    (i << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT);
@@ -156,9 +156,10 @@ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, 
            actionMasked == AMOTION_EVENT_ACTION_POINTER_UP;



    NotifyMotionArgs newArgs{args};

    newArgs.pointerCount = 0;

    newArgs.pointerProperties.clear();

    newArgs.pointerCoords.clear();

    int32_t newActionIndex = 0;

    for (uint32_t i = 0; i < args.pointerCount; i++) {

    for (uint32_t i = 0; i < args.getPointerCount(); i++) {

        const int32_t pointerId = args.pointerProperties[i].id;

        if (pointerIds.find(pointerId) != pointerIds.end()) {

            // skip this pointer
@@ -170,19 +171,18 @@ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, 
            }

            continue;

        }

        newArgs.pointerProperties[newArgs.pointerCount].copyFrom(args.pointerProperties[i]);

        newArgs.pointerCoords[newArgs.pointerCount].copyFrom(args.pointerCoords[i]);

        newArgs.pointerProperties.push_back(args.pointerProperties[i]);

        newArgs.pointerCoords.push_back(args.pointerCoords[i]);

        if (i == actionIndex) {

            newActionIndex = newArgs.pointerCount;

            newActionIndex = newArgs.getPointerCount() - 1;

        }

        newArgs.pointerCount++;

    }

    // Update POINTER_DOWN or POINTER_UP actions

    if (isPointerUpOrDownAction && newArgs.action != ACTION_UNKNOWN) {

        newArgs.action =

                actionMasked | (newActionIndex << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT);

        // Convert POINTER_DOWN and POINTER_UP to DOWN and UP if there's only 1 pointer remaining

        if (newArgs.pointerCount == 1) {

        if (newArgs.getPointerCount() == 1) {

            if (actionMasked == AMOTION_EVENT_ACTION_POINTER_DOWN) {

                newArgs.action = AMOTION_EVENT_ACTION_DOWN;

            } else if (actionMasked == AMOTION_EVENT_ACTION_POINTER_UP) {
@@ -201,13 +201,14 @@ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, 
 */

static std::optional<NotifyMotionArgs> removeStylusPointerIds(const NotifyMotionArgs& args) {

    std::set<int32_t> stylusPointerIds;

    for (uint32_t i = 0; i < args.pointerCount; i++) {

    for (uint32_t i = 0; i < args.getPointerCount(); i++) {

        if (isStylusToolType(args.pointerProperties[i].toolType)) {

            stylusPointerIds.insert(args.pointerProperties[i].id);

        }

    }

    NotifyMotionArgs withoutStylusPointers = removePointerIds(args, stylusPointerIds);

    if (withoutStylusPointers.pointerCount == 0 || withoutStylusPointers.action == ACTION_UNKNOWN) {

    if (withoutStylusPointers.getPointerCount() == 0 ||

        withoutStylusPointers.action == ACTION_UNKNOWN) {

        return std::nullopt;

    }

    return withoutStylusPointers;
@@ -272,7 +273,7 @@ std::optional<AndroidPalmFilterDeviceInfo> createPalmFilterDeviceInfo( 
std::vector<NotifyMotionArgs> cancelSuppressedPointers(

        const NotifyMotionArgs& args, const std::set<int32_t>& oldSuppressedPointerIds,

        const std::set<int32_t>& newSuppressedPointerIds) {

    LOG_ALWAYS_FATAL_IF(args.pointerCount == 0, "0 pointers in %s", args.dump().c_str());

    LOG_ALWAYS_FATAL_IF(args.getPointerCount() == 0, "0 pointers in %s", args.dump().c_str());



    // First, let's remove the old suppressed pointers. They've already been canceled previously.

    NotifyMotionArgs oldArgs = removePointerIds(args, oldSuppressedPointerIds);
@@ -284,7 +285,7 @@ std::vector<NotifyMotionArgs> cancelSuppressedPointers( 
    const int32_t actionMasked = MotionEvent::getActionMasked(args.action);

    // We will iteratively remove pointers from 'removedArgs'.

    NotifyMotionArgs removedArgs{oldArgs};

    for (uint32_t i = 0; i < oldArgs.pointerCount; i++) {

    for (uint32_t i = 0; i < oldArgs.getPointerCount(); i++) {

        const int32_t pointerId = oldArgs.pointerProperties[i].id;

        if (newSuppressedPointerIds.find(pointerId) == newSuppressedPointerIds.end()) {

            // This is a pointer that should not be canceled. Move on.
@@ -296,7 +297,7 @@ std::vector<NotifyMotionArgs> cancelSuppressedPointers( 
            continue;

        }



        if (removedArgs.pointerCount == 1) {

        if (removedArgs.getPointerCount() == 1) {

            // We are about to remove the last pointer, which means there will be no more gesture

            // remaining. This is identical to canceling all pointers, so just send a single CANCEL

            // event, without any of the preceding POINTER_UP with FLAG_CANCELED events.
@@ -314,7 +315,7 @@ std::vector<NotifyMotionArgs> cancelSuppressedPointers( 
    }



    // Now 'removedArgs' contains only pointers that are valid.

    if (removedArgs.pointerCount <= 0 || removedArgs.action == ACTION_UNKNOWN) {

    if (removedArgs.getPointerCount() <= 0 || removedArgs.action == ACTION_UNKNOWN) {

        return out;

    }

    out.push_back(removedArgs);
@@ -473,7 +474,7 @@ void UnwantedInteractionBlocker::monitor() { 
UnwantedInteractionBlocker::~UnwantedInteractionBlocker() {}



void SlotState::update(const NotifyMotionArgs& args) {

    for (size_t i = 0; i < args.pointerCount; i++) {

    for (size_t i = 0; i < args.getPointerCount(); i++) {

        const int32_t pointerId = args.pointerProperties[i].id;

        const int32_t resolvedAction = resolveActionForPointer(i, args.action);

        processPointerId(pointerId, resolvedAction);
@@ -571,7 +572,7 @@ std::vector<::ui::InProgressTouchEvdev> getTouches(const NotifyMotionArgs& args, 
                                                   const SlotState& newSlotState) {

    std::vector<::ui::InProgressTouchEvdev> touches;



    for (size_t i = 0; i < args.pointerCount; i++) {

    for (size_t i = 0; i < args.getPointerCount(); i++) {

        const int32_t pointerId = args.pointerProperties[i].id;

        touches.emplace_back(::ui::InProgressTouchEvdev());

        touches.back().major = args.pointerCoords[i].getAxisValue(AMOTION_EVENT_AXIS_TOUCH_MAJOR);
@@ -660,7 +661,7 @@ std::set<int32_t> PalmRejector::detectPalmPointers(const NotifyMotionArgs& args) 


    // Now that we know which slots should be suppressed, let's convert those to pointer id's.

    std::set<int32_t> newSuppressedIds;

    for (size_t i = 0; i < args.pointerCount; i++) {

    for (size_t i = 0; i < args.getPointerCount(); i++) {

        const int32_t pointerId = args.pointerProperties[i].id;

        std::optional<size_t> slot = oldSlotState.getSlotForPointerId(pointerId);

        if (!slot) {