1232989 - BMP: crash [@mozilla::image::Downscaler::DownscaleInputLine]

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Christoph Diehl [:posidron]]

The following testcase crashes on en-us.linux-x86_64-asan.tar.bz2 revision 5a587fbae89b27c66d28b6871cc54fba766958b9

See attachment.

Backtrace:

==4986==ERROR: AddressSanitizer: SEGV on unknown address 0x601fb82e8b48 (pc 0x7f6560d675fe sp 0x7f6545be0f20 bp 0x7f6545be0f80 T20)
    #0 0x7f6560d675fd in operator[] /tools/gcc-4.7.3-0moz1/lib/gcc/x86_64-unknown-linux-gnu/4.7.3/../../../../include/c++/4.7.3/bits/move.h:176
    #1 0x7f6560d675fd in mozilla::image::Downscaler::DownscaleInputLine() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/Downscaler.cpp:322
    #2 0x7f6560d65998 in mozilla::image::Downscaler::CommitRow() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/Downscaler.cpp:221
    #3 0x7f6560db3aa2 in mozilla::image::nsBMPDecoder::FinishRow() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsBMPDecoder.cpp:412
    #4 0x7f6560db3662 in mozilla::image::nsBMPDecoder::FinishInternal() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsBMPDecoder.cpp:243
    #5 0x7f6560d61490 in mozilla::image::Decoder::CompleteDecode() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/Decoder.cpp:196
    #6 0x7f6560d60078 in mozilla::image::Decoder::Decode(mozilla::image::IResumable*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/Decoder.cpp:122
    #7 0x7f6560d5f962 in mozilla::image::DecodePool::Decode(mozilla::image::Decoder*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:455
    #8 0x7f6560d7eb9c in mozilla::image::DecodePoolWorker::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:281
    #9 0x7f655ec86624 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:964
    #10 0x7f655ed0186a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:297
    #11 0x7f655f62c7af in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:326
    #12 0x7f655f598f8c in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #13 0x7f655f598f8c in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:227
    #14 0x7f655f598f8c in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #15 0x7f655ec82170 in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:376
    #16 0x7f656c5644b5 in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:212
    #17 0x7f656cba3181 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8181)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /tools/gcc-4.7.3-0moz1/lib/gcc/x86_64-unknown-linux-gnu/4.7.3/../../../../include/c++/4.7.3/bits/move.h:176 operator[]
