OOM crash in Firefox 37.0.2 when pasting large amounts of data

RESOLVED FIXED in Firefox 42

Status

()

Core
General
--
critical
RESOLVED FIXED
3 years ago
9 months ago

People

(Reporter: Xowia Technologies, Assigned: dmajor)

Tracking

({crash, crashreportid})

37 Branch
mozilla42
x86_64
Windows 7
crash, crashreportid
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

(crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150415140819

Steps to reproduce:

When you visit site like http://www.lettercount.com/ or any other site where you can put enter your BIG DATA (~250 Million Digit) like (1234512345123451234512345123451234512345123451234512345.....12345)


Actual results:

When you enter BIG DATA so Firefox will not handle it and application will crash.


Expected results:

It should process the data either give some kind of error instead of crashing application.
(Reporter)

Updated

3 years ago
Crash Signature: 1) bp-df9a7d84-9520-448d-961b-e71e92150429 2) bp-4a12bf72-843e-4d44-a412-6e7f02150429
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64

Comment 1

3 years ago
Crash signatures:

1) bp-df9a7d84-9520-448d-961b-e71e92150429
2) bp-4a12bf72-843e-4d44-a412-6e7f02150429
Group: core-security
Severity: normal → critical
Crash Signature: 1) bp-df9a7d84-9520-448d-961b-e71e92150429 2) bp-4a12bf72-843e-4d44-a412-6e7f02150429 → [@ OOM | large | mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | ConvertUnknownBreaks<wchar_t>]
Component: Untriaged → Untriaged
Keywords: crash, crashreportid
Product: Firefox → Core
Summary: Firefox 37.0.2 crashing due to BIG Data → OOM crash in Firefox 37.0.2 when pasting large amounts of data

Comment 2

3 years ago
Benjamin, looks like this crashes in xpcom/io... can we make things there not crash and burn if they get too much data, and/or should we change the clipboard handling code to truncate large amounts of text or something, or...? :-)
Flags: needinfo?(benjamin)

Comment 3

3 years ago
302 dmajor
Flags: needinfo?(benjamin) → needinfo?(dmajor)
(Assignee)

Comment 4

3 years ago
> OOM Allocation Size 	 620046000
Uh, yeah, that's bad.

The feature owners can weigh in if they want something more fancy, but at the very least it should be a fallible allocation. There's a null-check there already :-( I guess the code didn't know that nsMemory::Alloc was infallible.
Flags: needinfo?(dmajor)
(Assignee)

Comment 5

3 years ago
I am pretty sure I've seen this pattern before, where code calls an infallible allocation but is prepared to handle failure. It might be worthwhile auditing all callers of nsMemory::Alloc (now moz_xmalloc in m-c) for this issue.
(Assignee)

Comment 6

3 years ago
Well I'll take it if nobody else will.
Assignee: nobody → dmajor
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 7

3 years ago
I find it amusing that in its hg-lifetime, this file has had exactly one functional change and 15 stylistic changes.
(Assignee)

Comment 8

3 years ago
Created attachment 8600605 [details] [diff] [review]
Use fallible allocations
Attachment #8600605 - Flags: review?(bugs)
Don't we want to use moz_malloc, since free() (not delete) is used for releasing the memory?
(Assignee)

Comment 10

3 years ago
That's a good point. I was following glandium's dev-platform advice a little too aggressively!
(Assignee)

Updated

3 years ago
Attachment #8600605 - Attachment is obsolete: true
Attachment #8600605 - Flags: review?(bugs)
(Assignee)

Comment 11

3 years ago
Created attachment 8600906 [details] [diff] [review]
Use fallible allocations
Attachment #8600906 - Flags: review?(bugs)
Comment on attachment 8600906 [details] [diff] [review]
Use fallible allocations

Based on https://developer.mozilla.org/en-US/docs/Infallible_memory_allocation
we should probably use moz_malloc here. With that, r+
Attachment #8600906 - Flags: review?(bugs) → review+
(Assignee)

Comment 13

3 years ago
https://groups.google.com/forum/#!topic/mozilla.dev.platform/ffSQld7olEE
"PSA: moz_malloc, moz_realloc, moz_calloc and moz_free are no more"

I'll ping glandium to update the MDN pages. There's some old prose in there that I'm not sure about so I don't feel comfortable just blindly doing s/moz_malloc/malloc/ by myself.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1163109f2895
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
I cannot crash it on my Win 7 machine, but Socorro data ([1], [2]) seems to indicate that this no longer happens in Firefox 40 or later.

