Closed
Bug 580719
Opened 14 years ago
Closed 14 years ago
Arithmetic exception leads to crash [@hb_ot_layout_context_t::scale_x]
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: posidron, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Attachments
(7 files, 1 obsolete file)
85.51 KB,
application/x-zip
|
Details | |
84.42 KB,
application/zip
|
Details | |
16.56 KB,
text/plain
|
Details | |
4.67 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
1.05 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
2.63 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
103.20 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre Program received signal EXC_ARITHMETIC, Arithmetic exception. 0x00000001017cfd42 in hb_ot_layout_context_t::scale_x (this=0x7fff5fbf3880, v=0) at hb-ot-layout-private.hh:80 80 inline hb_position_t scale_x (int16_t v) { return (int64_t) this->font->x_scale * v / this->face->head_table->unitsPerEm; } Load the provided html file.
Reporter | ||
Comment 1•14 years ago
|
||
Updated•14 years ago
|
blocking2.0: --- → ?
Reporter | ||
Updated•14 years ago
|
Summary: Crash because of division by zero [@ hb_ot_layout_context_t::scale_x] → Arithmetic exception leads to crash [@hb_ot_layout_context_t::scale_x]
Updated•14 years ago
|
blocking2.0: ? → final+
Comment 2•14 years ago
|
||
A lot of the profile is in drawing. I wonder if the dirty regions while typing are wrong?
Assignee: nobody → roc
Assignee | ||
Comment 4•14 years ago
|
||
I am not able to reproduce this since other font robustness fixes have landed recently. Please reconfirm with current trunk build; we may be able to resolve this one as already fixed.
Reporter | ||
Comment 5•14 years ago
|
||
Confirmed.
Assignee | ||
Comment 6•14 years ago
|
||
(In reply to comment #5) > Confirmed. Sorry to re-ask, but could you make that clearer, please - did you mean you're confirming that you still see the crash, or that it's now fixed?
Reporter | ||
Comment 7•14 years ago
|
||
Hi Jonathan, I wasn't able to reproduce it, seems fixed.
Assignee | ||
Comment 8•14 years ago
|
||
OK, thanks. Resolving this as Fixed. I believe this particular testcase was initially fixed as a result of the patch in bug 580212, which causes the "bad" font to be rejected because its 'loca' table is invalid. The patch in bug 580863 would also have fixed this because it detects and rejects the bad upem value in the 'head' table, which was the specific cause of the actual crash here.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 9•14 years ago
|
||
Reopened because this new testcase crashes at the exact same location.
Assignee: jfkthame → christoph.diehl
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 10•14 years ago
|
||
Attachment #459077 -
Attachment is obsolete: true
Reporter | ||
Comment 11•14 years ago
|
||
Number of replaced values: 4 Offset: 284/0x00011c Value: ['22', '9f', 'a8'] (head) Offset: 775/0x000307 Value: ['22', '9f', 'a8'] (hmtx) Offset: 34378/0x00864a Value: ['22', '9f', 'a8'] (glyf) Offset: 74589/0x01235d Value: ['75', '13', '6f'] (glyf) Tag: b'head' Checksum: 0xf2f37c3f Offset: 284/0x0000011c Length: 54 Tag: b'hmtx' Checksum: 0xa6f73c27 Offset: 496/0x000001f0 Length: 2492 Tag: b'glyf' Checksum: 0xa6c28b35 Offset: 5808/0x000016b0 Length: 101752
Assignee | ||
Comment 12•14 years ago
|
||
This prevents harfbuzz trying to use a null blob (due to sanitize() rejection) as a head table (see message on the harfbuzz mailing list).
Assignee: christoph.diehl → jfkthame
Attachment #466296 -
Flags: review?(jdaggett)
Assignee | ||
Comment 13•14 years ago
|
||
Fixing the head table validation exposed a potential null-deref in hb-ot-layout.cc (see message to harfbuzz mailing list).
Attachment #466297 -
Flags: review?(jdaggett)
Assignee | ||
Comment 14•14 years ago
|
||
We could have avoided the harfbuzz issue here with slightly stricter validation of the 'head' table before loading a downloaded font. I think we should add a sanity-check of the tableVersionNumber field, in addition to the harfbuzz fixes (even though they will prevent us actually crashing on this testcase).
Attachment #466298 -
Flags: review?(jdaggett)
Updated•14 years ago
|
Attachment #466297 -
Flags: review?(jdaggett) → review+
Updated•14 years ago
|
Attachment #466298 -
Flags: review?(jdaggett) → review+
Comment 15•14 years ago
|
||
Comment on attachment 466296 [details] [diff] [review] patch, part 1 - [harfbuzz] check head table validity when creating face Looks good. It would be good to have a testcase that covers these cases.
Attachment #466296 -
Flags: review?(jdaggett) → review+
Assignee | ||
Comment 16•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/4fd842de2365 (pt1) http://hg.mozilla.org/mozilla-central/rev/67c26c853bb7 (pt2) http://hg.mozilla.org/mozilla-central/rev/007d994cac53 (pt3) I plan to push a crashtest based on attachment 466256 [details] as a follow-up.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 17•14 years ago
|
||
This is simply attachment 466256 [details] in crashtest form, to verify that it no longer crashes. (It does not actually test ALL the individual fixes here, because the sanity-check in part 3 now prevents the font getting as far as the other points of failure within harfbuzz.)
Attachment #467428 -
Flags: review?(jdaggett)
Updated•14 years ago
|
Attachment #467428 -
Flags: review?(jdaggett) → review+
Assignee | ||
Comment 18•14 years ago
|
||
Pushed crashtest: http://hg.mozilla.org/mozilla-central/rev/b72a0f0c45c5
Reporter | ||
Updated•12 years ago
|
Blocks: fuzzing-fonts
You need to log in
before you can comment on or make changes to this bug.
Description
•