Assertion failure: rt->gc.atomMarking.atomIsMarked(compartment->zone(), cell), at js/src/jscntxtinlines.h:92 with saveStack

RESOLVED FIXED in Firefox 54



2 years ago
2 years ago


(Reporter: decoder, Assigned: bhackett)


(Blocks: 1 bug, 4 keywords)

assertion, jsbugmon, regression, testcase
Dependency tree / graph

Firefox Tracking Flags

(firefox52 unaffected, firefox53 unaffected, firefox54 fixed)


(Whiteboard: [jsbugmon:update])


(1 attachment, 1 obsolete attachment)



2 years ago
The following testcase crashes on mozilla-central revision 9c06e744b1be (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off min.js):

low = high = newGlobal({})
high.low = low
high.eval("function a() { return saveStack(1, low) }")
set = eval("high.a()")


 received signal SIGSEGV, Segmentation fault.
0x0000000000535be8 in js::CompartmentChecker::checkAtom (cell=<optimized out>, this=<optimized out>) at js/src/jscntxtinlines.h:92
#0  0x0000000000535be8 in js::CompartmentChecker::checkAtom (cell=<optimized out>, this=<optimized out>) at js/src/jscntxtinlines.h:92
#1  js::CompartmentChecker::check (symbol=<optimized out>, this=<optimized out>) at js/src/jscntxtinlines.h:107
#2  js::CompartmentChecker::check (this=0x7fffffffcdd0, v=...) at js/src/jscntxtinlines.h:116
#3  0x0000000000936abc in js::CompartmentChecker::check<JS::Value> (handle=..., this=0x7fffffffcdd0) at js/src/jscntxtinlines.h:83
#4  js::assertSameCompartment<JS::Handle<JS::Value> > (cx=0x7ffff6946000, t1=...) at js/src/jscntxtinlines.h:189
#5  0x0000000000bcfd5c in JSStructuredCloneWriter::startWrite (this=this@entry=0x7fffffffd1a0, v=...) at js/src/vm/StructuredClone.cpp:1378
#6  0x0000000000bd0e3d in JSStructuredCloneWriter::traverseSavedFrame (this=this@entry=0x7fffffffd1a0, obj=..., obj@entry=...) at js/src/vm/StructuredClone.cpp:1351
#7  0x0000000000bd035f in JSStructuredCloneWriter::startWrite (this=this@entry=0x7fffffffd1a0, v=...) at js/src/vm/StructuredClone.cpp:1448
#8  0x0000000000bd263d in JSStructuredCloneWriter::write (v=..., this=0x7fffffffd1a0) at js/src/vm/StructuredClone.cpp:1616
#9  WriteStructuredClone (cx=cx@entry=0x7ffff6946000, v=..., bufp=bufp@entry=0x7fffffffd660, scope=scope@entry=JS::StructuredCloneScope::SameProcessSameThread, cloneDataPolicy=..., cloneDataPolicy@entry=..., cb=cb@entry=0x0, cbClosure=0x0, transferable=...) at js/src/vm/StructuredClone.cpp:541
#10 0x0000000000bd275c in JS_WriteStructuredClone (cx=cx@entry=0x7ffff6946000, value=..., bufp=bufp@entry=0x7fffffffd660, scope=JS::StructuredCloneScope::SameProcessSameThread, cloneDataPolicy=..., cloneDataPolicy@entry=..., optionalCallbacks=optionalCallbacks@entry=0x0, closure=0x0, transferable=...) at js/src/vm/StructuredClone.cpp:2497
#11 0x0000000000bd27db in JSAutoStructuredCloneBuffer::write (this=this@entry=0x7fffffffd658, cx=cx@entry=0x7ffff6946000, value=..., transferable=..., cloneDataPolicy=..., cloneDataPolicy@entry=..., optionalCallbacks=optionalCallbacks@entry=0x0, closure=0x0) at js/src/vm/StructuredClone.cpp:2662
#12 0x000000000086a961 in Serialize (cx=cx@entry=0x7ffff6946000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:2449
#13 0x0000000000535d09 in js::CallJSNative (cx=cx@entry=0x7ffff6946000, native=0x86a5f0 <Serialize(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:262
#26 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7975
rax	0x0	0
rbx	0x7ffff06aa128	140737226907944
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffcdb0	140737488342448
rsp	0x7fffffffcda0	140737488342432
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7fffffffcdd0	140737488342480
r13	0x7fffffffcf30	140737488342832
r14	0x7fffffffcf10	140737488342800
r15	0x7fffffffcf70	140737488342896
rip	0x535be8 <js::CompartmentChecker::check(JS::Value const&)+376>
=> 0x535be8 <js::CompartmentChecker::check(JS::Value const&)+376>:	movl   $0x0,0x0
   0x535bf3 <js::CompartmentChecker::check(JS::Value const&)+387>:	ud2    

Another GC assert, but I'm not sure if this one is shell-only or not (due to saveStack being involved). Marking s-s anyway until triaged.


2 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

2 years ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
user:        Brian Hackett
date:        Mon Jan 30 06:31:47 2017 -0700
summary:     Bug 1324002 - Mark atoms separately in each zone, r=jonco,mccr8,peterv.

This iteration took 0.437 seconds to run.
Brian, is bug 1324002 a likely regressor?
Blocks: 1324002
Flags: needinfo?(bhackett1024)
Keywords: sec-high
Keywords: csectype-uaf
[Tracking Requested - why for this release]:
status-firefox53: --- → unaffected
tracking-firefox54: --- → ?

Comment 4

2 years ago
Created attachment 8833041 [details] [diff] [review]

This is a false positive, JSStructuredCloneWriter::startWrite always asserts the value is in the same compartment as the context, but for non-objects it just writes out the value without storing it anywhere.  This patch only checks when objects are being written out, though an alternative I guess would be to mark the atoms in the context when the caller is inspecting an unwrapped saved frame.
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8833041 - Flags: review?(jcoppeard)
Tracking 54+ for this sec high issue.
tracking-firefox54: ? → +
Comment on attachment 8833041 [details] [diff] [review]

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

I think I'd like to keep the assertion as it is since it seems like an invariant that we should enforce.  Can you try your alternative approach?

If you really want to do this you could ask Steve what he thinks as he knows this code better then me.
Attachment #8833041 - Flags: review?(jcoppeard)

Comment 7

2 years ago
Created attachment 8833276 [details] [diff] [review]

Sure, here's the alternative patch.
Attachment #8833041 - Attachment is obsolete: true
Attachment #8833276 - Flags: review?(jcoppeard)
Comment on attachment 8833276 [details] [diff] [review]

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

Attachment #8833276 - Flags: review?(jcoppeard) → review+
Unhiding per comment 4.
Group: javascript-core-security
Keywords: csectype-uaf, sec-high
tracking-firefox54: + → ---

Comment 10

2 years ago
Pushed by
Fix bogus assert by marking atoms when serializing cross-zone saved frames, r=jonco.

Comment 11

2 years ago
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
status-firefox52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.