1914963 - Segmentation fault with wasm tail calls on aarch64

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Yury Delendik (:yury)]

Similar to bug 1913445 but involves non-trampoline code for wasm tail calls: the constants pool may be injected between call and (slow call) marker. I did not find confirmation in form of a bug/crash, but expecting that to be a problem. The refactoring might involve touching all platforms, so it will be nice to do incremental (non-uplift) patching.

Comment 1

[Yury Delendik (:yury)]

Attachment: Bug 1914963 - Combine wasmMarkSlowCall and call(reg). r?jseward

Comment 2

[Daniel Veditz [:dveditz]]

What versions are affected? Is it the same as for bug 1913445?

Comment 3

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

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:yury, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Comment 4

[Yury Delendik (:yury)]

What versions are affected? Is it the same as for bug 1913445?

That's correct. Also, tail calls were enabled in FF121.

Comment 5

[Yury Delendik (:yury)]

Attachment: test case that is fixed by this patch.

Here is test that is found by wasm-calls.js

Comment 6

[Yury Delendik (:yury)]

Crash signature:

UndefinedBehaviorSanitizer:DEADLYSIGNAL
==80065==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0xf94012e8f94003a5 (pc 0x28f9f78c446c bp 0x00016f2c5c40 sp 0x00016f2c5bf0 T17123598)
==80065==The signal is caused by a WRITE memory access.
    #0 0x28f9f78c446c  (<unknown module>)
    #1 0x28f9f78c4644  (<unknown module>)
    #2 0x1027099a8 in js::wasm::Instance::callExport(JSContext*, unsigned int, JS::CallArgs const&, js::wasm::CoercionLevel)+0xad0 (js:arm64+0x101bd19a8)
    #3 0x1027086b4 in WasmCall(JSContext*, unsigned int, JS::Value*)+0x18c (js:arm64+0x101bd06b4)
    #4 0x100c254e0 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&)+0x118 (js:arm64+0x1000ed4e0)
    #5 0x100c24bec in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)+0x2a0 (js:arm64+0x1000ecbec)
    #6 0x100c3345c in js::Interpret(JSContext*, js::RunState&)+0x8e9c (js:arm64+0x1000fb45c)

Comment 7

[Yury Delendik (:yury)]

Other possible signature:

[63075] Assertion failure: code_ == &instance()->code(), at /Users/yury/Work/mozilla-unified/js/src/wasm/WasmFrameIter.cpp:307
#01: js::wasm::WasmFrameIter::popFrame()[/Users/yury/Work/mozilla-unified/obj-aarch64-apple-darwin23.5.0/dist/bin/js +0x1b58d08]
#02: js::JitFrameIter::operator++()[/Users/yury/Work/mozilla-unified/obj-aarch64-apple-darwin23.5.0/dist/bin/js +0x48343c]
#03: js::jit::TraceJitActivations(JSContext*, JSTracer*)[/Users/yury/Work/mozilla-unified/obj-aarch64-apple-darwin23.5.0/dist/bin/js +0x138b70c]
#04: js::gc::GCRuntime::traceRuntimeCommon(JSTracer*, js::gc::GCRuntime::TraceOrMarkRuntime)[/Users/yury/Work/mozi

Comment 8

[Yury Delendik (:yury)]

The uplifting will require the applied bug 1913445 patch.

Comment 9

[Yury Delendik (:yury)]

Comment on attachment 9420826 [details]
Bug 1914963 - Combine wasmMarkSlowCall and call(reg). r?jseward

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Wasm programs can crash/corrupt memory on ARM(64) machines
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: since ff121
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: it depends on bug 1913445
• How likely is this patch to cause regressions; how much testing does it need?:
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 10

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 9420826 [details]
Bug 1914963 - Combine wasmMarkSlowCall and call(reg). r?jseward

Approved to land and request uplift

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/506a1f94f7a1
Combine wasmMarkSlowCall and call(reg). r=jseward

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/506a1f94f7a1

Comment 13

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

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox131 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Yury Delendik (:yury)]

Comment on attachment 9420826 [details]
Bug 1914963 - Combine wasmMarkSlowCall and call(reg). r?jseward

Beta/Release Uplift Approval Request

• User impact if declined: Some wasm programs with tail calls may crash on aarch64
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Affects only wasm programs and aarch64
• String changes made/needed:
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: Some wasm programs with tail calls may crash on aarch64
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Affects only wasm programs and aarch64

Comment 15

[Dianna Smith [:diannaS]]

Comment on attachment 9420826 [details]
Bug 1914963 - Combine wasmMarkSlowCall and call(reg). r?jseward

Approved for 131.0b6

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/904af4d2b8ef

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/ba89946f61c4

Comment 18

[Dianna Smith [:diannaS]]

Comment on attachment 9420826 [details]
Bug 1914963 - Combine wasmMarkSlowCall and call(reg). r?jseward

Approved for 128.3esr