Closed Bug 1108836 Opened 5 years ago Closed 5 years ago

Crash [@ js::gc::GCRuntime::sweepBackgroundThings]


(Core :: JavaScript: GC, defect, critical)

Not set



Tracking Status
firefox35 --- unaffected
firefox36 + verified
firefox37 + verified
firefox-esr31 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.2 --- fixed


(Reporter: decoder, Assigned: jonco)


(Blocks 1 open bug)


(4 keywords, Whiteboard: [jsbugmon:update,bisect,ignore][fuzzblocker][b2g-adv-main2.2-])

Crash Data


(2 files)

The following testcase crashes on mozilla-central revision 035a951fc24a (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-debug, run with --fuzzing-safe --thread-count=2):

const root = newGlobal();
var g = newGlobal();
for (var indexI = 0; indexI <= 65535; indexI++) {
    eval("/*var " + String.fromCharCode(indexI) + "xx = 1*/");


Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff7fe6700 (LWP 21844)]
0x00000000007d87e8 in js::gc::GCRuntime::sweepBackgroundThings (this=0x16ce338, zones=..., threadType=<optimized out>) at js/src/jsgc.cpp:3468
3468	                ArenaHeader *arenas = zone->allocator.arenas.arenaListsToSweep[kind];
#0  0x00000000007d87e8 in js::gc::GCRuntime::sweepBackgroundThings (this=0x16ce338, zones=..., threadType=<optimized out>) at js/src/jsgc.cpp:3468
#1  0x00000000007f16cf in doSweep (lock=..., this=<optimized out>) at js/src/jsgc.cpp:3713
#2  js::GCHelperState::work (this=0x16d5180) at js/src/jsgc.cpp:3599
#3  0x0000000000870726 in handleGCHelperWorkload (this=0x16e0830) at js/src/vm/HelperThreads.cpp:1309
#4  js::HelperThread::threadLoop (this=0x16e0830) at js/src/vm/HelperThreads.cpp:1367
#5  0x00000000008cd379 in nspr::Thread::ThreadRoutine (arg=0x16db350) at js/src/vm/PosixNSPR.cpp:45
#6  0x00007ffff7bc4e9a in start_thread (arg=0x7ffff7fe6700) at pthread_create.c:308
#7  0x00007ffff6cc131d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#8  0x0000000000000000 in ?? ()
rax	0x1	1
rbx	0x7ffff7fe5e30	140737354030640
rcx	0xa80	2688
rdx	0x1	1
rsi	0x0	0
rdi	0x7ffff7fe5e30	140737354030640
rbp	0x1652928	23406888
rsp	0x7ffff7fe5d30	140737354030384
r8	0x16db2e0	23966432
r9	0x5554	21844
r10	0x0	0
r11	0x1	1
r12	0x16ce338	23913272
r13	0x0	0
r14	0x6	6
r15	0xb1d690	11654800
rip	0x7d87e8 <js::gc::GCRuntime::sweepBackgroundThings(js::gc::ZoneList&, js::ThreadType)+152>
=> 0x7d87e8 <js::gc::GCRuntime::sweepBackgroundThings(js::gc::ZoneList&, js::ThreadType)+152>:	mov    0x378(%rdx,%rax,8),%rsi
   0x7d87f0 <js::gc::GCRuntime::sweepBackgroundThings(js::gc::ZoneList&, js::ThreadType)+160>:	test   %rsi,%rsi

Marking s-s due to crash instruction offset and GC-relatedness.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision f1f48ccb2d4e).
I'm seeing more GC crashes with newGlobal and gc, and they're all hard to reproduce/intermittent. Marking this as a fuzzblocker so we can figure out quickly, how many distinct issues we're dealing with here.

Needinfo from jandem for triage for now.
Flags: needinfo?(jdemooij)
Whiteboard: [jsbugmon:update,bisect,ignore] → [jsbugmon:update,bisect,ignore][fuzzblocker]
Jon, maybe this is something you would know about?
Flags: needinfo?(jcoppeard)
Keywords: sec-high
A similar crash signature has been showing up on TBPL in bug 1105123.
Thanks, investigating.
Assignee: nobody → jcoppeard
Flags: needinfo?(jdemooij)
Flags: needinfo?(jcoppeard)
This is a race when starting the background thread while it's currently inside doSweep() and was introduced by my changes in bug 989390.

This affects debug builds but is hard to reproduce there because of timing differences.  I managed to come up with a test that fails 90% of the time in opt builds though.

I also added a single release assert that catches the problem, where a zone that is queued for background sweeping is re-added the list.
Attachment #8534477 - Flags: review?(terrence)
Comment on attachment 8534477 [details] [diff] [review]

Review of attachment 8534477 [details] [diff] [review]:

Attachment #8534477 - Flags: review?(terrence) → review+
[Tracking Requested - why for this release]: sec-high
Blocks: 989390, 1105123
Component: JavaScript Engine → JavaScript: GC
As a sec-high or sec-critical bug that affects more than trunk this should have gotten sec-approval prior to landing.  Jon, can you confirm that this is a regression from bug 989390, or does it need to be backported further?  Also, please be sure to request Aurora approval at some point.
Flags: needinfo?(jcoppeard)
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
(In reply to Andrew McCreight [:mccr8] from comment #10)

I'm sorry for landing without approval (again!).

Yes, it's a regression from bug 989390.

I'm not sure exactly how the security classification works, but I don't think this is exploitable because the crash is always at a fixed address that is not under control of the user.
Flags: needinfo?(jcoppeard)
Comment on attachment 8534477 [details] [diff] [review]

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Certinaly non-trivial.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

The patch points to the existence of a race condition but not how to trigger or exploit it.

Which older supported branches are affected by this flaw?


If not all supported branches, which bug introduced the flaw?

Bug 989390.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

This patch should apply cleanly.

How likely is this patch to cause regressions; how much testing does it need?

Unlikely but we should let it bake on central for a few days.
Attachment #8534477 - Flags: sec-approval?
Comment on attachment 8534477 [details] [diff] [review]

sec-approval+ for trunk. Can you create and nominate an Aurora patch so we can get it in there too after some baking on trunk?
Attachment #8534477 - Flags: sec-approval? → sec-approval+
Approval Request Comment
[Feature/regressing bug #]: Bug 989390
[User impact if declined]: Possible intermittent crashes
[Describe test coverage new/current, TBPL]: On central since 12/12
[Risks and why]: Low
[String/UUID change made/needed]: None
Attachment #8536467 - Flags: approval-mozilla-aurora?
Attachment #8536467 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx36
Group: core-security
Whiteboard: [jsbugmon:update,bisect,ignore][fuzzblocker] → [jsbugmon:update,bisect,ignore][fuzzblocker][b2g-adv-main2.2-]
You need to log in before you can comment on or make changes to this bug.