746 bytes, text/html
18.29 KB, text/plain
4.00 KB, patch
|Details | Diff | Splinter Review|
Reproduces on trunk. Testcase with better stack coming. ================================================================= ==6483== ERROR: AddressSanitizer heap-use-after-free on address 0x7f12bb7cef80 at pc 0x7f12ebf2a6ad bp 0x7fff1ecacb70 sp 0x7fff1ecacb68 READ of size 8 at 0x7f12bb7cef80 thread T0 #0 0x7f12ebf2a6ac in nsQueryInterface::operator()(nsID const&, void**) const objdir-ff-asan/xpcom/build/nsCOMPtr.cpp:14 #1 0x7f12e91b069d in nsCOMPtr ../../dist/include/nsCOMPtr.h:554 #2 0x7f12e919e955 in imgRequest::OnStopFrame(imgIRequest*, unsigned int) image/src/imgRequest.cpp:645 #3 0x7f12e914d3cb in mozilla::image::Decoder::PostFrameStop() image/src/Decoder.cpp:246 0x7f12bb7cef80 is located 0 bytes inside of 24-byte region [0x7f12bb7cef80,0x7f12bb7cef98) freed by thread T0 here: #0 0x42b630 in free #1 0x7f12e948ca4d in nsBulletListener::Release() layout/generic/nsBulletFrame.cpp:1622 previously allocated by thread T0 here: #0 0x42b6f0 in __interceptor_malloc #1 0x7f12ef58f3a8 in moz_xmalloc memory/mozalloc/mozalloc.cpp:57 #2 0x7f12e94a86d4 in nsFrame::Init(nsIContent*, nsIFrame*, nsIFrame*) layout/generic/nsFrame.cpp:535 Shadow byte and word: 0x1fe2576f9df0: fd 0x1fe2576f9df0: fd fd fd fd fd fd fd fd More shadow bytes: 0x1fe2576f9dd0: fd fd fd fd fd fd fd fd 0x1fe2576f9dd8: fd fd fd fd fd fd fd fd 0x1fe2576f9de0: fa fa fa fa fa fa fa fa 0x1fe2576f9de8: fa fa fa fa fa fa fa fa =>0x1fe2576f9df0: fd fd fd fd fd fd fd fd 0x1fe2576f9df8: fd fd fd fd fd fd fd fd 0x1fe2576f9e00: fa fa fa fa fa fa fa fa 0x1fe2576f9e08: fa fa fa fa fa fa fa fa 0x1fe2576f9e10: 00 00 00 00 00 00 00 00 Stats: 352M malloced (405M for red zones) by 832581 calls Stats: 60M realloced by 88678 calls Stats: 304M freed by 665969 calls Stats: 197M really freed by 173839 calls Stats: 612M (156759 full pages) mmaped in 153 calls mmaps by size class: 8:557022; 9:49146; 10:81900; 11:12282; 12:3072; 13:2048; 14:1280; 15:256; 16:448; 17:1280; 18:240; 19:40; 20:20; 21:2; mallocs by size class: 8:655210; 9:61844; 10:88875; 11:13927; 12:4293; 13:2966; 14:2222; 15:488; 16:815; 17:1614; 18:259; 19:47; 20:19; 21:2; frees by size class: 8:513952; 9:46898; 10:84377; 11:10477; 12:3126; 13:2543; 14:1600; 15:435; 16:741; 17:1594; 18:165; 19:45; 20:16; rfrees by size class: 8:114982; 9:29585; 10:13571; 11:8791; 12:1614; 13:1699; 14:1294; 15:249; 16:554; 17:1311; 18:137; 19:37; 20:15; Stats: malloc large: 1941 small slow: 3643 ==6483== ABORTING
Created attachment 658876 [details] Testcase Reproduces reliably with a couple of firefox instances run simultaneously.
Who's triaging sec bugs in Core/ImageLib?
Created attachment 676232 [details] ASAN dump ==7177== ERROR: AddressSanitizer heap-use-after-free on address 0x7fe0bbe21b80 at pc 0x7fe0d75b79c4 bp 0x7ffff64bafd0 sp 0x7ffff64bafc8 READ of size 8 at 0x7fe0bbe21b80 thread T0 #0 0x7fe0d75b79c4 in nsQueryInterface::operator()(nsID const&, void**) const xpcom/build/nsCOMPtr.cpp:14 #1 0x7fe0cb6ed163 in nsCOMPtr<imgIOnloadBlocker>::assign_from_qi(nsQueryInterface, nsID const&) ../../dist/include/nsCOMPtr.h:1144 #2 0x7fe0cb6ece61 in nsCOMPtr ../../dist/include/nsCOMPtr.h:555 #3 0x7fe0cb6e13a8 in nsCOMPtr ../../dist/include/nsCOMPtr.h:555 #4 0x7fe0cb6e10bb in imgRequestProxy::BlockOnload() image/src/imgRequestProxy.cpp:741 #5 0x7fe0cb7050d9 in imgStatusTracker::SendBlockOnload(imgRequestProxy*) image/src/imgStatusTracker.cpp:736 #6 0x7fe0cb704a6d in imgStatusTrackerObserver::OnStartDecode() image/src/imgStatusTracker.cpp:64 #7 0x7fe0cb548900 in mozilla::image::Decoder::Init() image/src/Decoder.cpp:47 #8 0x7fe0cb55e352 in mozilla::image::RasterImage::InitDecoder(bool) image/src/RasterImage.cpp:2523 #9 0x7fe0cb59f22e in mozilla::image::RasterImage::RequestDecodeCore(mozilla::image::RasterImage::RequestDecodeType) image/src/RasterImage.cpp:2732 #10 0x7fe0cb59e6b8 in mozilla::image::RasterImage::StartDecoding() image/src/RasterImage.cpp:2672 #11 0x7fe0cb5723e5 in mozilla::image::RasterImage::WantDecodedFrames() image/src/RasterImage.cpp:2657 #12 0x7fe0cb571d0f in mozilla::image::RasterImage::GetImgFrame(unsigned int) image/src/RasterImage.cpp:794 #13 0x7fe0cb56ecca in mozilla::image::RasterImage::GetDrawableImgFrame(unsigned int) image/src/RasterImage.cpp:802 #14 0x7fe0cb572a37 in mozilla::image::RasterImage::GetCurrentDrawableImgFrame() image/src/RasterImage.cpp:850 #15 0x7fe0cb5a8ce2 in mozilla::image::RasterImage::Draw(gfxContext*, gfxPattern::GraphicsFilter, gfxMatrix const&, gfxRect const&, nsIntRect const&, nsIntSize const&, unsigned int) image/src/RasterImage.cpp:3018 #16 0x7fe0cbe7f0f5 in DrawImageInternal layout/base/nsLayoutUtils.cpp:3812 #17 0x7fe0cbe805d3 in nsLayoutUtils::DrawSingleImage(nsRenderingContext*, imgIContainer*, gfxPattern::GraphicsFilter, nsRect const&, nsRect const&, unsigned int, nsRect const*) layout/base/nsLayoutUtils.cpp:3928 #18 0x7fe0cc30b7ce in nsBulletFrame::PaintBullet(nsRenderingContext&, nsPoint, nsRect const&) layout/generic/nsBulletFrame.cpp:245 ...
What's the status here? This is marked tracking# for 18, which is entering beta this week.
I'm having trouble acquiring a working build of an ASAN-enabled clang, so I may not be the best owner here.
(In reply to Josh Matthews [:jdm] from comment #5) > I'm having trouble acquiring a working build of an ASAN-enabled clang, so I > may not be the best owner here. You can use latest one from here - http://commondatastorage.googleapis.com/chromium-browser-clang/index.html?path=Linux_x64/
Crumbs, I reproduced it the first time, and now can't seem to trigger the error no matter how many times I refresh/restart.
Created attachment 683774 [details] [diff] [review] Clear the listener associated with cancelled bullet frame image requests. With this patch I wasn't able to reproduce the intermittent crash in many minutes of frequent refreshing through multiple tabs.
Comment on attachment 683774 [details] [diff] [review] Clear the listener associated with cancelled bullet frame image requests. r=me
Comment on attachment 683774 [details] [diff] [review] Clear the listener associated with cancelled bullet frame image requests. [Security approval request comment] How easily can the security issue be deduced from the patch? Not easily at all, in my opinion. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? Presumably all of them. If not all supported branches, which bug introduced the flaw? Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? They should be exceedingly easily. How likely is this patch to cause regressions; how much testing does it need? My intuition says not likely; the functionality change involved in dealing with cancelled requests should not be noticeable. I don't believe this should require much testing.
Comment on attachment 683774 [details] [diff] [review] Clear the listener associated with cancelled bullet frame image requests. sec-approval+. Please prepare a patch for affected (supported) branches though. Could this use an automated test?
The same patch applies to all affected branches. Do I need to do anything special, or can I just land it?
You need to ask for approval to land.
Comment on attachment 683774 [details] [diff] [review] Clear the listener associated with cancelled bullet frame image requests. [Approval Request Comment] Bug caused by (feature/regressing bug #): Lost in the depths of time. User impact if declined: Possible exploit vector. Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): Slight, but unlikely, chance of unknown regression in behaviour when cancelling image requests for bullets. String or UUID changes made by this patch: None.
Does this need an in-testsuite testcase?