Closed Bug 1206744 Opened 7 years ago Closed 7 years ago

crash in skia::ConvolveVertically_SSE2_impl<T>(short const*, int, unsigned char* const*, int, unsigned char*)

Categories

(Core :: Graphics, defect)

41 Branch
Unspecified
Windows NT
defect
Not set
critical

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)

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
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...
I'm pretty sure those recent crashes on Nightly are caused by bug 1206836.
Depends on: 1206836
OK, bug 1206836 is now fixed in the latest Nightly. So any interference from that is gone.
Duplicate of this bug: 1207378
Bug 1207378 is a dupe of this with valgrind output, which may be useful.
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)
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)
Attached file crash testcase
The above testcase should match the data from comment 11.  (It's certainly the same image; I printed out the URL.)
I think this is a different problem than the one originally reported in this bug. I'm going to dedupe.
Flags: needinfo?(seth)
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]
(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)
(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.
(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.
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).
Bug 1208936 contains a test case for this.
Depends on: 1208936
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.
(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.
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
Duplicate of this bug: 1207059
Assignee: nobody → seth
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Whiteboard: [gfx-noted] → [gfx-noted][fixed by bug 1207378]
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)
Bug 1207378 landed on aurora 2015-10-05, see bug 1207378, comment 28. So we should be checking builds after 2015-10-05.
Yeah. I expect it to be fixed at this point in Aurora as well.
Flags: needinfo?(seth)
Looks good so far, no crashes after build 2015100200.
Group: gfx-core-security
You need to log in before you can comment on or make changes to this bug.