Thread T20 (ImgDecoder #1) created by T0 (Web Content) here:
    #0 0x461945 in pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:175
    #1 0x7f656c560e3d in _PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f656c5609ba in PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f655ec838ad in nsThread::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:496
    #4 0x7f655ec8a09e in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadManager.cpp:249
    #5 0x7f655ed008f8 in NS_NewThread(nsIThread**, nsIRunnable*, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:71
    #6 0x7f6560d5e4c8 in mozilla::image::DecodePool::DecodePool() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:357
    #7 0x7f6560d5dd0b in mozilla::image::DecodePool::Singleton() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:314
    #8 0x7f6560db0208 in mozilla::image::InitModule() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/build/nsImageModule.cpp:95
    #9 0x7f655ec57acd in Load /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/components/nsComponentManager.cpp:883
    #10 0x7f655ec57acd in nsFactoryEntry::GetFactory() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/components/nsComponentManager.cpp:1919
    #11 0x7f655ec59087 in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/components/nsComponentManager.cpp:1217
    #12 0x7f655ec4ff84 in nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/components/nsComponentManager.cpp:1576
    #13 0x7f655ecf0d91 in CallGetService /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsComponentManagerUtils.cpp:67
    #14 0x7f655ecf0d91 in nsGetServiceByContractID::operator()(nsID const&, void**) const /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsComponentManagerUtils.cpp:280
    #15 0x7f655ece5866 in nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsCOMPtr.cpp:103
    #16 0x7f6560befd3c in nsCOMPtr /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/gfx/thebes/../../dist/include/nsCOMPtr.h:540
    #17 0x7f6560befd3c in gfxPlatform::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/thebes/gfxPlatform.cpp:622
    #18 0x7f6560bee0d4 in gfxPlatform::GetPlatform() /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/thebes/gfxPlatform.cpp:455
    #19 0x7f6564170cb3 in mozilla::dom::ContentProcess::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/ipc/ContentProcess.cpp:83
    #20 0x7f65666fdc60 in XRE_InitChildProcess /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:603
    #21 0x48d760 in content_process_main(int, char**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:237
    #22 0x7f655c5a7ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

Comment 1

[Christoph Diehl [:posidron]]

Attachment: Testcase

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Related to bug 1224200?

Comment 3

[Nicholas Nethercote [inactive]]

I looked at this one. As far as I can tell the BMP is well-formed, and Firefox displays it without obvious problems. Furthermore, the code location described by the stack trace in comment 0 is one that should only be hit if the image's pixel data is truncated, which is not the case for this image.

cdiehl, three times now (in this bug, bug 1238551, and bug 1224979) you've reported a BMP decoder crash at a code location that should be impossible for the associated image. I'm guessing that your fuzzer generates lots of permutations of input files. Could there be a bug in it that makes it associate crash results with the wrong input file?

Comment 4

[Christoph Diehl [:posidron]]

It can happen that crashes get produced and the testcase does not work; may be related to threading? 

The fuzzer generates a test-case, then sends that testcase to Firefox via WebSocket and then the fuzzer awaits for response of the Firefox process and makes a report.

Comment 5

[Tyson Smith [:tsmith]]

(In reply to Christoph Diehl [:cdiehl] from comment #4)
> It can happen that crashes get produced and the testcase does not work; may
> be related to threading? 
> 
> The fuzzer generates a test-case, then sends that testcase to Firefox via
> WebSocket and then the fuzzer awaits for response of the Firefox process and
> makes a report.

I have the same issue. I am using the pref "image.multithreaded_decoding.limit=1" this reduces the delay between processing a bad image and detecting it down to about 1 iteration from ~5. The only thing I found that works is to wait 1 second between test cases and this is not realistic :(. Also the time to wait varies between browser builds (dbg, asan, etc).

Is there any way to tell via javascript when the image thread is done processing? I have tried loading a fuzzed image waiting for the onload event then setting src to a tiny known valid image and waiting for it to load again (with the above pref set) before moving on but that doesn't seem to work.

Anyone have any ideas?

Comment 6

[Timothy Nikkel (:tnikkel)]

The onload for an image only fires after all of the file contents have been delivered, the image hasn't necessarily been decoded yet at that point. So switching the src at that point could likely mean the image never gets painted, and hence never decoded.

After the load event try doing a drawImage of the img element to a canvas, that should force a sync decode.

We should also think about what else we can do to make fuzzing imagelib work more smoothly.

Comment 7

[Tyson Smith [:tsmith]]

(In reply to Timothy Nikkel (:tnikkel) from comment #6)
> We should also think about what else we can do to make fuzzing imagelib work
> more smoothly.

I don't know if this pref still exists but "image.multithreaded_decoding.enabled" would be nice if we could set it to false. Or image.multithreaded_decoding.limit=0 perhaps? Or would this even help if we can't get any feedback at the javascript level telling us the image is decoded?

The best solution would be a test shell that includes only the code we want to test and as little else as possible. This would allow us to also use feedback driven fuzzing which can be a huge win. We have been talking to the media team about this for fuzzing video and audio. We have tried gtests but it does not offer the benefits we'd hoped.

Comment 8

[Timothy Nikkel (:tnikkel)]

(In reply to Tyson Smith [:tsmith] from comment #7)
> I don't know if this pref still exists but
> "image.multithreaded_decoding.enabled" would be nice if we could set it to
> false. Or image.multithreaded_decoding.limit=0 perhaps? Or would this even
> help if we can't get any feedback at the javascript level telling us the
> image is decoded?

Using the drawImage trick should get you both of these. It will sync decode the image on the mainthread before returning (or wait on a background thread to decode until it is finished).

> The best solution would be a test shell that includes only the code we want
> to test and as little else as possible. This would allow us to also use
> feedback driven fuzzing which can be a huge win.

Interesting.

Comment 9

[Tyson Smith [:tsmith]]

(In reply to Timothy Nikkel (:tnikkel) from comment #8)
> Using the drawImage trick should get you both of these. It will sync decode
> the image on the mainthread before returning (or wait on a background thread
> to decode until it is finished).

That did this trick! Thanks for the suggestion :)

Comment 10

[Al Billings [:abillings - ex-MoCo]]

What are the next steps here?

Comment 11

[Timothy Nikkel (:tnikkel)]

If the test case causes no problems then probably just close this bug. Tyson said he hasn't been getting any more bmp crashes with his fuzzer. So it's likely that this bug (which we don't have a test case for) has been fixed.

Comment 12

[Tyson Smith [:tsmith]]

(In reply to Timothy Nikkel (:tnikkel) (mostly away until 2/20) from comment #11)
> If the test case causes no problems then probably just close this bug. Tyson
> said he hasn't been getting any more bmp crashes with his fuzzer. So it's
> likely that this bug (which we don't have a test case for) has been fixed.

I support closing this issue but I'll let Christoph have the final word.