2014365 - SEGV in IsWriteableAddress(void*) at gc/Nursery.cpp:968:11 via UpdateIonJSFrameForMinorGC

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[x0e]

Steps to reproduce:

engine version             : d8636da7d75fe4aff0a4ccea8413cfcdc7be08fd
engine version date    : 2026-02-03T00:30:44

Flags: js --fuzzing-safe --gc-zeal=7,153 --fast-warmup --ion-eager test.js

Test case:

for (let i1 = 2694; i1; --i1) {
}
for (let v5 = 0; v5 < 50; v5++) {
    const v8 = new BigUint64Array(v5);
    for (const v9 of v8) {
        const v12 = v8["subarray"](1)[14];
        v12 + v12;
    }
}

Actual results:

Backtrace:

==2081277==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x6138696eab95 bp 0x7fff6b738e10 sp 0x7fff6b738df0 T2081277)
==2081277==The signal is caused by a READ memory access.
==2081277==Hint: address points to the zero page.
/opt/swift/usr/bin/llvm-symbolizer: error: '[anon:js-executable-memory]': No such file or directory
    #0 0x6138696eab95 in IsWriteableAddress(void*) js/src/gc/Nursery.cpp:968:11
    #1 0x6138696eab95 in js::Nursery::forwardBufferPointer(unsigned long*) js/src/gc/Nursery.cpp:997:5
    #2 0x613869ebe034 in js::jit::UpdateIonJSFrameForMinorGC(JSRuntime*, js::jit::JSJitFrameIter const&) js/src/jit/JitFrames.cpp:1156:15
    #3 0x613869ebe034 in js::jit::UpdateJitActivationsForMinorGC(JSRuntime*) js/src/jit/JitFrames.cpp:1520:11
    #4 0x6138696ec573 in js::Nursery::doCollection(js::gc::AutoGCSession&, JS::GCOptions, JS::GCReason) js/src/gc/Nursery.cpp:1592:3
    #5 0x6138696c6ec3 in js::Nursery::collect(JS::GCOptions, JS::GCReason) js/src/gc/Nursery.cpp:1417:31
    #6 0x6138696c0e3c in js::gc::GCRuntime::collectNursery(JS::GCOptions, JS::GCReason, js::gcstats::PhaseKind) js/src/gc/GC.cpp:5076:13
    #7 0x6138696a89ad in js::gc::GCRuntime::minorGC(JS::GCReason, js::gcstats::PhaseKind) js/src/gc/GC.cpp:5047:3
    #8 0x6138696853c1 in js::gc::PreAllocGCChecks(JSContext*) js/src/gc/Allocator-inl.h:152:9
    #9 0x6138696853c1 in void* js::gc::CellAllocator::AllocNurseryOrTenuredCell<(JS::TraceKind)1, (js::AllowGC)1>(JSContext*, js::gc::AllocKind, unsigned long, js::gc::Heap, js::gc::AllocSite*) js/src/gc/Allocator-inl.h:194:5
    #10 0x61386a07f64c in JS::BigInt::createFromUint64(JSContext*, unsigned long, js::gc::Heap) js/src/vm/BigIntType.cpp
    #11 0x3d5cc0b2fc32  ([anon:js-executable-memory]+0x9c32)

Comment 1

[x0e]

Different reproducer with non null adress causing the crash. Note the different flags.

Flags:

js  --fuzzing-safe  --ion-regalloc=simple bug.js

Test case:

const v0 = [550891154,1608978634,-4,21135,-2147483647,536870887,5,-64072,-9007199254740991];
function f1(...a2) {
    a2[1] = a2;
    bar.apply({}, a2);
}
function bar(a7, a8) {
    f1(a8.push(v0));
}
f1();

Backtrace

