Crash when trying to optimize zero-sized image

RESOLVED FIXED in mozilla7

Status

()

Core
Graphics
--
critical
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: wsmwk, Assigned: Joe Drew (not getting mail))

Tracking

(4 keywords)

Trunk
mozilla7
crash, regression, reproducible, testcase
Points:
---

Firefox Tracking Flags

(firefox5-, firefox6-, blocking2.0 -)

Details

(Whiteboard: [sg:dos null-deref][inbound], crash signature)

Attachments

(3 attachments, 7 obsolete attachments)

crash [@ imgFrame::Optimize()]

bp-3a4443c1-0914-4bcf-9855-e90742101204

EXCEPTION_ACCESS_VIOLATION_READ
0x0
0	xul.dll	imgFrame::Optimize	modules/libpr0n/src/imgFrame.cpp:259
1	xul.dll	mozilla::imagelib::RasterImage::DecodingComplete	modules/libpr0n/src/RasterImage.cpp:1046
2	xul.dll	mozilla::imagelib::Decoder::Finish	modules/libpr0n/src/Decoder.cpp:132
3	xul.dll	mozilla::imagelib::RasterImage::ShutdownDecoder	modules/libpr0n/src/RasterImage.cpp:2138
4	xul.dll	mozilla::imagelib::imgDecodeWorker::Run	modules/libpr0n/src/RasterImage.cpp:2609
5	xul.dll	mozilla::imagelib::RasterImage::SourceDataComplete	modules/libpr0n/src/RasterImage.cpp:1269
6	xul.dll	imgRequest::OnStopRequest	modules/libpr0n/src/imgRequest.cpp:926
7	mozcrt19.dll	arena_dalloc	obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4284
8	xul.dll	nsStreamListenerTee::OnStopRequest	netwerk/base/src/nsStreamListenerTee.cpp:71
9	xul.dll	nsHttpChannel::OnStopRequest	netwerk/protocol/http/nsHttpChannel.cpp:4030
10	xul.dll	nsInputStreamPump::OnStateStop	netwerk/base/src/nsInputStreamPump.cpp:578

Comment 1

6 years ago
It is #271 top crasher in 4.0b11.

More reports at:
https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=imgFrame%3A%3AOptimize%28%29
A comment from one user that crashed: "This problem seems to confirm some Display Driver bug with Radeon Catalyst 2011.0308.2325.42017 - Repeated many times..."

Comment 3

6 years ago
Reproducible url http://www.beanrunnercafe.com/


Regression window(m-c hourly)::
Works;
http://hg.mozilla.org/mozilla-central/rev/484bd866905e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre ID:20100912040749
Crash:
http://hg.mozilla.org/mozilla-central/rev/389e836517bc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre ID:20100912085645
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=484bd866905e&tochange=389e836517bc

Comment 4

6 years ago
bp-f9961fa8-8c3e-4660-9426-7a07b2110519

Updated

6 years ago
Duplicate of this bug: 658295

Updated

6 years ago
OS: Windows Vista → All
Hardware: x86 → All

Updated

6 years ago
blocking2.0: --- → ?
tracking-firefox5: --- → ?
tracking-firefox6: --- → ?
Keywords: regression

Updated

6 years ago
Keywords: reproducible

Comment 6

6 years ago
Has this spiked in the crash data or something? Why has it been nominated as a concern for Firefox 5?

Comment 7

6 years ago
The reproducible url listed here: http://www.beanrunnercafe.com/  is a site that I created and just put online on 5.16.11. It was created in Freeway Pro 5.5 (developer Softpress). Uses an inline box model, with some CMS (WebYep) and some javascript feeds.

Updated

6 years ago
Blocks: 514033

Comment 8

6 years ago
Open http://www.beanrunnercafe.com/Resources/favicon.ico
Crash ...

Comment 9

6 years ago
ARe you suggesting I should remove the favicon.ico?

Comment 10

6 years ago
Created attachment 533813 [details]
Crash image

Comment 11

6 years ago
I removed the favicon from each page and it appears to no longer crash. Anyone know why this would cause a crash?
https://crash-stats.mozilla.com/query/query?query_type=simple&do_query=1&query=imgFrame%3A%3AOptimize() - 236 crashes in the last week, all Windows.

Comment 13

6 years ago
We are not going to track this but if there is a fix ready it should be nominated for beta approval with a risk analysis.
tracking-firefox5: ? → -
blocking2.0: ? → -
Keywords: testcase
Whiteboard: [sg:dos null-deref]

Comment 14

6 years ago
looks like this is also seen on other sites

