Crash [@ js::gc::Cell::zone] or "Assertion failure: shared->activeUseCount == 0,"

VERIFIED FIXED

Status

()

--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: gkw, Assigned: sstangl)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86_64
Mac OS X
assertion, regression, sec-critical, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox18 unaffected, firefox19 unaffected, firefox20 unaffected, firefox21+ fixed, firefox-esr10 unaffected, firefox-esr17 unaffected, b2g18 unaffected)

Details

(Whiteboard: [fuzzblocker] [jsbugmon:update,ignore], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 703808 [details]
stack

''.search(evalcx("/(?:}?)/", undefined))
gc()
gc()

asserts js debug shell on m-c changeset b52c02f77cf5 without any CLI arguments at Assertion failure: shared->activeUseCount == 0,

s-s because gc is involved.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   118977:f2e3d3913d70
user:        Sean Stangl
date:        Tue Jan 15 15:35:25 2013 -0800
summary:     Bug 829758 - use MatchOnly mode for str_search(). r=dvander
Blocks: 832203
(Reporter)

Comment 1

6 years ago
Assuming sec-critical, feel free to adjust as necessary.
Keywords: sec-critical
See also bug 832203 which is likely a dup to this, but has security analysis.
(Reporter)

Comment 3

6 years ago
(In reply to Christian Holler (:decoder) from comment #2)
> See also bug 832203 which is likely a dup to this, but has security analysis.

Valgrind doesn't show any error when the testcase in this bug is run on opt builds, so guessing based on the fact that gc is essential and present on the stack.

I guess bug 832203 is a more potent testcase.

Updated

6 years ago
tracking-firefox21: ? → +
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unknown exception (check manually)
Marking as fuzzblocker. This and the test in bug 832203 seem to cause all sorts of crashes.
Whiteboard: [jsbugmon:] → [jsbugmon:update][fuzzblocker]
(Reporter)

Comment 6

6 years ago
Sean, comment 0 points to bug 829758, do you think this is a possible regressing changeset?
Flags: needinfo?(sstangl)
(Assignee)

Comment 7

6 years ago
(In reply to Gary Kwong [:gkw] from comment #6)
> Sean, comment 0 points to bug 829758, do you think this is a possible
> regressing changeset?

Yes, that's the regressing changeset, but I'm not sure what's so special about str_search() that creates all these cross-compartment RegExpShared references. I would have expected the same behavior to be exposed by exec(), test(), or replace().

We don't actually need the changes to str_search(), so it would be reasonable to back out that patch.
Flags: needinfo?(sstangl)
Whiteboard: [jsbugmon:update][fuzzblocker] → [fuzzblocker] [jsbugmon:]
JSBugMon: Cannot process bug: Unknown exception (check manually)
Nasty bug in the automation was causing this comment although the toolchain was broken...
Whiteboard: [fuzzblocker] [jsbugmon:] → [fuzzblocker] [jsbugmon:update]
(Reporter)

Comment 10

6 years ago
Created attachment 706951 [details]
stack for new testcase

try {
    var r = evalcx("new RegExp(\"\",\"\")", w);
    var s = "";
    (s.search(r))(x)
} catch (e) {}
    r = /()()()()/;
try {
    for (let x = 0; x < 3; ++x) {
        gc()
    }
} catch (e) {}
var w

crashes 64-bit js opt shell on m-c changeset 80fed51ae074 with --no-ti -a --ion-gvn=optimistic at js::gc::Cell::zone on Mac 10.8
(Reporter)

Updated

6 years ago
Crash Signature: [@ js::gc::Cell::zone]
Summary: "Assertion failure: shared->activeUseCount == 0," → Crash [@ js::gc::Cell::zone] or "Assertion failure: shared->activeUseCount == 0,"
(Assignee)

Comment 11

6 years ago
I have backed out Bug 829758, so this bug will no longer reproduce and will not block fuzzing. The underlying (preexisting) cause is not fixed, and I would like to re-land that bug at a later date, but I am interested in seeing whether the fuzzers also find similar crashes with .exec(), .test(), and .replace().
tracking-firefox21: + → ?
Whiteboard: [fuzzblocker] [jsbugmon:update] → [fuzzblocker] [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 4e7c92906a79).
(Assignee)

Comment 13

6 years ago
Just for reference, not that this bug is fixed by the patch (pending review) in Bug 832217. I'll land the relevant test cases in the re-landing of Bug 829758.

Updated

6 years ago
tracking-firefox21: ? → +
Assignee: general → sstangl
(Assignee)

Comment 14

6 years ago
This should now be fixed without the workaround: fix landed in Bug 832217, testcases added in Bug 829758.
tracking-firefox21: + → ?
Flags: needinfo?(gary)
Flags: in-testsuite+
(Reporter)

Comment 15

6 years ago
I have not seen this bug anymore since the backout in comment 11.
Flags: needinfo?(gary)
(Assignee)

Comment 16

6 years ago
(In reply to Gary Kwong [:gkw] from comment #15)
> I have not seen this bug anymore since the backout in comment 11.

Bug 829758 was re-landed -- needinfo was to verify that the fix and resolve the bug :)
Re-tracking, please update status flag to fixed when this is verified.
tracking-firefox21: ? → +
(Reporter)

Comment 18

6 years ago
I verify that the testcases do not reproduce with mozilla-inbound changeset 8619a2942136 (which contains the fix(es) as described in comment 14).

Marking FIXED by bug 832217, and thus VERIFIED and in-testsuite+ because testcases landed in bug 829758.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Updated

6 years ago
Status: RESOLVED → VERIFIED
status-firefox21: affected → fixed
Group: core-security
You need to log in before you can comment on or make changes to this bug.