Closed
Bug 1206744
Opened 9 years ago
Closed 9 years ago
crash in skia::ConvolveVertically_SSE2_impl<T>(short const*, int, unsigned char* const*, int, unsigned char*)
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
WORKSFORME
Tracking | Status | |
---|---|---|
firefox42 | --- | unaffected |
firefox43 | + | fixed |
firefox44 | + | fixed |
firefox-esr38 | --- | unaffected |
People
(Reporter: away, Assigned: seth)
References
Details
(Keywords: crash, sec-critical, Whiteboard: [gfx-noted][fixed by bug 1207378])
Crash Data
Attachments
(3 files)
1.41 KB,
patch
|
Details | Diff | Splinter Review | |
1.92 KB,
image/gif
|
Details | |
106 bytes,
text/html
|
Details |
This bug was filed from the Socorro interface and is
report bp-f5d498ed-d32b-4e35-8da2-040c02150920.
=============================================================
This started after bug 1180225.
The crashes are on addresses ending with -000 (a new page) which suggest that the buffer wasn't as big as the function expected.
xul!skia::ConvolveVertically_SSE2_impl<0>
xul!mozilla::image::Downscaler::DownscaleInputLine
xul!mozilla::image::Downscaler::CommitRow
xul!mozilla::image::nsGIFDecoder2::OutputRow
xul!mozilla::image::nsGIFDecoder2::DoLzw
xul!mozilla::image::nsGIFDecoder2::WriteInternal
xul!mozilla::image::Decoder::Write
xul!mozilla::image::Decoder::Decode
xul!mozilla::image::DecodePool::Decode
xul!mozilla::image::DecodePoolWorker::Run
xul!nsThread::ProcessNextEvent
xul!NS_ProcessNextEvent
xul!mozilla::ipc::MessagePumpForNonMainThreads::Run
xul!MessageLoop::RunHandler
xul!MessageLoop::Run
xul!nsThread::ThreadFunc
nss3!_PR_NativeRunThread
nss3!pr_root
msvcr120!beginthreadex
Comment 1•9 years ago
|
||
Hm, the dates don't line up. This started in build 0919, when bug 1180225 landed on the 6th.
Could this be causing the recent string of random crashes on nightly? In the cases where we don't segfault, we'd be clobbering memory...
Assignee | ||
Comment 4•9 years ago
|
||
I'm pretty sure those recent crashes on Nightly are caused by bug 1206836.
Depends on: 1206836
Assignee | ||
Comment 5•9 years ago
|
||
OK, bug 1206836 is now fixed in the latest Nightly. So any interference from that is gone.
Assignee | ||
Comment 7•9 years ago
|
||
Bug 1207378 is a dupe of this with valgrind output, which may be useful.
Assignee | ||
Comment 8•9 years ago
|
||
I can't reproduce this (who knows why).
David, could you import Jeff's patch into your patch queue and see if it resolves the issue for you? If so, that's a good indication that we should move forward with that approach.
Flags: needinfo?(dbaron)
Assignee | ||
Comment 9•9 years ago
|
||
Interesting:
[16:15:04] <~dbaron> seth, dholbert, it may be relevant that I disable jemalloc so I can use valgrind
The patch doesn't help.
Flags: needinfo?(dbaron)
So I put in some printfs, and it seems like some sort of higher level logic error.
In particular, what I'm seeing is the following:
Decoder::AllocateFrame is called with aTargetSize being 24x24 (w*h) and aFrameRect being {0,1,24,22} ({x,y,w,h}). It allocates a buffer that is 2112 bytes (24 * 22 * 4).
Then the downscaler outputs 24 lines into that buffer, the last two of which are entirely after the end of the buffer (i.e., the start of line 23 is the first invalid byte).
Flags: needinfo?(seth)
The above testcase should match the data from comment 11. (It's certainly the same image; I printed out the URL.)
Assignee | ||
Comment 15•9 years ago
|
||
I think this is a different problem than the one originally reported in this bug. I'm going to dedupe.
Flags: needinfo?(seth)
Comment 16•9 years ago
|
||
Maybe I'm crazy, in which case just unset, but wouldn't this potentially allow an exploit if this is an out of bounds write?
Group: gfx-core-security
Flags: needinfo?(seth)
Whiteboard: [gfx-noted]
Assignee | ||
Comment 17•9 years ago
|
||
(In reply to Seth Fowler [:seth] [:s2h] from comment #15)
> I think this is a different problem than the one originally reported in this
> bug. I'm going to dedupe.
To clarify: it seems to be different because bug 1207378 is *definitely* a regression from bug 1194058, but the regression range for this bug seems to indicate bug 1180225.
Flags: needinfo?(seth)
Assignee | ||
Comment 18•9 years ago
|
||
(In reply to Seth Fowler [:seth] [:s2h] from comment #17)
> To clarify: it seems to be different because bug 1207378 is *definitely* a
> regression from bug 1194058, but the regression range for this bug seems to
> indicate bug 1180225.
Or maybe I misinterpreted comment 2 above. Looks like David is actually saying that bug 1180225 is *not* implicated. 0919 would indeed implicate bug 1194058; I just confirmed that it landed on 9/18.
If that's the case, bug 1207378 should have cleared this up. I'm not sure what to conclude; looks like crashstats is still showing some crashes for this signature.
Comment 19•9 years ago
|
||
(In reply to Seth Fowler [:seth] [:s2h] from comment #18)
> If that's the case, bug 1207378 should have cleared this up. I'm not sure
> what to conclude; looks like crashstats is still showing some crashes for
> this signature.
I don't think bug 1207378 made it to a nightly yet (or maybe it just made the most recent nightly?). Either way crashstats hasn't had enough time yet to determine.
Assignee | ||
Comment 20•9 years ago
|
||
Yep, was just about to post that. We'll find out tomorrow.
http://dbaron.org/mozilla/crashes-by-build has useful links for figuring out if crashes are fixed in a nightly, but bug 1207378 indeed did not make today's nightly, so you'll have to wait for tomorrow's (the 29th).
Comment 22•9 years ago
|
||
Bug 1208936 contains a test case for this.
Comment 23•9 years ago
|
||
There are still crashes with this signature on todays (sept 29) nightly. They come from the gif decoder as well. Looks like this is not fixed.
Comment 24•9 years ago
|
||
Tracking for 43 and 44.
status-firefox42:
--- → unaffected
status-firefox43:
--- → affected
status-firefox44:
--- → affected
tracking-firefox43:
--- → +
tracking-firefox44:
--- → +
Comment 25•9 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #23)
> There are still crashes with this signature on todays (sept 29) nightly.
> They come from the gif decoder as well. Looks like this is not fixed.
Scratch that. The crashes with buildid of sept 29 are all v 43, so aurora, and aurora doesn't have the patch from bug 1207378 yet.
Comment 26•9 years ago
|
||
related, same as https://bugzilla.mozilla.org/show_bug.cgi?id=1207958#c4 ?
Comment 27•9 years ago
|
||
I'd investigate but I can't access that bug.
Based on the links in http://dbaron.org/mozilla/crashes-by-build this is pretty clearly fixed in today's (September 29) nightly, presumably by the patches in bug 1207378.
Keywords: sec-critical
Updated•9 years ago
|
Assignee: nobody → seth
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
Whiteboard: [gfx-noted] → [gfx-noted][fixed by bug 1207378]
Updated•9 years ago
|
Comment 30•9 years ago
|
||
There are still a lot of crashes for 43.0a2 with this signature - and from the 2015100200 build. So I don't think we can call this fixed in 43. Is there an uplift in an associated bug that we've missed here?
Flags: needinfo?(seth)
Comment 31•9 years ago
|
||
Bug 1207378 landed on aurora 2015-10-05, see bug 1207378, comment 28. So we should be checking builds after 2015-10-05.
Assignee | ||
Comment 32•9 years ago
|
||
Yeah. I expect it to be fixed at this point in Aurora as well.
Flags: needinfo?(seth)
Comment 33•9 years ago
|
||
Looks good so far, no crashes after build 2015100200.
Updated•9 years ago
|
Group: gfx-core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•