1747586 - Windows 32-bit WOFF2 Crash [@ moz_wasm2c_trap_handler]

Status: VERIFIED FIXED

(Core :: Security: RLBox, defect)

Description

[Bobby Holley (:bholley)]

Forked off of bug 1744460 comment 19.

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #19)

I reproduced the crash with the signature reported here [@ w2c_memset] using old Nightly build https://crash-stats.mozilla.org/report/index/6f90c406-ce87-4bc5-94f8-99e9a0211221.
Using latest Nightly (97.0a1) and Beta (96.0b7) builds I still get a crash using steps from comment 0 but with a different crash signature [@ moz_wasm2c_trap_handler], not sure if they are related or not. Shravanrn could you please take a look at these crashes?

https://crash-stats.mozilla.org/report/index/1ac73c92-cb61-4c29-b9a3-cd0070211221 - Nightly
https://crash-stats.mozilla.org/report/index/ee216d5b-9c20-4455-8227-bcb8f0211221 - Beta

Comment 1

[Shravan Narayan]

The source of the issue is a malloc that fails inside woff2 sandboxed code. The malloc result inside woff2 is not checked and so further memory ops (performed on offsets on this null pointer) corrupt the insides of the sandbox. So, we need to be more generous with the sizing of the woff2 sandbox

We are currently setting the size to 1.33 * (aLength + expectedSize)
When using 2* (aLength + expectedSize) and the sandbox memory appears sufficient for this use case.
I'll submit a patch with this fix shortly

We have added a crash reporter annotation that tracks this, so this is an easy way to see if this bug occurs again going forward
https://searchfox.org/mozilla-central/source/config/external/rlbox_wasm2c_sandbox/rlbox_wasm2c_thread_locals.cpp#39

One orthogonal issue here is that this crash only appears on builds from try, and not local builds. I am not immediately sure why this is the case. But somehow the inference from these behaviors is that malloc is somehow different on local builds and try builds. The sandboxed code uses dlmalloc to implement its memory allocation on a pool of memory, so this should not be inconsistent across the two builds.

Comment 2

[Shravan Narayan]

Attachment: Bug 1747586 - Increase woff2 rlbox sbx size to prevent win32 crash r=bholley

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/885f824b309f
Increase woff2 rlbox sbx size to prevent win32 crash r=bholley

Comment 4

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/885f824b309f

Comment 5

[Bobby Holley (:bholley)]

Comment on attachment 9256894 [details]
Bug 1747586 - Increase woff2 rlbox sbx size to prevent win32 crash r=bholley

Beta/Release Uplift Approval Request

• User impact if declined: Crashes.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• 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): Extremely low-risk patch. Just bumps a pre-existing safety margin from 1.3x to 2x to eliminate some edge-case OOMs.
• String changes made/needed:

Comment 6

[Bogdan Maris, Desktop Test Engineering]

I'm not getting the crash anymore with the latest Nightly 97.0a1 from treeherder.

Comment 7

[Dianna Smith [:diannaS]]

Comment on attachment 9256894 [details]
Bug 1747586 - Increase woff2 rlbox sbx size to prevent win32 crash r=bholley

Approved for 96.0b10

Comment 8

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d7fbaeec45c4

Comment 9

[Bogdan Maris, Desktop Test Engineering]

Also verified that this is fixed using 96.0b10, no crashes recorded.