Assertion failure: size % pageSize == 0, at js/src/gc/Memory.cpp:940 on ARM64

RESOLVED FIXED in Firefox 65

Status

()

defect
P1
critical
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: decoder, Assigned: jonco)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla65
ARM64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 fixed)

Details

(Whiteboard: [jsbugmon:ignore][fuzzblocker][arm64:m1])

Attachments

(1 attachment)

Reporter

Description

6 months ago
The following testcase crashes on mozilla-central revision f6df375b8698 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --enable-debug --disable-jemalloc, run with --fuzzing-safe --ion-offthread-compile=off test.js):

gczeal(10, 9);
var g = newGlobal();


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000aaaaab33fa34 in js::gc::ProtectPages (p=<optimized out>, size=<optimized out>) at js/src/gc/Memory.cpp:942
942	    MOZ_RELEASE_ASSERT(p);
#0  0x0000aaaaab33fa34 in js::gc::ProtectPages (p=<optimized out>, size=<optimized out>) at js/src/gc/Memory.cpp:942
#1  0x0000aaaaab2c200c in js::gc::GCRuntime::protectAndHoldArenas (this=0xaaaaacc4a8e8, arenaList=0x700001bc000) at js/src/gc/GC.cpp:3151
#2  0x0000aaaaab2d9654 in js::gc::GCRuntime::compactPhase (this=0xaaaaacc4a8e8, reason=JS::gcreason::DEBUG_GC, sliceBudget=..., session=...) at js/src/gc/GC.cpp:7145
#3  0x0000aaaaab2dcaf0 in js::gc::GCRuntime::incrementalSlice (this=0xaaaaacc4a8e8, budget=..., reason=<optimized out>, session=...) at js/src/gc/GC.cpp:7641
#4  0x0000aaaaab2de53c in js::gc::GCRuntime::gcCycle (this=0xaaaaacc4a8e8, nonincrementalByAPI=false, budget=..., reason=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7945
#5  0x0000aaaaab2e0198 in js::gc::GCRuntime::collect (this=0xaaaaacc4a8e8, nonincrementalByAPI=<optimized out>, budget=..., reason=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:8126
#6  0x0000aaaaab2aecc8 in js::gc::GCRuntime::runDebugGC (this=0xaaaaacc4a8e8) at js/src/gc/GC.cpp:8732
#7  0x0000aaaaab2ae82c in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0xaaaaacc4a8e8, cx=0xaaaaacc57a30) at js/src/gc/Allocator.cpp:336
#8  0x0000aaaaab2f14d8 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0x0, cx=0xaaaaacc57a30, kind=js::gc::AllocKind::BASE_SHAPE) at js/src/gc/Allocator.cpp:295
#9  0x0000aaaaab2f3e10 in js::Allocate<js::BaseShape, (js::AllowGC)1> (cx=0xaaaaacc57a30) at js/src/gc/Allocator.cpp:239
#10 0x0000aaaaaaf28ed0 in js::BaseShape::getUnowned (cx=0xaaaaacc57a30, base=...) at js/src/vm/Shape.cpp:1552
#11 0x0000aaaaaaf2b6a4 in js::EmptyShape::getInitialShape (cx=0xaaaaacc57a30, clasp=0xaaaaac02a518 <js::DebuggerFrame::class_>, proto=..., nfixed=7, objectFlags=0) at js/src/vm/Shape.cpp:2256
#12 0x0000aaaaaae7b384 in NewObject (cx=0xaaaaacc57a30, group=..., kind=<optimized out>, newKind=<optimized out>, initialShapeFlags=0) at js/src/vm/JSObject.cpp:798
#13 0x0000aaaaaae7a428 in js::NewObjectWithGivenTaggedProto (cx=0xaaaaacc57a30, clasp=<optimized out>, proto=..., allocKind=<optimized out>, newKind=<optimized out>, initialShapeFlags=<optimized out>) at js/src/vm/JSObject.cpp:875
#14 0x0000aaaaaae83c74 in js::NewObjectWithClassProto (cx=<optimized out>, clasp=<optimized out>, newKind=js::SingletonObject, proto=..., allocKind=<optimized out>) at js/src/vm/JSObject-inl.h:546
#15 js::NewObjectWithClassProto (newKind=js::SingletonObject, cx=<optimized out>, clasp=<optimized out>, proto=...) at js/src/vm/JSObject-inl.h:554
#16 js::NewNativeObjectWithClassProto (newKind=js::SingletonObject, cx=<optimized out>, clasp=<optimized out>, proto=...) at js/src/vm/NativeObject-inl.h:798
#17 DefineConstructorAndPrototype (cx=<optimized out>, key=<optimized out>, constructor=<optimized out>, ps=<optimized out>, fs=<optimized out>, static_ps=<optimized out>, static_fs=<optimized out>, ctorp=<optimized out>, obj=..., atom=..., protoProto=..., clasp=<optimized out>, nargs=<optimized out>) at js/src/vm/JSObject.cpp:2037
#18 js::InitClass (cx=0xaaaaacc57a30, obj=..., protoProto_=..., clasp=0xaaaaac02a518 <js::DebuggerFrame::class_>, constructor=0xaaaaaad76e60 <js::DebuggerFrame::construct(JSContext*, unsigned int, JS::Value*)>, nargs=0, ps=0xaaaaac02adc0 <js::DebuggerFrame::properties_>, fs=0xaaaaac02b090 <js::DebuggerFrame::methods_>, static_ps=0x0, static_fs=0x0, ctorp=0x0) at js/src/vm/JSObject.cpp:2167
#19 0x0000aaaaaad76dd8 in js::DebuggerFrame::initClass (cx=0xaaaaacc57a30, dbgCtor=..., global=...) at js/src/vm/Debugger.cpp:8537
#20 0x0000aaaaaad96c90 in JS_DefineDebuggerObject (cx=0xaaaaacc57a30, obj=...) at js/src/vm/Debugger.cpp:12750
#21 0x0000aaaaaabc8ed0 in NewGlobalObject (cx=0xaaaaacc57a30, options=..., principals=<optimized out>) at js/src/shell/js.cpp:10101
#22 0x0000aaaaaabd9584 in NewGlobal (cx=0xaaaaacc57a30, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:6352
#23 0x0000aaaaaac56f30 in CallJSNative (cx=0xaaaaacc57a30, native=0xaaaaaabd93f0 <NewGlobal(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:468
[...]
#36 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:11297
x0	0x0	0
x1	0x0	0
x2	0x1	1
x3	0x0	0
x4	0x0	0
x5	0x0	0
x6	0x0	0
x7	0xfffffffd	4294967293
x8	0x0	0
x9	0x6a2f6c61	8246987515394747489
x10	0x1010101	72340172838076673
x11	0x38	56
x12	0x0	0
x13	0x3ac	940
x14	0xbe2b3e08	281473872248328
x15	0x2a8	680
x16	0xbe313a60	281473872640608
x17	0xac0b0420	187650007565344
x18	0x1fff	8191
x19	0x3ac	940
x20	0xacc4a8e8	187650019731688
x21	0xacc4bd30	187650019736880
x22	0x1bc000	7696583213056
x23	0x1b7000	7696583192576
x24	0x1bc000	7696583213056
x25	0xffffc780	281474976696192
x26	0xacc4d608	187650019743240
x27	0xacc4d240	187650019742272
x28	0xad00e820	187650023680032
x29	0xffffc3c0	281474976695232
x30	0xab33fa30	187649993472560
sp	0xffffc3b0	281474976695216
pc	0xaaaaab33fa34 <js::gc::ProtectPages(void*, unsigned long)+156>
cpsr	[ EL=0 C Z ]
fpcsr	void
fpcr	0x0	0
=> 0xaaaaab33fa34 <js::gc::ProtectPages(void*, unsigned long)+156>:	str	w19, [x8]
   0xaaaaab33fa38 <js::gc::ProtectPages(void*, unsigned long)+160>:	bl	0xaaaaaac03a28 <abort()>


This is on native ARM64 and blocking further fuzzing. Marking s-s due to GC involved, but I'm not sure if this is a problem in the browser too, given how frequently this happens.
Reporter

Comment 1

6 months ago
The pageSize is 64K btw:

0x0000aaaaab33fa34 in js::gc::ProtectPages (p=<optimized out>, size=<optimized out>) at /js/src/gc/Memory.cpp:942
942         MOZ_RELEASE_ASSERT(p);
(gdb) p pageSize
$1 = 65536



Jon, can you look into this or forward to someone who can?
Flags: needinfo?(jcoppeard)
Reporter

Comment 2

6 months ago
Also not s-s, this is a release assert and probably some platform compat problem.
Group: javascript-core-security
We knew we were going to hit this problem, so chances are it's not too hard to correct for it.
Assignee

Comment 4

6 months ago
Patch to check the system page size before trying to protect arenas.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Reporter

Comment 5

6 months ago
Comment on attachment 9024989 [details] [diff] [review]
bug1506954-check-before-protect

Confirmed, fixes the problem, thanks!
Attachment #9024989 - Flags: feedback+
Assignee

Comment 6

6 months ago
Comment on attachment 9024989 [details] [diff] [review]
bug1506954-check-before-protect

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

When we relocate arenas in a compacting GC that was triggered with GC zeal, we use mprotect to make the relocated arenas inaccessible for a while to catch any stray accesses.  This doesn't work if the system page size is larger than the size of an arena, which it is on some platforms.   This patch just adds a check and skips this if it's not supported.
Attachment #9024989 - Flags: review?(pbone)
Whiteboard: [jsbugmon:ignore][fuzzblocker] → [jsbugmon:ignore][fuzzblocker][arm64:m1]
Priority: -- → P1

Comment 7

6 months ago
Comment on attachment 9024989 [details] [diff] [review]
bug1506954-check-before-protect

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

LGTM

::: js/src/gc/GC.cpp
@@ +2476,5 @@
> +CanProtectArenas()
> +{
> +    // On some systems the page size does not match the size of an
> +    // arena so we can't change the mapping permissions per arena.
> +    return SystemPageSize() == ArenaSize;

Could make this <= in case we ever change the size of arenas.  (I don't know of anything using pages less than 4K.)
Attachment #9024989 - Flags: review?(pbone) → review+

Comment 8

6 months ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7211db666d59
Don't attempt to protect relocated pages on systems that don't support this r=pbone

Comment 9

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7211db666d59
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Should we land this testcase still?
Flags: needinfo?(jcoppeard)
Assignee

Comment 11

6 months ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
This is covered by existing tests.
Flags: needinfo?(jcoppeard)
You need to log in before you can comment on or make changes to this bug.