1784116 - ArrayBuffers can contain data in the wrong malloc arena

Status: NEW

(Core :: JavaScript Engine, defect, P3)

Description

[Steve Fink [:sfink] [:s:]]

It looks like the right way to allocate memory that will end up held in an ArrayBuffer is js_pod_arena_malloc<char>(js::ArrayBufferContentsArena, size), which is bleeding internal details all over. It would better to provide JS::MallocForArrayBuffer(size) or perhaps JS::MallocForArrayBuffer(cx, size) to encapsulate this, and encourage calls of JS::NewArrayBufferWithContents to allocate the memory that way.

(Even better would be to able to assert when freeing ArrayBuffer data that it is in the correct arena.)

Comment 1

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 1: Add JSAPI functions to allocate in ArrayBufferContentsArena. r=sfink!

This uses the suggested names from https://bugzilla.mozilla.org/show_bug.cgi?id=1784116#c0,
therefore it doesn't match the existing JS_string_malloc function. (JS_string_malloc is
unused, so we could also rename it to JS::MallocForString.)

Comment 2

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 2: Update JS_malloc to JS::MallocForArrayBuffer. r=sfink!

Update JS_malloc to instead use JS::MallocForArrayBuffer.

Depends on D198800

Comment 3

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 3: Update js_realloc to js_arena_realloc. r=sfink!

Keep using the internal API instead of JS::ReallocForArrayBuffer to minimise
the changes.

Depends on D198802

Comment 4

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 4: Update js_calloc to js_arena_calloc. r=sfink!

Keep using the internal API to minimise the changes.

Depends on D198804

Comment 5

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 5: Update js_pod_malloc to js_pod_arena_malloc. r=sfink!

Keep using the internal API to minimise the changes.

Depends on D198806

Comment 6

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 6: Update remaining NewArrayBufferWithContents callers to use the ArrayBufferContentsArena. r=sfink!

dom/indexedDB/test/gtest/TestKey:

• Call JS::MallocForArrayBuffer instead of using strdup to allocate memory
  for an ArrayBuffer.

dom/media/webcodecs/EncoderTemplate:

• Call JS::MallocForArrayBuffer instead of new[] to allocate memory for an
  ArrayBuffer.
• Out-of-memory is ignored because the existing code already ignores OOM, for
  example the result of JS::NewArrayBufferWithContents is not checked.

dom/media/webrtc/jsapi:

• Use js_pod_arena_malloc instead of using new[] (through MakeUnique).
• Update the UniquePtr to use the correct deleter (JS::FreePolicy).

dom/serviceworkers/ServiceWorkerEvents:

• Call JS::MallocForArrayBuffer instead of malloc when allocating memory
  for an ArrayBuffer.

Depends on D198807

Comment 7

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 7: Copy data in CacheEntry shell function. r=sfink!

JS::TranscodeBuffer allocates in the default arena, so we have to copy the
data instead of using BufferContents::createMalloced.

Depends on D198809

Comment 8

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 8: Assert malloced buffer contents are in the correct arena. r=sfink!

Parts 1-7 ensure the buffer contents are allocated in the correct arena, so we
can now assert this, too. Also see js::AssertJSStringBufferInCorrectArena for
a similar check for JSString memory.

Depends on D198810

Comment 9

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part 9: Allocate SharedArrayBuffers in ArrayBufferContentsArena. r=sfink!

SharedArrayBuffers should also be allocated in a separate arena.

Depends on D198811

Comment 10

[André Bargull [:anba]]

Attachment: WIP: Bug 1784116 - Part ?: Test alloc policy.

This change is missing in parts 1-7. This approach allocates all StreamLoader
memory in js::ArrayBufferContentsArena, which isn't really what we want to
do. I guess we need a separate StreamLoader which uses the correct jemalloc
arena, so that mozilla::dom::BodyConsumer can use that new StreamLoader when
mozilla::dom::BodyConsumer::mConsumeType is
mozilla::dom::BodyConsumer::CONSUME_ARRAYBUFFER.

Depends on D198812