domains/pages:

  85 www.mafia2multiplayer.com
  66 www.sendmepc.com
   2 http://www.sendmepc.com/toshiba/191-toshiba-satellite-l650-15g.html
   2 http://www.sendmepc.com/acer/204-acer-aspire-5741-i5-ati.html
   2 http://www.sendmepc.com/208-dell-inspiron-n5010-new-shape-.html
   2 http://www.sendmepc.com/14-2500-le-to-3000-le
   1 http://www.sendmepc.com/toshiba/306-toshiba-satellite-c660-162-i3-253-ghz-320-gb-ram-2-gb.html
   1 http://www.sendmepc.com/toshiba/178-toshiba-satellite-c650-1cg.html
   1 http://www.sendmepc.com/search.php?orderby=position&orderway=desc&search_query=N5110&submit_search=Search
   1 http://www.sendmepc.com/lang-fr/308-hp-pavilion-g6-1040ee-core-i3-4g-ram-ati-5650-win7.html
   1 http://www.sendmepc.com/hp-laptop/239-hp-pavilion-dv6-3170ee-core-i7-.html
   1 http://www.sendmepc.com/hp-laptop/182-hp-pavilion-dv6-3053ee.html
   1 http://www.sendmepc.com/fujitsu-siemens/251-fujitsu-lifebook-core-i5-500gb-ati.html
   1 http://www.sendmepc.com/dell/252-dell-inspiron-n5010-i7-6mb-cache-4-gb-ram-1-year-warantee.html
   1 http://www.sendmepc.com/dell/244-dell-inspiron-n5010-i7-6mb-cache-4-gb-ram-3-years-warantee.html
   1 http://www.sendmepc.com/asus/295-laptop-asus-x42jy-i5-266ghz-3g-ram-ddr3-500g-hd-ati-hd-1gb-2-yrs-ltd-warranty.html
   1 http://www.sendmepc.com/asus/294-asus-x42jy-i3-253ghz-3g-ram-ddr3-500g-2yr-ltd-warranty.html
   1 http://www.sendmepc.com/acer/301-acer-aspire-5742g-i5-nvidia-geforce.html
   1 http://www.sendmepc.com/acer/301-acer-aspire-5740g-i5.html
   1 http://www.sendmepc.com/acer/300-acer-aspire-5740g-i5.html
   1 http://www.sendmepc.com/acer/151-acer-aspire-5741g-i5-226-ghz-up-to-253ghz-ati-hd-5470-512mb-up-2234mb-hd-500gb-4gb-ddr3-win-7.html
   1 http://www.sendmepc.com/316-inspiron-n5010-253-ghz-i5-320-gb-hd-3-gb-ram-ati-512mb-.html
   1 http://www.sendmepc.com/315-dell-inspiron-n5110-i7-win7-3years-6gb-ram.html
   1 http://www.sendmepc.com/308-hp-pavilion-g6-1040ee-core-i3-4g-ram-ati-5650-win7.html
   1 http://www.sendmepc.com/285-dell-inspiron-n5010-i5-3-gb-253-ghz-320-gb-ati-512-mb.html
   1 http://www.sendmepc.com/237-dell-inspiron-n5010-i5-266-ghz-290-ghz-ram-4gb-500-gb-hd-ati-1gb-windows-7.html
   1 http://www.sendmepc.com/236-inspiron-n5010-266-ghz-i5-500-gb-hd-4-gb-ram-ati-hd-5650-1gb-up-2775mb-.html


  35 www.facebook.com
  21 www.beanrunnercafe.com
  2 http://www.beanrunnercafe.com/test/webyep-system/program/l-save.php

  15 www.google.com.eg
  14 gobowling.com.au
  11 www.youtube.com
  10 www.heritagehumanesociety.org
   9 hurrichips.com
   9 bugzilla.mozilla.org
      9 https://bugzilla.mozilla.org/attachment.cgi?id=533813

   8 mafia2multiplayer.com
   7 www.samradford.com
   1 http://www.samradford.com/post/5583012305/is-the-anc-fit-to-lead-south-africa-anymore
   1 http://www.samradford.com/post/5417121537/the-importance-of-unwritten-plans
   1 http://www.samradford.com/post/5360994454/skype-only-makes-money-when-it-changes-hands
   1 http://www.samradford.com/post/5268435883/cameron-and-clegg-one-year-on-from-the-times
   1 http://www.samradford.com/


   7 www.leutesdorf-rhein.de
   1 http://www.leutesdorf-rhein.de/weingut-emmerich/download/weinliste.pdf
   1 http://www.leutesdorf-rhein.de/service/web-quiz-mai-2011.html
   1 http://www.leutesdorf-rhein.de/pension-will/index.html
   1 http://www.leutesdorf-rhein.de/gastronomie.html
   1 http://www.leutesdorf-rhein.de/

   7 www.google.com

Comment 15

6 years ago
per comment 13, we're not going to be tracking this specific issue.
tracking-firefox6: ? → -
Crash Signature: [@ imgFrame::Optimize()]
(Assignee)

Updated

6 years ago
Duplicate of this bug: 667624
(Assignee)

Updated

6 years ago
Assignee: nobody → joe
Summary: crash [@ imgFrame::Optimize()] → Crash when trying to optimize zero-sized image
(Assignee)

Comment 17

6 years ago
Created attachment 542585 [details] [diff] [review]
handle zero-sized images in imgFrame::Optimize

