1332587 - Crash in memcpy | NS_CopySegmentToBuffer rising in Firefox 49

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Andrea Marchesini [:baku]]

See: https://bugzilla.mozilla.org/show_bug.cgi?id=1330273#c26

Comment 1

[Andrea Marchesini [:baku]]

Attachment: fr.patch

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8828752 [details] [diff] [review]
fr.patch

ok, the limit is coming from ArrayBufferObject::setByteLength
but make >= just >

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b5cc104aaf6
FileReader cannot allocate more than INT32_MAX for an ArrayBuffer, r=smaug

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9b5cc104aaf6

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Setting 51 to affected since this is something we may want to keep on the radar for a dot release ride-along if such a thing comes into being. Otherwise, this feels edge-casey enough that we could uplift to 52 and let it ride the trains from there.

Comment 6

[Andrea Marchesini [:baku]]

We can uplift this only if we also uplift 1332602. Are we OK with it?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Assuming there isn't a big risk in doing so, sure.

Comment 8

[Andrea Marchesini [:baku]]

Comment on attachment 8828752 [details] [diff] [review]
fr.patch

Approval Request Comment
[Feature/Bug causing the regression]: FileReader
[User impact if declined]: a crash if the size of the buffer is > INT32_MAX
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes in bug 1332602
[Needs manual test from QE? If yes, steps to reproduce]: follow bug 1332602
[List of other uplifts needed for the feature/fix]: 1332602 _must_ be uplift as well.
[Is the change risky?]: no
[Why is the change risky/not risky?]: Just a size check
[String changes made/needed]: none

Comment 9

[Julien Cristau [:jcristau]]

Comment on attachment 8828752 [details] [diff] [review]
fr.patch

check for files > 2GB in FileReader, beta52+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/391cffbadd09

Comment 11

[Gerry Chang [:gchang]]

Track 51+ as 51 is affected.

Comment 12

[Andrei Vaida [:avaida]]

Flagging this for verification per Comment 8. Instructions available in Comment 0.

Comment 13

[Gerry Chang [:gchang]]

Too late for 51 and the volume of crash is low now. Mark 51 as won't fix.

Comment 15

[Emil Ghitta, QA [:emilghitta]]

I've reproduced the issue described in comment https://bugzilla.mozilla.org/show_bug.cgi?id=1330273#c25 using 53.0a1 Nightly (Build Id:20170116030326,Crash Signature: bp-e44065d7-1831-436b-afc1-f7b9d2170223)and on 52.0a2 Aurora (Build Id:20170117004014, Crash Signature: bp-1762c275-0711-44e8-a147-63a892170223). 

I have verified that the issue is not reproducible using 52.0b8 (Build Id:20170220070057) and using 53.0a2 (Build Id:20170221004019) on Windows 10 64bit.