1978767 - Assertion failure: oldBytes == SmallBufferRegion::from(alloc)->allocBytes(alloc), at gc/BufferAllocator.cpp:1168

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20250722-73304b4f70e7 (debug build, run with --fuzzing-safe --ion-offthread-compile=off):

gczeal(5, 4)
a = []
for (b = 0; b < 20; b++) a.push(b)
Object.defineProperty(a, "length", {
    writable: false
}).shift()

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x000055555792e3f7 in js::gc::BufferAllocator::realloc(void*, unsigned long, unsigned long, bool) ()
#1  0x000055555734bb42 in js::HeapSlot* js::ReallocateCellBuffer<js::HeapSlot>(JSContext*, js::gc::Cell*, js::HeapSlot*, unsigned int, unsigned int) ()
#2  0x000055555734e5a9 in js::NativeObject::shrinkElements(JSContext*, unsigned int) ()
#3  0x00005555570acf58 in array_shift(JSContext*, unsigned int, JS::Value*) ()
#4  0x000055555703b255 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
#5  0x000055555703a960 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#6  0x0000555557b22980 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICFallbackStub*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) ()
#7  0x00002e4b45f920ff in ?? ()
[..]
#15 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff46f8740	140737294337856
rcx	0x490	1168
rdx	0x1	1
rsi	0x0	0
rdi	0x7ffff7bee7d0	140737349871568
rbp	0x7fffffffc550	140737488340304
rsp	0x7fffffffc500	140737488340224
r8	0x0	0
r9	0x3	3
r10	0x0	0
r11	0x0	0
r12	0xb0	176
r13	0x100	256
r14	0x24447820cf30	39876491792176
r15	0x100	256
rip	0x55555792e3f7 <js::gc::BufferAllocator::realloc(void*, unsigned long, unsigned long, bool)+1207>
=> 0x55555792e3f7 <_ZN2js2gc15BufferAllocator7reallocEPvmmb+1207>:	mov    %rcx,(%rax)
   0x55555792e3fa <_ZN2js2gc15BufferAllocator7reallocEPvmmb+1210>:	callq  0x555556f54f80 <abort>

S-s until confirmed to be harmless.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20250722162531-73304b4f70e7.
The bug appears to have been introduced in the following build range:

Start: 59690b4d1c940a5784215da8c1d6c34af5306f85 (20250721141603)
End: 742051417634b357b311c389368ac1e378770f32 (20250721151107)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=59690b4d1c940a5784215da8c1d6c34af5306f85&tochange=742051417634b357b311c389368ac1e378770f32

Comment 4

[Dianna Smith [:diannaS]]

Might be related to bug Bug 1973033

Comment 5

[Daniel Veditz [:dveditz]]

This assertion was added in bug 1976826, checked in at the same time as 1973033

