Last Comment Bug 716594 - OOM crash in nsNSSCertificate::Read
: OOM crash in nsNSSCertificate::Read
: crash, regression, topcrash
Product: Core
Classification: Components
Component: Security (show other bugs)
: 11 Branch
: x86 Windows 7
-- critical (vote)
: mozilla13
Assigned To: Benjamin Smedberg [:bsmedberg]
: David Keeler [:keeler] (use needinfo?)
Depends on:
Blocks: 680556
  Show dependency treegraph
Reported: 2012-01-09 10:30 PST by Scoobidiver (away)
Modified: 2012-03-05 16:04 PST (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Make nsBinaryInputStream::ReadBytes use the fallible allocator, rev. 1 (2.51 KB, patch)
2012-01-09 12:28 PST, Benjamin Smedberg [:bsmedberg]
dbaron: review+
Details | Diff | Splinter Review

Description User image Scoobidiver (away) 2012-01-09 10:30:34 PST
It's #42 top browser crasher in 11.0a2 and #116 in 12.0a1.

It first appeared in 11.0a1/20111124. The regression window might be:

Signature 	mozalloc_abort(char const* const) | mozalloc_handle_oom() | nsBinaryInputStream::ReadBytes(unsigned int, char**) More Reports Search
UUID	fc267d8d-3438-4073-8338-eef072120108
Date Processed	2012-01-08 11:18:23.406570
Uptime	26007
Last Crash	2.0 weeks before submission
Install Age	1.2 days since version was first installed.
Install Time	2012-01-07 15:22:20
Product	Firefox
Version	12.0a1
Build ID	20120107031040
Release Channel	nightly
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 22 stepping 1
Crash Address	0xa8195d
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x29c2, AdapterSubsysID: 26991019, AdapterDriverVersion:
D3D10 Layers? D3D10 Layers-
D3D9 Layers? D3D9 Layers-
EMCheckCompatibility	True

Frame 	Module 	Signature [Expand] 	Source
0 	mozalloc.dll 	mozalloc_abort 	memory/mozalloc/mozalloc_abort.cpp:77
1 	mozalloc.dll 	mozalloc_handle_oom 	memory/mozalloc/mozalloc_oom.cpp:54
2 	xul.dll 	nsBinaryInputStream::ReadBytes 	xpcom/io/nsBinaryStream.cpp:728
3 	xul.dll 	nsNSSCertificate::Read 	security/manager/ssl/src/nsNSSCertificate.cpp:1812
4 	xul.dll 	nsNSSSocketInfo::Read 	security/manager/ssl/src/nsNSSIOLayer.cpp:770
5 	xul.dll 	nsBinaryInputStream::ReadObject 	xpcom/io/nsBinaryStream.cpp:795
6 	xul.dll 	NS_DeserializeObject 	netwerk/base/src/nsSerializationHelper.cpp:106
7 	xul.dll 	nsDiskCacheEntry::CreateCacheEntry 	
8 	xul.dll 	nsDiskCacheDevice::FindEntry 	netwerk/cache/nsDiskCacheDevice.cpp:551
9 	xul.dll 	nsCacheService::SearchCacheDevices 	netwerk/cache/nsCacheService.cpp:1905
10 	xul.dll 	nsCacheService::ActivateEntry 	netwerk/cache/nsCacheService.cpp:1806
11 	xul.dll 	nsCacheService::ProcessRequest 	netwerk/cache/nsCacheService.cpp:1651
12 	xul.dll 	nsProcessRequestEvent::Run 	netwerk/cache/nsCacheService.cpp:1021
13 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:660
14 	xul.dll 	nsThreadStartupEvent::`vector deleting destructor' 	
15 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:292
16 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:426
17 		@0xedeb9109 	
18 	msvcr80.dll 	_getptd_noexit 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\tidtable.c:633
19 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:122
20 	msvcr80.dll 	_callthreadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c:348
21 	msvcr80.dll 	_threadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c:326
22 	kernel32.dll 	BaseThreadStart 	

More reports at:*%20const%29%20|%20mozalloc_handle_oom%28%29%20|%20nsBinaryInputStream%3A%3AReadBytes%28unsigned%20int%2C%20char**%29
Comment 1 User image Benjamin Smedberg [:bsmedberg] 2012-01-09 11:32:56 PST
Caused, somewhat intentionally, by bug 680556. This means that nsMemory::Alloc can't return NULL and we don't hit the check at

So presumably either the person is really close to OOM, or we're hitting an unusually or maliciously large certificate...
Comment 2 User image Ted Mielczarek [:ted.mielczarek] 2012-01-09 11:48:05 PST
We should probably give mozalloc_handle_oom a place to indicate the allocation size when we OOM crash. That'd make the diagnosis here easier.
Comment 3 User image Ted Mielczarek [:ted.mielczarek] 2012-01-09 11:51:31 PST
FWIW, from the raw JSON of the crash in comment 0:
"TotalVirtualMemory": "2147352576"
"AvailableVirtualMemory": "1614151680",
"SystemMemoryUsePercentage": "45"
Comment 4 User image Benjamin Smedberg [:bsmedberg] 2012-01-09 12:28:00 PST
Created attachment 587079 [details] [diff] [review]
Make nsBinaryInputStream::ReadBytes use the fallible allocator, rev. 1
Comment 5 User image Ted Mielczarek [:ted.mielczarek] 2012-01-09 12:35:17 PST
I filed bug 716638 on the idea in comment 2.
Comment 6 User image David Baron :dbaron: ⌚️UTC-8 2012-01-17 13:52:13 PST
Comment on attachment 587079 [details] [diff] [review]
Make nsBinaryInputStream::ReadBytes use the fallible allocator, rev. 1

r=dbaron, but you should probably also update the documentation of NS_Alloc etc. in build/nsXPCOM.h since those appear to be infallible but they're documented as returning null on out-of-memory.
Comment 7 User image Alex Keybl [:akeybl] 2012-02-06 16:43:35 PST
Tracking for Firefox 11. Is there any remaining work before landing on m-c and uplifting to Beta 11 and Aurora 12?
Comment 8 User image Benjamin Smedberg [:bsmedberg] 2012-02-07 10:05:10 PST
In absolutely every one of these crashes OOMAllocationSize: "4294901763" (0xFFFF0003). The initial 0xFFFF looks suspicious to me. In any case I will land this patch and file a followup on the odd data.
Comment 9 User image Benjamin Smedberg [:bsmedberg] 2012-02-07 10:06:42 PST
Comment 10 User image Ed Morley [:emorley] 2012-02-08 09:42:27 PST
Comment 11 User image Sheila Mooney 2012-02-27 22:36:31 PST
I don't see any of these crashes on the trunk. Should we uplift to 11?
Comment 12 User image Scoobidiver (away) 2012-02-28 04:07:52 PST
It's #19 top browser crasher in 11.0b4.
The Aurora branch seems unaffected so I don't know if this patch has fixed crashes.
Comment 13 User image Sheila Mooney 2012-02-28 14:38:33 PST
Bsmedberg, is there any reason to take this for beta? Risks? Or should we just let it ride the trains.
Comment 14 User image Benjamin Smedberg [:bsmedberg] 2012-02-29 08:28:16 PST
Well, it's a known regression from something which landed in 11... it's pretty low-risk, reverting to the original behavior. I guess it depends on how you perceive the rewards based on its topcrash ranking, no?
Comment 15 User image Alex Keybl [:akeybl] 2012-02-29 14:06:33 PST
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #14)
> Well, it's a known regression from something which landed in 11... it's
> pretty low-risk, reverting to the original behavior. I guess it depends on
> how you perceive the rewards based on its topcrash ranking, no?

Doesn't it make the most sense to back bug 680556 out of FF11 for our final beta and uplift this patch to FF12 (if necessary)?
Comment 16 User image Benjamin Smedberg [:bsmedberg] 2012-03-01 11:27:50 PST
That's up to you, but I think that's the same amount of change that this is...
Comment 17 User image Alex Keybl [:akeybl] 2012-03-02 11:23:47 PST
We ended up backing out bug 680556 for both this bug and bug 723472. 2 birds, one low risk stone.
Comment 18 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-05 16:04:01 PST
Is there anything QA can do to verify this fix?

Note You need to log in before you can comment on or make changes to this bug.