Crash in memcpy | NS_CopySegmentToBuffer rising in Firefox 49

VERIFIED FIXED in Firefox 52

Status

()

Core
DOM
--
critical
VERIFIED FIXED
11 months ago
10 months ago

People

(Reporter: baku, Assigned: baku)

Tracking

({crash})

49 Branch
mozilla53
x86
Windows
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 wontfix, firefox51+ wontfix, firefox52+ verified, firefox53+ verified)

Details

(crash signature)

Attachments

(1 attachment)

(Assignee)

Description

11 months ago
See: https://bugzilla.mozilla.org/show_bug.cgi?id=1330273#c26
(Assignee)

Updated

11 months ago
Assignee: nobody → amarchesini
(Assignee)

Comment 1

11 months ago
Created attachment 8828752 [details] [diff] [review]
fr.patch
Attachment #8828752 - Flags: review?(bugs)
Comment on attachment 8828752 [details] [diff] [review]
fr.patch

ok, the limit is coming from ArrayBufferObject::setByteLength
but make >= just >
Attachment #8828752 - Flags: review?(bugs) → review+

Comment 3

11 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b5cc104aaf6
FileReader cannot allocate more than INT32_MAX for an ArrayBuffer, r=smaug

Comment 4

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9b5cc104aaf6
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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.
status-firefox50: --- → wontfix
status-firefox51: --- → affected
status-firefox52: --- → affected
tracking-firefox51: --- → ?
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
Flags: needinfo?(amarchesini)
(Assignee)

Comment 6

11 months ago
We can uplift this only if we also uplift 1332602. Are we OK with it?
Flags: needinfo?(amarchesini) → needinfo?(ryanvm)
Assuming there isn't a big risk in doing so, sure.
Flags: needinfo?(ryanvm)
Flags: needinfo?(amarchesini)
(Assignee)

Comment 8

11 months ago
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
Flags: needinfo?(amarchesini)
Attachment #8828752 - Flags: approval-mozilla-aurora?
Attachment #8828752 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8828752 [details] [diff] [review]
fr.patch

check for files > 2GB in FileReader, beta52+
Attachment #8828752 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 10

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/391cffbadd09
status-firefox52: affected → fixed

Comment 11

10 months ago
Track 51+ as 51 is affected.
tracking-firefox51: ? → +
Flagging this for verification per Comment 8. Instructions available in Comment 0.
Flags: qe-verify+
tracking-firefox52: ? → +
tracking-firefox53: ? → +

Comment 13

10 months ago
Too late for 51 and the volume of crash is low now. Mark 51 as won't fix.
status-firefox51: affected → wontfix
Duplicate of this bug: 1330273
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.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.