2001452 - AddressSanitizer: heap-use-after-free involving js::WaitAsyncTimeoutTask::clear with js::AsyncFutexWaiter::maybeClearTimeout on the stack

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

Attachment: debug stack

function f() {
  var x = new Int32Array(new SharedArrayBuffer(4));
  x[0] = 1;
  Atomics.waitAsync(x, 0, 1, 65535);
  oomTest(f);
}
f();

==334736==ERROR: AddressSanitizer: heap-use-after-free on address 0x767678de99e0 at pc 0x5c438f423f4a bp 0x7ffc9c8bb750 sp 0x7ffc9c8bb748
WRITE of size 8 at 0x767678de99e0 thread T0
    #0 0x5c438f423f49 in js::WaitAsyncTimeoutTask::clear(js::AutoLockFutexAPI&) /home/msf2/trees/firefox/js/src/builtin/AtomicsObject.cpp:885:43
    #1 0x5c438f423f49 in js::AsyncFutexWaiter::maybeClearTimeout(js::AutoLockFutexAPI&) /home/msf2/trees/firefox/js/src/builtin/AtomicsObject.cpp:1047:19
    #2 0x5c438f423f49 in js::WaitAsyncNotifyTask::prepareForCancel() /home/msf2/trees/firefox/js/src/builtin/AtomicsObject.cpp:1006:11
    #3 0x5c438fa4b1d6 in js::OffThreadPromiseTask::DestroyUndispatchedTask(js::OffThreadPromiseTask*, js::OffThreadPromiseRuntimeState&, js::AutoLockHelperThreadState const&) /home/msf2/trees/firefox/js/src/vm/OffThreadPromiseRuntimeState.cpp:166:9
    #4 0x5c438fa4f01c in js::OffThreadPromiseRuntimeState::shutdown(JSContext*) /home/msf2/trees/firefox/js/src/vm/OffThreadPromiseRuntimeState.cpp:457:5
    #5 0x5c438f7c6e4b in js::DestroyContext(JSContext*) /home/msf2/trees/firefox/js/src/vm/JSContext.cpp:231:35
/snip

42652110128d-587084
42652110128d48bd7f950f2de56628e822aeb9e5 is the first interesting commit
commit 42652110128d48bd7f950f2de56628e822aeb9e5
Author: Daniel Minor
Date:   Tue Sep 16 20:10:59 2025 +0000

    Bug 1884148 - Ship the Atomics.waitAsync proposal; r=mgaudet

    Differential Revision: https://phabricator.services.mozilla.com/D258612

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with AR=ar sh ~/trees/firefox/js/src/configure --enable-debug --enable-fuzzing --without-sysroot --enable-address-sanitizer --disable-jemalloc --disable-stdcxx-compat --enable-undefined-sanitizer --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on gh rev 70425199e9a5fa80aa7419fa51763013a67226e1.

Unsure if Dan is the right person to needinfo, so setting needinfo? from Matt for now. Matt, is bug 1884148 a likely regressor?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1884148

Comment 2

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

Running with --fuzzing-safe --no-threads --no-baseline --no-ion --setpref=atomics_wait_async=true causes this possible regressor to show up:

ec973761bca5-568571
ec973761bca508c97cd226d18bae1d37e0fc32b8 is the first interesting commit
commit ec973761bca508c97cd226d18bae1d37e0fc32b8
Author: Iain Ireland
Date:   Tue May 13 10:01:47 2025 +0000

    Bug 1467846: Part 9: Implement waitAsync r=arai

    This implements Atomics.waitAsync, excluding cases with non-zero timeouts. Most of the rest of this patch stack is dedicated to adding timeout support.

    Breaking out the critical section into its own function made locking easier.

    Depends on D212874

    Differential Revision: https://phabricator.services.mozilla.com/D212875

Also setting needinfo? for Iain, please feel free to pass it on if needed.

Comment 3

[Iain Ireland [:iain]]

This is shell-only.

If we trigger an OOM here while trying to enqueue a WaitAsyncTimeoutTask on the shell's internal queue, we will end up deleting the task in WaitAsyncTask::transferToRuntime. But we already added the task to the cancellable list here, before dispatching the task. During context shutdown we iterate through all the cancellable WaitAsyncNotifyTasks and call prepareToCancel on each of them. One of them is pointing to the freed timeout task, and we null out the waiter_ pointer of the already freed memory.

One option is to just use AutoEnterOOMUnsafeRegion instead of trying to recover from an inability to allocate space in the priority queue. However, it seems like this is the precise sort of thing that the whole transferToRuntime mechanism was intended to address. Arai, do you have thoughts about what went wrong here?

Comment 4

[Tooru Fujisawa (away) [:arai]]

Given that each of instance manages the lifetime on its own, with raw pointers to each other, they should manage the incoming references and cut them whenever getting deleted.
So, in this case, timeoutTask is responsible for clearing the waiter's reference.
Same applies to other cases if there's any missing such code.

Of course some cases could be redundant, given that both instances may get deleted at the same time, but I assume the destruction of those instances aren't too hot, and properly clearing those raw pointers in all cases is safer.

Comment 5

[Tooru Fujisawa (away) [:arai]]

Attachment: (secure)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7d5ae0c50c49
https://hg.mozilla.org/integration/autoland/rev/3f0d167c4df2
Clear the raw pointer references when AsyncFutexWaiter/WaitAsyncNotifyTask/WaitAsyncTimeoutTask are getting deleted. r=iain

Comment 7

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/mozilla-central/rev/3f0d167c4df2

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:arai, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox146 to wontfix.

For more information, please visit BugBot documentation.

Comment 9

[Tooru Fujisawa (away) [:arai]]

Given this is specific issue is shell-only, we don't need uplift.