==2085639==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0xfff8800000000003 (pc 0x5e8e15bd2b95 bp 0x7ffd86b319e0 sp 0x7ffd86b319c0 T2085639)
==2085639==The signal is caused by a READ memory access.
/opt/swift/usr/bin/llvm-symbolizer: error: '[anon:js-executable-memory]': No such file or directory
    #0 0x5e8e15bd2b95 in IsWriteableAddress(void*) js/src/gc/Nursery.cpp:968:11
    #1 0x5e8e15bd2b95 in js::Nursery::forwardBufferPointer(unsigned long*) js/src/gc/Nursery.cpp:997:5
    #2 0x5e8e163a60e3 in js::jit::UpdateIonJSFrameForMinorGC(JSRuntime*, js::jit::JSJitFrameIter const&) js/src/jit/JitFrames.cpp:1175:13
    #3 0x5e8e163a60e3 in js::jit::UpdateJitActivationsForMinorGC(JSRuntime*) js/src/jit/JitFrames.cpp:1520:11
    #4 0x5e8e15bd4573 in js::Nursery::doCollection(js::gc::AutoGCSession&, JS::GCOptions, JS::GCReason) js/src/gc/Nursery.cpp:1592:3
    #5 0x5e8e15baeec3 in js::Nursery::collect(JS::GCOptions, JS::GCReason) js/src/gc/Nursery.cpp:1417:31
    #6 0x5e8e15ba8e3c in js::gc::GCRuntime::collectNursery(JS::GCOptions, JS::GCReason, js::gcstats::PhaseKind) js/src/gc/GC.cpp:5076:13
    #7 0x5e8e15b909ad in js::gc::GCRuntime::minorGC(JS::GCReason, js::gcstats::PhaseKind) js/src/gc/GC.cpp:5047:3
    #8 0x5e8e15b6cb1f in void* js::gc::CellAllocator::RetryNurseryAlloc<(js::AllowGC)1>(JSContext*, JS::TraceKind, js::gc::AllocKind, unsigned long, js::gc::AllocSite*) js/src/gc/Allocator.cpp:106:23
    #9 0x5e8e15667c3d in js::ArrayObject* js::gc::CellAllocator::NewObject<js::ArrayObject, (js::AllowGC)1>(JSContext*, js::gc::AllocKind, js::gc::Heap, JSClass const*, js::gc::AllocSite*) js/src/gc/Allocator-inl.h:112:16
    #10 0x5e8e1565bff7 in js::ArrayObject* js::gc::CellAllocator::NewCell<js::ArrayObject, (js::AllowGC)1, js::gc::AllocKind&, js::gc::Heap&, JSClass const*&, js::gc::AllocSite*&>(JSContext*, js::gc::AllocKind&, js::gc::Heap&, JSClass const*&, js::gc::AllocSite*&) js/src/gc/Allocator-inl.h:35:12
    #11 0x5e8e1565bff7 in js::ArrayObject* JSContext::newCell<js::ArrayObject, (js::AllowGC)1, js::gc::AllocKind&, js::gc::Heap&, JSClass const*&, js::gc::AllocSite*&>(js::gc::AllocKind&, js::gc::Heap&, JSClass const*&, js::gc::AllocSite*&) js/src/vm/JSContext-inl.h:379:10
    #12 0x5e8e1565bff7 in js::ArrayObject::create(JSContext*, js::gc::AllocKind, js::gc::Heap, JS::Handle<js::SharedShape*>, unsigned int, unsigned int, js::AutoSetNewObjectMetadata&, js::gc::AllocSite*) js/src/vm/ArrayObject-inl.h:42:27
    #13 0x5e8e15657aa2 in js::ArrayObject* NewArrayWithShape<4294967295u>(JSContext*, JS::Handle<js::SharedShape*>, unsigned int, js::NewObjectKind, js::gc::AllocSite*) js/src/builtin/Array.cpp:5310:22
    #14 0x5e8e15632c92 in js::ArrayObject* NewArray<4294967295u>(JSContext*, unsigned int, js::NewObjectKind, js::gc::AllocSite*) js/src/builtin/Array.cpp:5380:10
    #15 0x5e8e15632fd4 in js::NewDenseCopiedArray(JSContext*, unsigned int, JS::Value const*, js::NewObjectKind) js/src/builtin/Array.cpp:5525:22
    #16 0x084ef8a3026d  ([anon:js-executable-memory]+0x2226d)

Comment 2

[Jan de Mooij [:jandem]]

Slightly simplified test case below.

