1891412 - Assertion failure: isDouble(), at dist/include/js/Value.h:1070

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

newGlobal({ newCompartment: true }).Debugger(this).memory.trackingAllocationSites = true
for (let i = 0; i < 9; i++) {
  oomTest(function () {
    class C extends WebAssembly.Memory {}
    new C({
      initial: 0,
      maximum: 1,
      shared: 1,
    })
  })
}

(gdb) bt
#0  JS::Value::toPrivate (this=0x12eb36288468) at /home/ubumain/shell-cache/js-dbg-64-linux-x86_64-1dc823d4e96d/objdir-js/dist/include/js/Value.h:1070
#1  js::SharedArrayBufferObject::byteLengthOrMaxByteLength (this=0x12eb36288448) at /home/ubumain/trees/mozilla-central/js/src/vm/SharedArrayObject.h:351
#2  0x00005555576b4f98 in js::SharedArrayBufferObject::addSizeOfExcludingThis (obj=0x12eb36288448, mallocSizeOf=<optimized out>, info=0x7fffffffb3c0, runtimeSizes=0x0) at /home/ubumain/trees/mozilla-central/js/src/vm/SharedArrayObject.cpp:642
#3  0x0000555557515b88 in JS::ubi::Concrete<JSObject>::size (this=<optimized out>, mallocSizeOf=0x0) at /home/ubumain/trees/mozilla-central/js/src/vm/JSObject.cpp:3321
#4  0x00005555579b76f8 in JS::ubi::Node::size (this=0x7fffffffb480, mallocSizeof=0x0) at /home/ubumain/shell-cache/js-dbg-64-linux-x86_64-1dc823d4e96d/objdir-js/dist/include/js/UbiNode.h:817
/snip

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev 1dc823d4e96d.

This goes as far back as m-c rev 443c7bf9d76b (August 2023), will try and go back further.

Setting s-s as a start.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The testcase does not reproduce with the latest debug js shell from FTP (2015-10-21) but reproduces with m-c rev a5887514ddfb (Feb 2022).

I'm going to take a guess - since this has Debugger, I'll set a needinfo? for Arai-san to take a look as a start.

Comment 2

[Tooru Fujisawa [:arai]]

This comes from SharedArrayBufferObject::addSizeOfExcludingThis not being compatible with partially-initialized object.

Here's what's going on:

in SharedArrayBufferObject::createFromNewRawBuffer, it allocates AutoSetNewObjectMetadata and then allocates an object, which ultimately calls JS::Realm::setObjectPendingMetadata, given SharedArrayBufferObject has JSCLASS_DELAY_METADATA_BUILDER flag.

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/SharedArrayObject.cpp#671-673,675-676,684

