2010315 - Don't generate wasm traps if load/store is outside reserved guard regions

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, task, P3)

Description

[Ryan Hunt [:rhunt]]

The wasm signal handler only checks if a faulting instruction is:

1. in wasm code
2. at an intentionally marked 'TrapSite' PC

So if we have a wasm load/store that somehow accesses memory outside of the reserved wasm memory and hits an access violation, we convert that into wasm trap. This obscures real bugs and prevents fuzzers from finding them.

We should see if we can use the effective address to filter out bad loads. One difficult part of this is that we need to somehow find the valid regions of wasm memory. I think we could use the wasm::Instance and iterate over the wasm memory objects. Wasm-GC would be harder, so probably won't start with that.

The other difficult part is getting the effective address. We have 3 cases:

1. Win32 VectoredExceptionHandling - ExceptionInfo->ExceptionRecord->ExceptionInformation[1] holds the effective address
2. Mach Exception Handlers - [1] request.body.code[1]
3. POSIX signals - si_addr holds the address [2]

[1] https://searchfox.org/firefox-main/rev/5917a9f2af3294b27a325371c5c499e7dd9554fd/js/src/wasm/WasmSignalHandlers.cpp#617
[2] https://man7.org/linux/man-pages/man2/sigaction.2.html

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: (secure)

Comment 2

[Ryan Hunt [:rhunt]]

The patch fixes this and also bug 1818639. I need to test it more, and on other platforms before requesting review.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a8ad25509c8f
https://hg.mozilla.org/integration/autoland/rev/07dc477b4b48
Generate better error messages for out of bounds. r=bvisness

Comment 4

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

https://hg.mozilla.org/mozilla-central/rev/07dc477b4b48

Comment 5

[Florin Bilt]

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #4)

https://hg.mozilla.org/mozilla-central/rev/07dc477b4b48

Perfherder has detected a browsertime performance change from push 07dc477b4b4841f21501709ee46a3194716ec9e4.

No action is required from the author; this comment is provided for informational purposes only.

Improvement	Test	Platform	Options	Absolute values [old vs new]	Performance Profiles
5%	jetstream3 cdjs-Average (doc)	macosx1500-aarch64-shippable	fission webrender	27.38 ms -> 25.94 ms

Need Help or Information?

If you have any questions, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.