Zero-sized images are special-cased in gfxImageSurface by leaving mData set to null. We should not even try to optimize zero-sized images.
Attachment #542585 - Flags: review?(jmuizelaar)
(Assignee)

Comment 18

6 years ago
Created attachment 542587 [details] [diff] [review]
zero sized image crashtest
Attachment #542587 - Flags: review?(jmuizelaar)
Comment on attachment 542585 [details] [diff] [review]
handle zero-sized images in imgFrame::Optimize

Go straight to hell.
Attachment #542585 - Flags: review?(jmuizelaar) → review-
(In reply to comment #19)
> Comment on attachment 542585 [details] [diff] [review] [review]
> handle zero-sized images in imgFrame::Optimize
> 
> Go straight to hell.

The reasons for which have been communicated out of band.
(Assignee)

Comment 21

6 years ago
The reason this came up is because our ICO decoder (potentially) incorrectly says images with a width or height of 0 actually have that width or height, but various other places disagree and say that its width/height are actually 256. I filed bug 668068 on that issue.

We already handled a 0-height image, but we didn't handle 0-width.
Comment on attachment 542587 [details] [diff] [review]
zero sized image crashtest

This is a poor name for the crash test.
Attachment #542587 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 23

6 years ago
Created attachment 542632 [details] [diff] [review]
Correctly reject both 0-width and 0-height images

Jeff didn't like us allowing 0-height and 0-width images. It turned out that we already rejected 0-height, so I just extended that to reject 0-width too.
Attachment #542585 - Attachment is obsolete: true
Attachment #542632 - Flags: review?(jmuizelaar)
(Assignee)

Comment 24

6 years ago
Created attachment 542634 [details] [diff] [review]
max-sized image crashtest

Due to the above revelations, I'm retitling this crashtest to be max-width, not zero-width.
Attachment #542587 - Attachment is obsolete: true
Comment on attachment 542632 [details] [diff] [review]
Correctly reject both 0-width and 0-height images

<= is better than ==
Attachment #542632 - Flags: review?(jmuizelaar) → review-
(Assignee)

Updated

6 years ago
Attachment #542634 - Flags: review?(jmuizelaar)
(Assignee)

Comment 26

6 years ago
Created attachment 542635 [details] [diff] [review]
correctly reject invalid sizes
Attachment #542632 - Attachment is obsolete: true
Attachment #542635 - Flags: review?(jmuizelaar)
(Assignee)

Comment 27

6 years ago
Created attachment 542638 [details] [diff] [review]
correctly reject invalid sizes

forgot to qref
Attachment #542635 - Attachment is obsolete: true
Attachment #542638 - Flags: review?(jmuizelaar)
Attachment #542635 - Flags: review?(jmuizelaar)
Created attachment 542639 [details]
256 height
Created attachment 542640 [details]
256 width
Attachment #542634 - Flags: review?(jmuizelaar) → review-
Attachment #542638 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 30

6 years ago
Created attachment 542643 [details] [diff] [review]
maximum-width and maximum-height icon crashtests
Attachment #542634 - Attachment is obsolete: true
Attachment #542639 - Attachment is obsolete: true
Attachment #542640 - Attachment is obsolete: true
Attachment #542643 - Flags: review?(jmuizelaar)
Attachment #542643 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

6 years ago
Crash Signature: [@ imgFrame::Optimize()] → [@ imgFrame::Optimize() ]
(Assignee)

Comment 31

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/ff318d0e5d72
http://hg.mozilla.org/integration/mozilla-inbound/rev/098f5469308d

Accidentally labeled the second push under the wrong bug, though.
Whiteboard: [sg:dos null-deref] → [sg:dos null-deref][inbound]
this push, along with bug 552605, greatly increased random oranges in the followint reftest: layout/reftests/svg/as-image/img-and-image-1.html
backed out from inbound since the reftests failures were not something I'd love to merge to central.
fixing the above one or marking as random may be enough, but I don't know what this code does and what the test is supposed to do.

Updated

6 years ago
Whiteboard: [sg:dos null-deref][inbound] → [sg:dos null-deref]
(Assignee)

Comment 34

6 years ago
I marked that reftest as random until dholbert gets a chance to fix the bug.

http://hg.mozilla.org/integration/mozilla-inbound/rev/17f5ec50a7f1
http://hg.mozilla.org/integration/mozilla-inbound/rev/3bc48f2e9899
Whiteboard: [sg:dos null-deref] → [sg:dos null-deref][inbound]
http://hg.mozilla.org/mozilla-central/rev/17f5ec50a7f1
http://hg.mozilla.org/mozilla-central/rev/3bc48f2e9899
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Duplicate of this bug: 679645
Build identifier: Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0

Verified as fixed on Ubuntu: none of the pages specified in comment 3 and comment 14 crashed.
Build identifier: Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0

Verified as fixed on Windows: none of the pages specified in comment 3 and comment 14 crashed.
You need to log in before you can comment on or make changes to this bug.