Crash [@InvalidateAllFrames]

RESOLVED FIXED in Firefox 54

Status

()

P2
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jkratzer, Assigned: jdai)

Tracking

(Blocks: 1 bug, {crash, csectype-nullptr, testcase})

43 Branch
mozilla55
crash, csectype-nullptr, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 wontfix, firefox-esr52 wontfix, firefox53 wontfix, firefox54 fixed, firefox55 fixed)

Details

(crash signature)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8851648 [details]
Testcase

Testcase found while fuzzing mozilla-central rev 20170326-cc53710589fb.

ASAN:DEADLYSIGNAL
=================================================================
==5156==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7f364628bc2e bp 0x7ffdfa6ab030 sp 0x7ffdfa6ab000 T0)
==5156==The signal is caused by a READ memory access.
==5156==Hint: address points to the zero page.
    #0 0x7f364628bc2d in get /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:284:27
    #1 0x7f364628bc2d in operator-> /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:316
    #2 0x7f364628bc2d in NodeType /home/worker/workspace/build/src/dom/base/nsINode.h:585
    #3 0x7f364628bc2d in InvalidateAllFrames /home/worker/workspace/build/src/dom/base/nsRange.cpp:61
    #4 0x7f364628bc2d in nsRange::AutoInvalidateSelection::~AutoInvalidateSelection() /home/worker/workspace/build/src/dom/base/nsRange.cpp:3275
    #5 0x7f3646279c35 in nsRange::SelectNode(nsINode&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsRange.cpp:1521:1
    #6 0x7f3646279e0f in nsRange::SelectNodeJS(nsINode&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsRange.cpp:1494:3
    #7 0x7f3646adace6 in mozilla::dom::RangeBinding::selectNode(JSContext*, JS::Handle<JSObject*>, nsRange*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/RangeBinding.cpp:775:9
    #8 0x7f36479fbcbe in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2953:13
    #9 0x7f364d1ccdd3 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #10 0x7f364d1ccdd3 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:454
    #11 0x7f364d1b5618 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:505:12
    #12 0x7f364d1b5618 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2998
    #13 0x7f364d19b57e in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #14 0x7f364d1cf2e7 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:683:15
    #15 0x7f364d1cfb52 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:715:12
Flags: in-testsuite?
Do you know if this is a recent regression?
Flags: needinfo?(jkratzer)
Priority: -- → P2
(Reporter)

Comment 2

2 years ago
Unfortunately I don't have an automated way of determining when this bug was introduced.  This is something we're working towards but likely won't have an answer for you any time soon.
Flags: needinfo?(jkratzer)
(Assignee)

Comment 3

2 years ago
(In reply to Andrew Overholt [:overholt] from comment #1)
> Do you know if this is a recent regression?

It seems not a recent regression, it shows 2015-09-14 on the mozregression in Firefox 43.

Regression window:
 9:50.85 INFO: Narrowed nightly regression window from [2015-09-07, 2015-09-20] (13 days) to [2015-09-07, 2015-09-14] (7 days) (~2 steps left)
 9:50.85 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=df817a53e24bbacd62347cb9873d86039448b666&tochange=9ed17db42e3e46f1c712e4dffd62d54e915e0fac


Regressed by: Bug 571294
Mystor, could you take a look? Probably not an issue in your patch but just old missing null check or something.
Flags: needinfo?(michael)
(Assignee)

Comment 5

2 years ago
At my first glance, aNode[1] is NULL when it calls InvalidateAllFrames.

[1] http://searchfox.org/mozilla-central/source/dom/base/nsRange.cpp#61
(Assignee)

Updated

2 years ago
Assignee: nobody → jdai
Status: NEW → ASSIGNED
(Assignee)

Comment 6

2 years ago
Created attachment 8856456 [details] [diff] [review]
patch, v1

It might returns NULL when AutoInvalidateSelection dtor doesn't have commonAncestor[1][2]. Add a null check to guard it.

[1] https://searchfox.org/mozilla-central/source/dom/base/nsRange.cpp#3273
[2] https://searchfox.org/mozilla-central/source/dom/base/nsRange.cpp#148

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b55850033e3e9be75a70b90edd3d09e7f443cd66&group_state=expanded&filter-tier=1
Flags: needinfo?(michael)
Attachment #8856456 - Flags: review?(bugs)
(Assignee)

Comment 7

2 years ago
Created attachment 8856838 [details] [diff] [review]
patch, v2

- Add crashtest asserts expect numbers.
- Carry r+.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd556d161d95b591f806da146726a283e359d4bd&filter-tier=1&group_state=expanded
Attachment #8856456 - Attachment is obsolete: true
Attachment #8856838 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4117dddd0a72
Avoid crash when AutoInvalidateSelection dtor doesn't have commonAncestor. r=smaug
Keywords: checkin-needed

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4117dddd0a72
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Aurora approval on this when you get a chance.
Blocks: 571294
Crash Signature: [@ InvalidateAllFrames ]
status-firefox52: --- → wontfix
status-firefox53: --- → wontfix
status-firefox54: --- → affected
status-firefox-esr52: --- → wontfix
Flags: needinfo?(jdai)
Flags: in-testsuite?
Flags: in-testsuite+
Version: unspecified → 43 Branch
(Assignee)

Comment 11

2 years ago
Created attachment 8857749 [details] [diff] [review]
(Aurora) patch, final

Approval Request Comment
[Feature/Bug causing the regression]: Range and selection
[User impact if declined]: user use Range.selectNode() can crash
[Is this code covered by automated tests?]: Yes, it's located at dom/html/crashtests/1350972.html 
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low
[Why is the change risky/not risky?]: This change mainly added a null check, it can make our code more robust.
[String changes made/needed]: none
Flags: needinfo?(jdai)
Attachment #8857749 - Flags: review+
Attachment #8857749 - Flags: approval-mozilla-aurora?
Comment on attachment 8857749 [details] [diff] [review]
(Aurora) patch, final

Fix a crash. Aurora54+.
Attachment #8857749 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 13

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/87db9cca835a
status-firefox54: affected → fixed
You need to log in before you can comment on or make changes to this bug.