Reporter, does this work for you now? The fix should be in the latest Firefox 40 build: ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/40.0b4-candidates/build1/win32/en-US/

[1] - https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=OOM+%7C+large+%7C+mozalloc_abort%28char+const%2A+const%29+%7C+mozalloc_handle_oom%28unsigned+int%29+%7C+moz_xmalloc+%7C+ConvertUnknownBreaks%3Cwchar_t%3E#tab-sigsummary
[2] - https://crash-stats.mozilla.com/report/list?signature=OOM%20%7C%20large%20%7C%20mozalloc_abort(char%20const*%20const)%20%7C%20mozalloc_handle_oom(unsigned%20int)%20%7C%20moz_xmalloc%20%7C%20ConvertUnknownBreaks%3CT%3E
Crash Signature: [@ OOM | large | mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | ConvertUnknownBreaks<wchar_t>] → [@ OOM | large | mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | ConvertUnknownBreaks<wchar_t>] [@ OOM | large | mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | ConvertUnknownBre…
Flags: needinfo?(xowiatech)
(Reporter)

Comment 19

3 years ago
Are you sure? Check these crash report ids.
1) Firefox 39.0 - bp-c44f27d7-3137-4ca1-b056-97ed72150721
2) Firefox 40.0b- bp-bca36083-37b4-4bff-b0ac-55e9d2150721
Flags: needinfo?(xowiatech)
(In reply to Xowia Technologies from comment #19)
> Are you sure? Check these crash report ids.
> 1) Firefox 39.0 - bp-c44f27d7-3137-4ca1-b056-97ed72150721
> 2) Firefox 40.0b- bp-bca36083-37b4-4bff-b0ac-55e9d2150721

So what you mean is that you're still crashing with the same steps as originally reported?
(Assignee)

Comment 21

3 years ago
The crash has moved somewhere else, now that the line-break code is no longer the first point of failure.
(Assignee)

Comment 22

3 years ago
It's frustrating to see so many moz_xmalloc's where the return value is null-checked. The latest one is nsClipboard::GetGlobalData in bp-bca36083-37b4-4bff-b0ac-55e9d2150721. I suspect that these codepaths were originally null-safe but ended up with moz_xmalloc through refactoring.

Ehsan, how hard would it be to write static analysis to catch these cases?
Flags: needinfo?(ehsan)

Comment 23

3 years ago
You mean checking places where we null check the result of an infallible allocation?  It is possible to detect, yes, but we do that all over the tree, so I'm afraid we'd only see the forest then, not the trees that you're looking for...
Flags: needinfo?(ehsan)
(Assignee)

Comment 24

3 years ago
Well that sounds like a good opportunity for cleanup, independent of this bug.

Comment 25

3 years ago
I agree.  I just don't want to sign up to do that work.  :-)  Please feel free to file a bug, it's at least a good idea to keep it tracked.
(Assignee)

Updated

3 years ago
See Also: → bug 1186212
(Assignee)

Comment 26

3 years ago
Created attachment 8636860 [details] [diff] [review]
Use more fallible allocations
Attachment #8636860 - Flags: review?(netzen)
Comment on attachment 8636860 [details] [diff] [review]
Use more fallible allocations

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

Thanks. Please either re-ope this bug or put into a new follow up bug.
Attachment #8636860 - Flags: review?(netzen) → review+
(Assignee)

Updated

3 years ago
Status: RESOLVED → REOPENED
status-firefox40: fixed → ---
Resolution: FIXED → ---
Target Milestone: mozilla40 → ---
https://hg.mozilla.org/mozilla-central/rev/275301e64d0e
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42

Updated

3 years ago
Blocks: 1192040
(Reporter)

Comment 30

3 years ago
Is this issue is resolved? If yes, then why it is not mentioned on release notes of 42.0?
(In reply to Xowia Technologies from comment #30)
> Is this issue is resolved?

It looks like the most recent patch landed in Firefox 42, based on comment 29.

> If yes, then why it is not mentioned on release notes of 42.0?

The release notes are a very small subset of things that have been fixed in a given release.
Moving from Core::Untriaged to Core::General https://bugzilla.mozilla.org/show_bug.cgi?id=1407598
Component: Untriaged → General
You need to log in before you can comment on or make changes to this bug.