Closed
Bug 1159604
Opened 9 years ago
Closed 9 years ago
OOM crash in Firefox 37.0.2 when pasting large amounts of data
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: xowiatech, Assigned: away)
References
Details
(Keywords: crash, crashreportid)
Crash Data
Attachments
(2 files, 1 obsolete file)
2.61 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.20 KB,
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
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•9 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•9 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>]
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•9 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)
> 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)
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.
Well I'll take it if nobody else will.
Assignee: nobody → dmajor
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
I find it amusing that in its hg-lifetime, this file has had exactly one functional change and 15 stylistic changes.
Attachment #8600605 -
Flags: review?(bugs)
Comment 9•9 years ago
|
||
Don't we want to use moz_malloc, since free() (not delete) is used for releasing the memory?
Assignee | ||
Comment 10•9 years ago
|
||
That's a good point. I was following glandium's dev-platform advice a little too aggressively!
Attachment #8600605 -
Attachment is obsolete: true
Attachment #8600605 -
Flags: review?(bugs)
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8600906 -
Flags: review?(bugs)
Comment 12•9 years ago
|
||
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•9 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 | ||
Comment 14•9 years ago
|
||
Limited try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=122f4c838dc1
Keywords: checkin-needed
Comment 15•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1163109f2895
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1163109f2895
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment 18•9 years ago
|
||
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•9 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)
Comment 20•9 years ago
|
||
(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•9 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•9 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•9 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•9 years ago
|
||
Well that sounds like a good opportunity for cleanup, independent of this bug.
Comment 25•9 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 | ||
Comment 26•9 years ago
|
||
Attachment #8636860 -
Flags: review?(netzen)
Comment 27•9 years ago
|
||
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+
Status: RESOLVED → REOPENED
status-firefox40:
fixed → ---
Resolution: FIXED → ---
Target Milestone: mozilla40 → ---
Comment 29•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/275301e64d0e
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Reporter | ||
Comment 30•9 years ago
|
||
Is this issue is resolved? If yes, then why it is not mentioned on release notes of 42.0?
Comment 31•9 years ago
|
||
(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.
Comment 32•7 years ago
|
||
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.
Description
•