SharedArrayBufferObject* SharedArrayBufferObject::createFromNewRawBuffer(
    JSContext* cx, WasmSharedArrayRawBuffer* buffer, size_t initialSize) {
  MOZ_ASSERT(cx->realm()->creationOptions().getSharedMemoryAndAtomicsEnabled());
...
  AutoSetNewObjectMetadata metadata(cx);
  auto* obj = NewBuiltinClassInstance<FixedLengthSharedArrayBufferObject>(cx);
...
  if (!obj->acceptRawBuffer(buffer, initialSize)) {

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/NativeObject-inl.h#480-483,513-515

/* static */
inline NativeObject* NativeObject::create(
    JSContext* cx, js::gc::AllocKind kind, js::gc::Heap heap,
    js::Handle<SharedShape*> shape, js::gc::AllocSite* site /* = nullptr */) {
...
  if (MOZ_UNLIKELY(cx->realm()->hasAllocationMetadataBuilder())) {
    if (clasp->shouldDelayMetadataBuilder()) {
      cx->realm()->setObjectPendingMetadata(nobj);

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/SharedArrayObject.cpp#763-765,774-776

const JSClass FixedLengthSharedArrayBufferObject::class_ = {
    "SharedArrayBuffer",
    JSCLASS_DELAY_METADATA_BUILDER |
...
const JSClass GrowableSharedArrayBufferObject::class_ = {
    "SharedArrayBuffer",
    JSCLASS_DELAY_METADATA_BUILDER |

Then, js::SharedArrayBufferObject::LENGTH_SLOT slot is initialized js::SharedArrayBufferObject::acceptRawBuffer, but only if addSharedMemory doesn't fail.

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/SharedArrayObject.cpp#588-594,597

bool SharedArrayBufferObject::acceptRawBuffer(SharedArrayRawBuffer* buffer,
                                              size_t length) {
  if (!zone()->addSharedMemory(buffer,
                               SharedArrayMappedSize(buffer->isWasm(), length),
                               MemoryUse::SharedArrayRawBuffer)) {
    return false;
  }
...
  setFixedSlot(LENGTH_SLOT, PrivateValue(length));

So, if addSharedMemory fails, the slot is left uninitialized, and AutoSetNewObjectMetadata::~AutoSetNewObjectMetadata grabs the partially-initialized SharedArrayRawBuffer object and tries to create metadata from it, and it tries to read LENGTH_SLOT slot value, assuming it's already initialized.

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/SharedArrayObject.cpp#632-634,642

void SharedArrayBufferObject::addSizeOfExcludingThis(
    JSObject* obj, mozilla::MallocSizeOf mallocSizeOf, JS::ClassInfo* info,
    JS::RuntimeSizes* runtimeSizes) {
...
  size_t nbytes = buf.byteLengthOrMaxByteLength();

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/SharedArrayObject.h#350-351

size_t byteLengthOrMaxByteLength() const {
  return size_t(getFixedSlot(LENGTH_SLOT).toPrivate());

So, possible solution is either:

• (a) reset the pending metadata build on failure
• (b) make the metadata building more robust

and in this case, I think (b), especially making SharedArrayBufferObject::addSizeOfExcludingThis more robust, is better.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1891412 - Handle uninitialized object in SharedArrayBufferObject::addSizeOfExcludingThis. r?jonco!

Comment 4

[Tooru Fujisawa [:arai]]

Then, LENGTH_SLOT slot value is just an integer, and in the failure case, it's used only as a value reported to memory consumption.
Also, if it's not initialized, RAWBUF_SLOT slot value is also undefined, and that's dereferenced as SharedArrayRawBuffer pointer.
on 32-bit, interpreting undefined value as raw pointer results in nullptr, and
on 64-bit, it becomes invalid pointer, given higher bits are set, and immediately crashes, e.g. with EXC_BAD_ACCESS.
So I assume this isn't exploitable, except as DoS.

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/SharedArrayObject.cpp#610-613

SharedArrayRawBuffer* SharedArrayBufferObject::rawBufferObject() const {
  Value v = getFixedSlot(RAWBUF_SLOT);
  MOZ_ASSERT(!v.isUndefined());
  return reinterpret_cast<SharedArrayRawBuffer*>(v.toPrivate());

https://searchfox.org/mozilla-central/rev/d23849dd6d83edbe681d3b4828700256ea34a654/js/src/vm/SharedArrayObject.cpp#632-634,643

void SharedArrayBufferObject::addSizeOfExcludingThis(
    JSObject* obj, mozilla::MallocSizeOf mallocSizeOf, JS::ClassInfo* info,
    JS::RuntimeSizes* runtimeSizes) {
...
  size_t owned = nbytes / buf.rawBufferObject()->refcount();

Comment 5

[Tooru Fujisawa [:arai]]

Given this is not exploitable, this bug can be opened up

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a4588287c80
Handle uninitialized object in SharedArrayBufferObject::addSizeOfExcludingThis. r=jonco

Comment 7

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

Backed out for causing sm bustages:
https://hg.mozilla.org/integration/autoland/rev/2949cb403e680d76f7bcb101117ceefeb480b064

Push with failures
Failure log

TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/sharedbuf/size-with-uninitialized.js | [11229] Hit MOZ_CRASH(Thunk execution failed but no exception was raised - missing call to js::ReportOutOfMemory()?) at /builds/worker/checkouts/gecko/js/src/builtin/TestingFunctions.cpp:4161 (code -11, args "--baseline-eager --write-protect-code=off") [0.1 s]

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e146830d19d
Handle uninitialized object in SharedArrayBufferObject::addSizeOfExcludingThis. r=jonco

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/4e146830d19d