Closed Bug 1609054 Opened 5 years ago Closed 5 years ago

AddressSanitizer: heap-buffer-overflow [@ js::gc::MarkingValidator::validate] with READ of size 1 and gcslice

Categories

(Core :: JavaScript: GC, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox-esr68 --- unaffected
firefox72 --- unaffected
firefox73 --- unaffected
firefox74 --- fixed

People

(Reporter: gkw, Assigned: jonco)

References

(Blocks 1 open bug, Regression)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(2 files, 1 obsolete file)

The following testcase crashes on mozilla-central revision d1406439c461 (build with --enable-address-sanitizer, run with --fuzzing-safe --no-threads --no-baseline --no-ion --gc-zeal=11):

try {} catch (e) {}
try {
    x;
} catch (e) {}
gcslice(6326);
uneval(this);

Backtrace:

==7832==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60300000a13f at pc 0x55c6e5533f58 bp 0x7fffd6698720 sp 0x7fffd6698718
READ of size 1 at 0x60300000a13f thread T0
    #0 0x55c6e5533f57 in js::gc::MarkingValidator::validate() js/src/gc/Verifier.cpp:703:15
    #1 0x55c6e5431b88 in js::gc::GCRuntime::beginSweepingSweepGroup(JSFreeOp*, js::SliceBudget&) js/src/gc/GC.cpp:5150:3
    #2 0x55c6e549ec27 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) js/src/gc/GC.cpp:5882:23
    #3 0x55c6e54904d6 in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) js/src/gc/GC.cpp:5917:19
    #4 0x55c6e543d4e7 in js::gc::GCRuntime::performSweepActions(js::SliceBudget&) js/src/gc/GC.cpp:6050:48
    #5 0x55c6e5443b5a in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason, js::gc::AutoGCSession&) js/src/gc/GC.cpp:6572:11
/snip

For detailed crash information, see attachment.

Setting s-s as a start as this ASan issue involves GC.

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/d389e3aa4522
user: Jon Coppeard
date: Thu Jan 09 10:35:24 2020 +0000
summary: Bug 1424934 - Replace the chunk's mark bitmap with one byte per cell in the arena r=sfink,jandem

Jon, is bug 1424934 a likely regressor?

Flags: needinfo?(jcoppeard)
Regressed by: 1424934

Opening up, :decoder pointed out to me that uneval is shell-only.

Group: javascript-core-security

Gary I can't reproduce this (tried the exact changeset / configure flags / options). Are you seeing any other cases with failures like this?

Assignee: nobody → jcoppeard
Group: javascript-core-security
Component: JavaScript Engine → JavaScript: GC
Flags: needinfo?(jcoppeard) → needinfo?(nth10sd)
Group: javascript-core-security

I can still reproduce on m-c rev e71639544560 .

Jon, does this Pernosco link help? (Tested on Ubuntu 18.04.3 LTS, clang 8)

https://pernos.co/debug/_Wy4fepHebGpNjoJMV2ywQ/index.html

Flags: needinfo?(nth10sd) → needinfo?(jcoppeard)

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #5)
Thanks that helps a lot.

This is a problem with the incremental marking verifier. The problem is that an arena can be freed after having its mark bits checked and reallocated in a new zone before we check the mark bits for that zone.

Flags: needinfo?(jcoppeard)
Attachment #9120827 - Attachment description: Bug 1609054 - Check whether arenas have been reallocated in a different zone in the incremental marking verifier r?sfink → Bug 1609054 - Check whether arenas have been reallocated in a different zone in the incremental marking verifier r=sfink
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dbd45b7bb449 Check whether arenas have been reallocated in a different zone in the incremental marking verifier r=sfink
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

Should we land a test for this?

Flags: needinfo?(jcoppeard)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #11)
I'd like to have a test for this, but I couldn't get the testcase to reproduce at all so it's unlikely it would do much good landing it.

Flags: needinfo?(jcoppeard)
Backout by malexandru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/33932b670a10 Backed out changeset dbd45b7bb449 as requested by jonco:

This was backed out because the change that caused it (bug 1424934) was also backed out.

Attachment #9120827 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: