1322660 - Crash in woff2 module

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

[Martin Husemann]

Attachment: Use memset instead of dereferencing a pointer after casting

The woff2 module dereferences pointers after casting to a type requiring higher alignement. This causes crashes on some architectures (will also report this upstream).

Fix is trivial.

Comment 1

[Martin Husemann]

Attachment: Use memset instead of dereferencing a pointer after casting (v2)

Comment 2

[Martin Husemann]

Upstream pull request: https://github.com/google/woff2/pull/69

Comment 3

[Loic]

Martin, what's the status of your patch? Is it reviewable?

Comment 4

[Jan Beich]

[Tracking Requested - why for this release]: Improve stability on strict alignment architectures (e.g. sparc64). Bug 1323303 broke big-endian architectures (e.g. sparc64), so it'd be nice to have a stable ESR base.

Comment 5

[Martin Husemann]

Yes, the patch should be good to review/merge.

Comment 6

[Jonathan Kew [:jfkthame]]

Comment on attachment 8817606 [details] [diff] [review]
Use memset instead of dereferencing a pointer after casting (v2)

Review of attachment 8817606 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, patch looks fine; I think we should go ahead and take this, even though we're still waiting for upstream to act on it.

(This attachment doesn't actually have the necessary patch metadata to be able to land directly; but I'll just fix that up locally and push it.)

Comment 7

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c8f82fbc5cfa2f1e30a2cfe10d6ec743d54be67
Bug 1322660 - Use memcpy() in WOFF2 code instead of dereferencing a pointer after casting to a type with different alignment requirements. r=jfkthame

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1c8f82fbc5cf

Comment 9

[Jan Beich]

Comment on attachment 8817606 [details] [diff] [review]
Use memset instead of dereferencing a pointer after casting (v2)

Approval Request Comment
[ESR consideration]: FF53+ via Skia and Rust require more porting on non-x86 non-arm platforms
[Feature/Bug causing the regression]: bug 1064737 + bug 1227058
[User impact if declined]: Crash on strict-alignment architectures (e.g. sparc64)
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Only downstream on 50.0-52.0
[Needs manual test from QE? If yes, steps to reproduce]: None
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Can only crash with downloaded fonts.
[String changes made/needed]: None

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8817606 [details] [diff] [review]
Use memset instead of dereferencing a pointer after casting (v2)

Fix a crash. Aurora54+ & Beta53+.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a0e707ab7898

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/063278f7c6ee

Comment 13

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Jan's assessment on manual testing needs (see Comment 9) and the fact that this fix has automated coverage.

Comment 14

[Julien Cristau [:jcristau]]

Comment on attachment 8817606 [details] [diff] [review]
Use memset instead of dereferencing a pointer after casting (v2)

avoid unaligned memory access, esr52+

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/9d6d60e64255