Jon: is this more of a "warning" assert (there's a TODO comment here) or is it a security problem?

Comment 6

[Christian Holler (:decoder)]

I do have evidence that this causes more than just an assert. I have a case of a hard to reduce test here that intermittently triggers an UBSan "load as bool" failure - that same test causes this assert in debug builds. The described UBSan failure is usually indicating the use of uninitialized memory or corruption of some kind (a part of memory is interpreted as an explicit bool but contains a random value).

Comment 7

[Jon Coppeard (:jonco)]

My current understanding is that this assertion failure will not cause any problems but I don't understand how it would cause the UBSan issue in comment 6. So let's leave this as a security bug for now.

Comment 8

[Jon Coppeard (:jonco)]

Attachment: (secure)

Bug 1976826 changed the buffer allocator realloc API to require the previous
allocated size as well as the new size. This testcase is failing because we're
passing the wrong size for an array where the length property has been made
non-writeable. This is because we shrink the recorded capacity in this case.

The patch checks for this case and calculates the original size.

Comment 9

[Jon Coppeard (:jonco)]

Attachment: (secure)

These bitmaps are modified on a background thread while sweeping. This may
clear the allocation end bit for swept allocations. We want to allow the main
thread to find the allocation end bit for live allocations so we need to make
the storage relaxed atom to avoid undefined behaviour.

It is not possible for the background thread to modify the bit the main thread
is searching for as the allocation must be live if itis referenced from the
main thread.

Comment 10

[Jon Coppeard (:jonco)]

Attachment: (secure)

This backs out commit 1d424ecf7f2c2de2b3a569ee15dd1888c7cf76ab.

Since the allocation end bitmaps are atomic we can relax this restriction and
allow getAllocSize during sweeping.

This fixes bug 1979584 and bug 1979525.

Comment 11

[Jon Coppeard (:jonco)]

Attachment: (secure)

This backs out commit f962a998941539d1f88e1a1e16f24461d0f5ba9f.

The previous changes remove the need to pass the old size into realloc. It
turns out it's complicated to track this exactly. It's more robust to not
require this which is possible now we can always look it up.

Comment 12

[Jon Coppeard (:jonco)]

Attachment: (secure)

Comment 15

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox143 (nightly). However, the bug still has low severity.

:sdetar, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8144fa5c74fe
https://hg.mozilla.org/integration/autoland/rev/168d2240ed0e
Part 1: Make buffer allocator alloc end bitmaps atomic r=sfink
https://github.com/mozilla-firefox/firefox/commit/1e4b6e491642
https://hg.mozilla.org/integration/autoland/rev/ebb10a85ecae
Part 2: Back out: Bug 1976826 - Part 5: Don't allow getAllocSize during sweeping r=sfink
https://github.com/mozilla-firefox/firefox/commit/adcd1f103138
https://hg.mozilla.org/integration/autoland/rev/394c34498758
Part 3: Back out: Bug 1976826 - Part 4: Pass old size to realloc methods r=sfink
https://github.com/mozilla-firefox/firefox/commit/cf67f41d062c
https://hg.mozilla.org/integration/autoland/rev/f20b3e55fd17
Part 4: Add testcase r=sfink

Comment 17

[Christian Holler (:decoder)]

Here's yet another indicator that this is possibly an out-of-bounds:

[1939628] Hit MOZ_CRASH(ElementAt(aIndex = 17, aLength = 17)) at mfbt/Assertions.cpp:78
==1939628==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x5579f9d51b35 bp 0x7ffc5524f390 sp 0x7ffc5524f380 T0)
==1939628==The signal is caused by a WRITE memory access.
==1939628==Hint: address points to the zero page.
/home/worker/builds/mc-64-opt-asan/dist/bin/llvm-symbolizer: error: '[anon:js-executable-memory]': No such file or directory
    #0 0x5579f9d51b35 in MOZ_CrashSequence dist/include/mozilla/Assertions.h:248:3
    #1 0x5579f9d51b35 in MOZ_Crash dist/include/mozilla/Assertions.h:381:3
    #2 0x5579f9d51b35 in mozilla::detail::InvalidArrayIndex_CRASH(unsigned long, unsigned long) mfbt/Assertions.cpp:77:3
    #3 0x5579fad49cbe in operator[] dist/include/mozilla/Array.h:44:7
    #4 0x5579fad49cbe in remove js/src/gc/BufferAllocator.cpp:541:3
    #5 0x5579fad49cbe in js::gc::BufferAllocator::updateFreeRegionStart(js::gc::BufferAllocator::FreeLists*, js::gc::BufferAllocator::FreeRegion*, unsigned long, js::gc::BufferAllocator::SizeKind) js/src/gc/BufferAllocator.cpp:3351:16
    #6 0x5579fad39a86 in js::gc::BufferAllocator::freeMedium(void*) js/src/gc/BufferAllocator.cpp:3237:5
    #7 0x5579fad37b10 in free js/src/gc/BufferAllocator.cpp:1206:5
    #8 0x5579fad37b10 in operator() js/src/gc/BufferAllocator.cpp:1180:51
    #9 0x5579fad37b10 in ~ScopeExit dist/include/mozilla/ScopeExit.h:106:7
    #10 0x5579fad37b10 in js::gc::BufferAllocator::realloc(void*, unsigned long, unsigned long, bool) js/src/gc/BufferAllocator.cpp:1195:1
    #11 0x5579fa36178e in ReallocateCellBuffer<js::HeapSlot> js/src/gc/Nursery-inl.h:376:15
    #12 0x5579fa36178e in ReallocateCellBuffer<js::HeapSlot> js/src/gc/Nursery-inl.h:384:15
    #13 0x5579fa36178e in js::NativeObject::shrinkElements(JSContext*, unsigned int) js/src/vm/NativeObject.cpp:1037:30
    #14 0x5579fa35ca9b in js::NativeObject::shrinkCapacityToInitializedLength(JSContext*) js/src/vm/NativeObject.cpp:1069:3
    #15 0x5579f9f77e25 in DeleteArrayElement js/src/builtin/Array.cpp:525:17
    #16 0x5579f9f77e25 in DeletePropertyOrThrow(JSContext*, JS::Handle<JSObject*>, unsigned long) js/src/builtin/Array.cpp:549:8
    #17 0x5579f9fc7aef in DeletePropertiesOrThrow(JSContext*, JS::Handle<JSObject*>, unsigned long, unsigned long) js/src/builtin/Array.cpp:578:10
    #18 0x5579f9fc6bf1 in array_splice_impl(JSContext*, unsigned int, JS::Value*, bool) js/src/builtin/Array.cpp:3331:12
    #19 0x5579f9f14137 in CallJSNative js/src/vm/Interpreter.cpp:501:13
    #20 0x5579f9f14137 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) js/src/vm/Interpreter.cpp:597:12
    #21 0x5579f9f15f51 in InternalCall js/src/vm/Interpreter.cpp:664:10
    #22 0x5579f9f15f51 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) js/src/vm/Interpreter.cpp:696:8
    #23 0x5579fa1ecb88 in js::fun_call(JSContext*, unsigned int, JS::Value*) js/src/vm/JSFunction.cpp:1109:10
    #24 0x5579f9f14137 in CallJSNative js/src/vm/Interpreter.cpp:501:13
    #25 0x5579f9f14137 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) js/src/vm/Interpreter.cpp:597:12
    #26 0x5579faffc6e9 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICFallbackStub*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/jit/BaselineIC.cpp:1705:10
    #27 0x32846230c8c3  ([anon:js-executable-memory]+0x28c3)

Comment 18

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

https://hg.mozilla.org/mozilla-central/rev/168d2240ed0e
https://hg.mozilla.org/mozilla-central/rev/ebb10a85ecae
https://hg.mozilla.org/mozilla-central/rev/394c34498758
https://hg.mozilla.org/mozilla-central/rev/f20b3e55fd17

Comment 20

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20250805210924-2d8353914a6b.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.