1707642 - Hit MOZ_CRASH(IPC message size is too large) while fuzzing

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.zip

Recently, we've been seeing a large number of unsymbolized crashes. Most of these testcases involve creating a large buffer via AudioContext. However, a bisection reveals that these changes were introduced in the following build range (bug 1691886 or bug 1703505).

Start: e63ed2ba5c128314b9206b7e137f079c5d1ffab4 (20210412155838)
End: 76f9bff99e53a919cfa139cc72fbbcbae0d2e6e7 (20210412160428)
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e63ed2ba5c128314b9206b7e137f079c5d1ffab4&tochange=76f9bff99e53a919cfa139cc72fbbcbae0d2e6e7

The following testcase was found while fuzzing mozilla-central rev 289e41464376 (built with --enable-address-sanitizer --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build 289e41464376 --asan --fuzzing -n build
$ python -m grizzly.replay --xvfb ./build/firefox ./testcase.zip

Hit MOZ_CRASH(IPC message size is too large) at /builds/worker/checkouts/gecko/ipc/glue/MessageLink.cpp:143

==2642756==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7fa0bc39a918 bp 0x7ffed3052bf0 sp 0x7ffed3052b00 T0)
==2642756==The signal is caused by a WRITE memory access.
==2642756==Hint: address points to the zero page.
==2642756==WARNING: failed to fork (errno 12)
==2642756==WARNING: failed to fork (errno 12)
==2642756==WARNING: failed to fork (errno 12)
==2642756==WARNING: failed to fork (errno 12)
==2642756==WARNING: failed to fork (errno 12)
==2642756==WARNING: Failed to use and restart external symbolizer!
    #0 0x7fa0bc39a918  (/home/jkratzer/builds/mc-asan/libxul.so+0x62e7918)
    #1 0x7fa0bc3866ed  (/home/jkratzer/builds/mc-asan/libxul.so+0x62d36ed)
    #2 0x7fa0bc384bb6  (/home/jkratzer/builds/mc-asan/libxul.so+0x62d1bb6)
    #3 0x7fa0bc3a76ba  (/home/jkratzer/builds/mc-asan/libxul.so+0x62f46ba)
    #4 0x7fa0bc970738  (/home/jkratzer/builds/mc-asan/libxul.so+0x68bd738)
    #5 0x7fa0c23b2215  (/home/jkratzer/builds/mc-asan/libxul.so+0xc2ff215)
    #6 0x7fa0c23b3594  (/home/jkratzer/builds/mc-asan/libxul.so+0xc300594)
    #7 0x7fa0bc971663  (/home/jkratzer/builds/mc-asan/libxul.so+0x68be663)
    #8 0x7fa0bc8fb622  (/home/jkratzer/builds/mc-asan/libxul.so+0x6848622)
    #9 0x7fa0bc3943ba  (/home/jkratzer/builds/mc-asan/libxul.so+0x62e13ba)
    #10 0x7fa0bc390a6e  (/home/jkratzer/builds/mc-asan/libxul.so+0x62dda6e)
    #11 0x7fa0bc392428  (/home/jkratzer/builds/mc-asan/libxul.so+0x62df428)
    #12 0x7fa0bc392f8b  (/home/jkratzer/builds/mc-asan/libxul.so+0x62dff8b)
    #13 0x7fa0bb179f8a  (/home/jkratzer/builds/mc-asan/libxul.so+0x50c6f8a)
    #14 0x7fa0bb1464f0  (/home/jkratzer/builds/mc-asan/libxul.so+0x50934f0)
    #15 0x7fa0bb144027  (/home/jkratzer/builds/mc-asan/libxul.so+0x5091027)
    #16 0x7fa0bb14447d  (/home/jkratzer/builds/mc-asan/libxul.so+0x509147d)
    #17 0x7fa0bb183071  (/home/jkratzer/builds/mc-asan/libxul.so+0x50d0071)
    #18 0x7fa0bb161163  (/home/jkratzer/builds/mc-asan/libxul.so+0x50ae163)
    #19 0x7fa0bb16c0ec  (/home/jkratzer/builds/mc-asan/libxul.so+0x50b90ec)
    #20 0x7fa0bc39bcef  (/home/jkratzer/builds/mc-asan/libxul.so+0x62e8cef)
    #21 0x7fa0bc2a6541  (/home/jkratzer/builds/mc-asan/libxul.so+0x61f3541)
    #22 0x7fa0c2a0f317  (/home/jkratzer/builds/mc-asan/libxul.so+0xc95c317)
    #23 0x7fa0c652258f  (/home/jkratzer/builds/mc-asan/libxul.so+0x1046f58f)
    #24 0x7fa0bc2a6541  (/home/jkratzer/builds/mc-asan/libxul.so+0x61f3541)
    #25 0x7fa0c6521e1f  (/home/jkratzer/builds/mc-asan/libxul.so+0x1046ee1f)
    #26 0x5568fa42d20d  (/home/jkratzer/builds/mc-asan/firefox+0x10720d)
    #27 0x5568fa42d631  (/home/jkratzer/builds/mc-asan/firefox+0x107631)
    #28 0x7fa0db7e80b2  (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #29 0x5568fa380ba9  (/home/jkratzer/builds/mc-asan/firefox+0x5aba9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/jkratzer/builds/mc-asan/libxul.so+0x62e7918) 

