Closed Bug 1298102 Opened 3 years ago Closed 3 years ago

Invalid array access in InputQueue::CurrentBlock()

Categories

(Core :: Panning and Zooming, defect, P2, critical)

51 Branch
Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox50 --- unaffected
firefox51 --- fixed

People

(Reporter: mccr8, Unassigned)

References

Details

(Keywords: crash, csectype-bounds, Whiteboard: [nightly only][gfx-noted])

Crash Data

This bug was filed from the Socorro interface and is 
report bp-c6f32959-3a4f-496a-9217-35cc52160825.
=============================================================

This code looks like this:
  MOZ_ASSERT(!mInputBlockQueue.IsEmpty());
  return mInputBlockQueue[0].get();

It appears that at least sometimes mInputBlockQueue is empty, at least when called from OnLongPress().

Here's another similar crash report:
  bp-4b72a77f-f93d-4e12-ac24-b584c2160824
Oh, interesting. On Windows this code doesn't run unless touch events are enabled, and that's #ifdef NIGHTLY_BUILD, so this should only affect Nightly for now.

That being said I'm not entirely sure how this can happen. If the long-tap timeout is firing, that means the GestureEventListener must be in state GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN, which in turn means that neither HandleInputTouchEnd() nor HandleInputTouchCancel() has been called... and that means there should still be an active input block.

Best guess for now is that this is caused somehow by interleaving different input types (maybe a mouse click during a longpress, which interrupts and throws away the touch block, and then the mouse block ends too?). I'd have to think about it more.
Blocks: 1244402
Whiteboard: [nightly only]
Version: Trunk → 51 Branch
Thanks for taking a look. We have bounds checks in release builds, and this is nightly only, so I'll just unhide it.
Group: core-security
Keywords: csectype-bounds
Whiteboard: [nightly only] → [nightly only][gfx-noted]
I think there are cases where this can happen with interleaved input events. I ran into a crash that looked like this one while running the APZCGestureDetectorTester.LongPressInterruptedByWheel gtest with some local changes applied. The local changes were shifting things around but basically I think if we get a wheel event in the middle of a long-press, it can clear out the touch block object before the long-tap timeout fires.

The patches I'm working on in bug 1289432 should fix it.
Depends on: 1289432
Is there a way to search crash-stats for this particular crash (coming from InputBlock) amongst all the crashes that match the InvalidArrayIndex_CRASH signature? In expecting this to be fixed by bug 1289432 but would like to verify on crash-stats. I can't see a way to search by the second stack frame though.
Flags: needinfo?(continuation)
I added InvalidArrayIndex_CRASH to the skip list, so the signature will now contain both InvalidArrayIndex_CRASH and the next frame. You can do a super search, add "signature" "contains" "InvalidArrayIndex_CRASH", and then look at the facet on signature. (If it wasn't in the skip list, you could do the same thing except with "proto signature" instead of "signature".) I only see something with the expiration tracker and some IMM thing. (which both have other bugs on file.)
Flags: needinfo?(continuation)
Thanks.

If I'm doing it right, I can only find a grand total of 5 crash reports that match this bug, listed below. The first two are presumably from before you added InvalidArrayIndex_CRASH to the skip list and the last three are from after, because they show up with different signatures.

4b72a77f-f93d-4e12-ac24-b584c2160824
c6f32959-3a4f-496a-9217-35cc52160825 (the one in comment 0)
43c82ff1-f4c3-437a-b1ab-6cfcf2160908
c67ef3ba-5d11-4dbb-be00-6ecaf2160911
320dfd07-60c9-4393-96bc-dcedc2160912

I don't think the frequency here is sufficiently high to confirm that my patch fixed it, so I'll close this bug for now and reopen it if it reappears. I'm updating the Crash Signature field so that new crashes that show up with this signature get linked to this bug.
Status: NEW → RESOLVED
Crash Signature: [@ InvalidArrayIndex_CRASH] → [@ InvalidArrayIndex_CRASH] [@ InvalidArrayIndex_CRASH | mozilla::layers::InputQueue::CurrentBlock]
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.