[jsdbg2] Crash after resuming from breakpoint

RESOLVED FIXED in Firefox 37

Status

()

defect
--
critical
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: shu, Assigned: shu)

Tracking

({sec-moderate})

unspecified
mozilla39
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox36 wontfix, firefox37 fixed, firefox38 fixed, firefox39 fixed, firefox-esr31 unaffected, firefox-esr38 fixed, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 unaffected, b2g-v2.1S unaffected, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [adv-main37+][post-critsmash-triage])

Attachments

(1 attachment)

Assignee

Description

4 years ago
STR:

1) Turn off parallel parsing in about:config to work around existing stupid problems in setting breakpoints.

2) Go to http://playcanv.as/p/JtL2iqIH and wait for it to load.

3) Find game.js, set a bp on line 158 inside setFuel.

4) Reload, wait for pause, then resume.

Socorro shows crash inside EnterBaselineAtBranch, like other crashes recently. I'll look into this promptly.
Assignee

Updated

4 years ago
Assignee: nobody → shu
Severity: normal → critical
Keywords: sec-moderate
Assignee

Comment 1

4 years ago
Setting sec-moderate as this can only be triggered when using the frontend debugger.
Assignee

Comment 2

4 years ago
Jan, the explanation is in the bug. This is pretty bad, but luckily only
triggerable when debugging. It should also be backported post haste.
Attachment #8568878 - Flags: review?(jdemooij)
Comment on attachment 8568878 [details] [diff] [review]
Ensure OSR frame scripts have debug instrumentation.

Review of attachment 8568878 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, great catch!

It will be interesting to watch crash-stats after this lands.
Attachment #8568878 - Flags: review?(jdemooij) → review+
Assignee

Updated

4 years ago
Attachment #8568878 - Flags: sec-approval?
Comment on attachment 8568878 [details] [diff] [review]
Ensure OSR frame scripts have debug instrumentation.

As a sec-moderate, this can just go in. Sec-approval is only needed for sec-high and sec-critical issues.
Attachment #8568878 - Flags: sec-approval?
Assignee

Comment 5

4 years ago
Comment on attachment 8568878 [details] [diff] [review]
Ensure OSR frame scripts have debug instrumentation.

Approval Request Comment
[Feature/regressing bug #]: 1032869
[User impact if declined]: Crashing when using the Debugger
[Describe test coverage new/current, TreeHerder]: on inbound
[Risks and why]: Low? Fixes a frequent crasher and security bug, but the patch paints a pretty clear picture of how to exploit the bug, though the exploit requires explicit user interaction with the frontend JS debugger.
[String/UUID change made/needed]: None
Attachment #8568878 - Flags: approval-mozilla-beta?
Attachment #8568878 - Flags: approval-mozilla-aurora?
As Al said, as a sec-moderate bug, we can check this in on m-c. Let's verify the fix there before uplifting to Aurora and Beta.
Assignee

Comment 7

4 years ago
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #6)
> As Al said, as a sec-moderate bug, we can check this in on m-c. Let's verify
> the fix there before uplifting to Aurora and Beta.

How do we verify the fix? The biggest tell is if crashes on Socorro go down, and most of those crashes came from Aurora, thus the uplifts.
(In reply to Shu-yu Guo [:shu] from comment #7)
> How do we verify the fix? The biggest tell is if crashes on Socorro go down,
> and most of those crashes came from Aurora, thus the uplifts.

Typically we look to see if the signature is present on Nightly before the change is committed and, after a few days, see if any new reports with the signature are present on Nightly.
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #9)
> Typically we look to see if the signature is present on Nightly before the
> change is committed and, after a few days, see if any new reports with the
> signature are present on Nightly.

Shu was able to reproduce the debugger crash on PlayCanvas and verified the patch fixes this crash. I agree we should keep an eye on crash-stats, but the patch is known to fix at least some crashes so I think there's a good reason to backport right? :)

(And the other crashes look very similar to this one..)
I'm a little lost as to why this bug wasn't resolved when it landed on m-c, but oh well.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8568878 [details] [diff] [review]
Ensure OSR frame scripts have debug instrumentation.

(In reply to Jan de Mooij [:jandem] from comment #10)
> Shu was able to reproduce the debugger crash on PlayCanvas and verified the
> patch fixes this crash. I agree we should keep an eye on crash-stats, but
> the patch is known to fix at least some crashes so I think there's a good
> reason to backport right? :)

Yes. I think this is a good argument for backporting. I missed that this was easily reproducible and didn't see any indication that the fix had been verified. Let's take this fix in Beta 3. Beta+ Aurora+
Attachment #8568878 - Flags: approval-mozilla-beta?
Attachment #8568878 - Flags: approval-mozilla-beta+
Attachment #8568878 - Flags: approval-mozilla-aurora?
Attachment #8568878 - Flags: approval-mozilla-aurora+
Whiteboard: [adv-main37+]

Updated

4 years ago
Group: core-security → core-security-release
Whiteboard: [adv-main37+] → [adv-main37+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.