Comment 1

[Tyson Smith [:tsmith]]

I am marking this as a fuzzblocker because this is breaking our crash bucketing (not limited to this issue). Please prioritize appropriately.

Comment 2

[Tyson Smith [:tsmith]]

Looks like this might be audio related.

Comment 3

[Paul Adenot (:padenot)]

We probably need to limit AudioBuffers to 2GB again, until we have time to figure out what to do, which is not urgent, because there is no use case I can think of to have a single AudioBuffer that is > 2GB (especially now that AudioWorklet + SharedArrayBuffer and soon Web Codecs will allow high performance audio streaming with custom scheduling).

This means blocking decodeAudioData (after resampling) and the AudioBuffer ctor from having underlying buffers > 2GB I assume, restoring the previous behaviour. There's maybe other paths ?

Karl, would you mind having a look at this?

Comment 4

[Karl Tomlinson (:karlt)]

This assertion fails even when the AudioBuffer creation is removed from the testcase.
https://pernos.co/debug/MvT2uYvMpT3BzwL3sAAZww/index.html

The crash then reproduces even when javascript.options.large_arraybuffers is false and even in Firefox 80.

A MessagePort is attempting to send a large ImageData which is written using JS_WriteTypedArray().

The serialized data is not split on sending AFAICT because PMessagePortChild::SendPostMessages() creates a single IPC::Message.

The message size limit is 256MB.

I don't know how the caller is meant to catch such cases before the assertion is hit.

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to Jason Kratzer [:jkratzer] from comment #0)

Recently, we've been seeing a large number of unsymbolized crashes. Most of these testcases involve creating a large buffer via AudioContext.

The testcase here with the large AudioBuffer is not crashing when just the postMessage() is removed.
If you have test cases without postMessage() please let me know, preferably in another bug, and I'll have a look.

Comment 7

[Andrew McCreight [:mccr8]]

This is kind of a generic issue where it is easy to hit this assertion while fuzzing. This is blocking the fuzzing of test cases with large data, and we have seen at least one externally reported test case that might have been catchable by our own fuzzing if it wasn't blocked by issues like these. I'll bring this up in the IPC meeting today. The easiest thing would just be to bump the size limit up a lot in fuzzing builds. If these messages are too large then maybe the fuzzers will get bogged down because it is so slow to send big messages but I don't know if that's an issue in practice.

Comment 9

[Jason Kratzer [:jkratzer]]

We've recently updated the fuzzers to generate buffers with a maximum size of 4GB. While we prefer to generate smaller buffers due to performance issues, it would be great if we could still create buffers at this upper limit without crashing unnecessarily.

Comment 10

[Andrew McCreight [:mccr8]]

I'll write a patch to bump the limit to 4GB in fuzzing builds, then.

Comment 11

[Andrew McCreight [:mccr8]]

Attachment: Bug 1707642 - Increase message size limit in fuzzing builds.

If the limit is small, then the fuzzers hit crashes in various places when
passing in large data structures to DOM APIs, so increase the limit.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c667e28154b6
Increase message size limit in fuzzing builds. r=ipc-reviewers,nika

Comment 13

[Marian-Vasile Laza]

Backed out changeset c667e28154b6 (Bug 1707642) for causing build bustages on MessageLink.cpp.
Backout link
Push with failures
Failure Log

Comment 14

[Andrew McCreight [:mccr8]]

Bizarre. Something about my change made it hit the crash in fuzzing builds. Maybe it is treating it as a 32-bit value and hit some kind of overflow?

Comment 15

[Andrew McCreight [:mccr8]]

It turns out that this value is a 32 bit int (signed? unsigned? not sure precisely) so 4GB turned into 0. I'll reduce the limit a bit to 1.75GB. Hopefully that will work: https://treeherder.mozilla.org/jobs?repo=try&revision=7733402a0f214e2817516e8199b2d6fe94bd1bcc

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be74db9154b3
Increase message size limit in fuzzing builds. r=ipc-reviewers,nika

Comment 17

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/be74db9154b3

Comment 18

[Andrew McCreight [:mccr8]]

Comment on attachment 9250229 [details]
Bug 1707642 - Increase message size limit in fuzzing builds.

Beta/Release Uplift Approval Request

• User impact if declined: None. This is a fuzzing-only change. It might be useful at some point to allow verifying that test cases found via fuzzing are actually fixed on beta and ESR.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• 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): This change only affects fuzzing builds.
• String changes made/needed: none

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version: 96
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 19

[Andrew McCreight [:mccr8]]

(In reply to Andrew McCreight [:mccr8] from comment #18)

• User impact if declined: None. This is a fuzzing-only change. It might be useful at some point to allow verifying that test cases found via fuzzing are actually fixed on beta and ESR.

I'm not sure how useful test cases will actually be that need this patch, come to think of it, but it will help fuzzing beta and ESR, if that's something that anybody does.

Comment 20

[Pascal Chevrel:pascalc]

Comment on attachment 9250229 [details]
Bug 1707642 - Increase message size limit in fuzzing builds.

I am not sure there are people that will do fuzzing on 95 beta but uplifting that patch looks like no risk, approved for 95 beta 8.

Comment 21

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/74e7c6f7d224

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9250229 [details]
Bug 1707642 - Increase message size limit in fuzzing builds.

Approved for 91.4esr.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/9613a40dc2c7