anba, I think this is a problem with LArrayBufferViewElementsWithOffset. It returns an elements pointer that's at an offset from the base elements pointer and this confuses the GC's buffer forwarding mechanism.

// --fuzzing-safe --gc-zeal=7,153 --fast-warmup --ion-eager
function test() {
  for (let i = 0; i < 2000; i++) {
  }
  for (let i = 0; i < 50; i++) {
    const ta = new BigUint64Array(i);
    for (const v of ta) {
      const val = ta.subarray(1)[14];
    }
  }
}
test();

Comment 3

[André Bargull [:anba]]

I'm not sure the LArrayBufferViewElementsWithOffset bug is exploitable, because the elements pointer from LArrayBufferViewElementsWithOffset isn't used after the minor GC, unlike for example in bug 1992902. Is my assumption correct that an unused elements pointer can't cause any issues?

Also the test case from comment #1 looks like a different bug.

Test case which asserts without gczeal:

// (No extra shell options needed.)
function test() {
  let r = [];
  for (let i = 0; i < 10; ++i) {
    for (let j = 0; j < 100; ++j) {
      const ta = new BigUint64Array(j);
      for (let k = 0; k < j; ++k) {
        const val = ta.subarray(1)[14];
        r[k & 1] = val;
      }
    }
  }
  print("r=", r);
}
test();

Comment 4

[Jan de Mooij [:jandem]]

I've filed bug 2015813 for the test in comment 1 because it's a different issue. I think it requires the simple allocator so it's less urgent for us to fix.

Comment 5

[Iain Ireland [:iain]]

This isn't the first time that we've been bitten here. Is the root of the problem here that we're creating a value that we claim is Elements-typed, but is actually not? I wonder if there's an approach to fixing this that adds a new MIR type (eg ElementsWithOffset), and then teaches the consumers of ArrayBufferViewElementsWithOffset to also consume values of that type.

Comment 6

[André Bargull [:anba]]

Attachment: (secure)

This allows to remove the ArrayBufferViewElementsWithOffset instruction.

Comment 7

[André Bargull [:anba]]

Attachment: (secure)

This avoids MKeepAliveObject for the MAdd added in part 1.

Comment 8

[André Bargull [:anba]]

It should be possible to remove ArrayBufferViewElementsWithOffset by adjusting the unboxed scalar indices during scalar replacement. I've attached two patches I've originally written bug 1987481.

Comment 9

[Jan de Mooij [:jandem]]

(In reply to André Bargull [:anba] from comment #8)

It should be possible to remove ArrayBufferViewElementsWithOffset by adjusting the unboxed scalar indices during scalar replacement. I've attached two patches I've originally written bug 1987481.

I like that approach. To make sure we don't do duplicate work, do you want to take this bug or should I do it?

Comment 10

[Frederik Braun [:freddy]]

I'm tagging this as sec-low. anba described this as a non-exploitable crash in comment 3. We can always change the rating if information arrives (different PoC, new analysis when fixing the issue etc.).

Comment 11

[André Bargull [:anba]]

Attachment: Bug 2014365: Add test case. r=jandem!

Comment 12

[André Bargull [:anba]]

(In reply to Jan de Mooij [:jandem] from comment #9)

I like that approach. To make sure we don't do duplicate work, do you want to take this bug or should I do it?

I've uploaded the patches for proper review.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3c9fd678aa6c
https://hg.mozilla.org/integration/autoland/rev/8c1b3d1d0d46
Part 1: Directly adjust unboxed scalar indices during scalar replacement. r=jandem
https://github.com/mozilla-firefox/firefox/commit/98bb00978e59
https://hg.mozilla.org/integration/autoland/rev/116937e44442
Part 2: Allow MAdd in NeedsKeepAlive. r=jandem

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8c1b3d1d0d46
https://hg.mozilla.org/mozilla-central/rev/116937e44442

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/61ff36f7fca4
https://hg.mozilla.org/integration/autoland/rev/1c3d3a62f38a
Add test case. r=jandem

Comment 16

[rperta]

https://hg.mozilla.org/mozilla-central/rev/1c3d3a62f38a