Last Comment Bug 750231 - Opus crash illegal instruction [@quant_band]
: Opus crash illegal instruction [@quant_band]
Status: VERIFIED FIXED
[asan]
: crash, sec-critical, testcase
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: unspecified
: x86_64 Mac OS X
: -- critical (vote)
: mozilla15
Assigned To: Timothy B. Terriberry (:derf)
:
Mentors:
Depends on:
Blocks: fuzzing-opus
  Show dependency treegraph
 
Reported: 2012-04-30 06:13 PDT by Christoph Diehl [:posidron]
Modified: 2012-06-21 17:11 PDT (History)
8 users (show)
tterribe: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
unaffected


Attachments
testcase (6.88 KB, application/zip)
2012-04-30 06:13 PDT, Christoph Diehl [:posidron]
no flags Details
callstack (21.62 KB, text/plain)
2012-04-30 06:13 PDT, Christoph Diehl [:posidron]
no flags Details
Use larger stacks on the media decoder threads with ASAN (1.03 KB, patch)
2012-06-02 16:59 PDT, Timothy B. Terriberry (:derf)
kinetik: review+
Details | Diff | Splinter Review

Description Christoph Diehl [:posidron] 2012-04-30 06:13:04 PDT
Created attachment 619537 [details]
testcase

Reproducible in ASAN builds. Due to a bug in ASAN it is right now not possible to provide values for the functions.
Comment 1 Christoph Diehl [:posidron] 2012-04-30 06:13:49 PDT
Created attachment 619538 [details]
callstack
Comment 2 Timothy B. Terriberry (:derf) 2012-04-30 10:32:06 PDT
Haven't investigated too deeply, but this file does not crash opusdec (available from opus-tools: http://git.xiph.org/?p=users/greg/opus-tools.git).
Comment 3 Christoph Diehl [:posidron] 2012-04-30 10:34:53 PDT
Sorry, should have mentioned that I have deactivated the checksum verification inside the source.
Comment 4 Timothy B. Terriberry (:derf) 2012-04-30 10:42:35 PDT
(In reply to Christoph Diehl [:cdiehl] from comment #3)
> Sorry, should have mentioned that I have deactivated the checksum
> verification inside the source.

Yes, I ran it through rogg_crcfix. opusdec reported "Decoding error: corrupt stream" in a few places, but did not crash.
Comment 5 Christoph Diehl [:posidron] 2012-04-30 10:44:55 PDT
This bug has [asan] in whiteboard. You will need to test it with an ASAN build of Firefox or compile the decoder with ASAN.
Comment 6 Timothy B. Terriberry (:derf) 2012-04-30 11:07:16 PDT
(In reply to Christoph Diehl [:cdiehl] from comment #5)
> This bug has [asan] in whiteboard. You will need to test it with an ASAN
> build of Firefox or compile the decoder with ASAN.

That doesn't change the results. It also ran clean under valgrind with memcheck, exp-ptrcheck, and exp-sgcheck.
Comment 7 Jean-Marc Valin (:jmspeex) 2012-04-30 11:11:13 PDT
The only other thing I can think of is trying with different allocators and even trying with the pseudo-stack and ENABLE_VALGRIND.
Comment 8 Christoph Diehl [:posidron] 2012-04-30 11:30:15 PDT
I would recommend compiling a Firefox version with ASAN enabled on MacOSX. This is my environment. The bug itself is reproducible every time. Except that I have commented out the "goto" in https://hg.mozilla.org/mozilla-central/file/40455cbb1ad3/media/libogg/src/ogg_framing.c#l701 nothing was touched.
Comment 9 Timothy B. Terriberry (:derf) 2012-04-30 11:49:13 PDT
(In reply to Christoph Diehl [:cdiehl] from comment #8)
> I would recommend compiling a Firefox version with ASAN enabled on MacOSX.

Yes, that's probably the best idea. But since it doesn't look like the decoder state is getting corrupted by libopus itself (or it would happen in opusdec, also), I think this is rillian's bug to track down.
Comment 10 Al Billings [:abillings] 2012-05-02 10:43:38 PDT
I have an ASAN OS X build from Monday, 4/30, and I get no crash with the attached testcase. I see a playback slider appear momentarily, which then goes away. No errors show up in console output.
Comment 11 Christoph Diehl [:posidron] 2012-05-02 11:06:35 PDT
Did you comment out the goto clause?
Comment 12 Al Billings [:abillings] 2012-05-02 13:07:55 PDT
No, I did not. I'm using a "standard" ASAN build with no changed to pulled source. This only happens if you comment out a clause?
Comment 13 Christoph Diehl [:posidron] 2012-05-02 13:10:35 PDT
You will also need to apply the Opus patches from here: https://bugzilla.mozilla.org/show_bug.cgi?id=674225
Comment 14 Al Billings [:abillings] 2012-05-02 13:15:37 PDT
I think I'll just wait for someone to fix the bug since it is assigned. :-)
Comment 15 Ralph Giles (:rillian) needinfo me 2012-05-02 13:19:04 PDT
That's fine. I don't have an asan build yet though.

Which llvm/clang/compiler-rt revision are you using?
Comment 16 Christoph Diehl [:posidron] 2012-05-02 13:22:30 PDT
https://developer.mozilla.org/en/Building_Firefox_with_Address_Sanitizer
In case of problems/info ping the author "decoder", he is also our ASAN maintainer.
Comment 17 Ralph Giles (:rillian) needinfo me 2012-05-02 15:26:45 PDT
Ok, I'm able to reproduce now, thanks to help from decoder, and no thanks to xcode version skew.
Comment 18 Al Billings [:abillings] 2012-05-16 10:40:15 PDT
Ralph, is this crash exploitable? We can't tell much from the callstack.
Comment 19 Ralph Giles (:rillian) needinfo me 2012-05-23 09:52:00 PDT
I don't know, but given the illegal instruction, the stack's probably corrupted. Which would mean yes.
Comment 20 Timothy B. Terriberry (:derf) 2012-06-02 16:29:42 PDT
Okay, so I was finally able to get an ASAN build going on this aging MacBook loaner.

I think all of the ASAN crashes are simply stack overflows. Opus uses a fair amount of stack (a few dozen kB) and the decoder threads are all created with a mere 128 kB of stack space. That's normally plenty, but apparently ASAN makes things like alloca (or more specifically, C99 vararrays) use quite a bit more than normal. Simply increasing the stack to 1 MB makes the crash in this bug, and ever other open ASAN Opus crash go away.

Patch forthcoming (a non-clobber build takes over 20 minutes on this machine for even a single changed file).
Comment 21 Timothy B. Terriberry (:derf) 2012-06-02 16:59:36 PDT
Created attachment 629534 [details] [diff] [review]
Use larger stacks on the media decoder threads with ASAN

My build still hasn't finished, but I need to run. This _should_ work.
Comment 22 Christoph Diehl [:posidron] 2012-06-02 21:52:05 PDT
Fixed. I will mark the other bugs as resolved.
Comment 23 Timothy B. Terriberry (:derf) 2012-06-03 14:37:43 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/902cd184dca8

Note You need to log in before you can comment on or make changes to this bug.