716594 - OOM crash in nsNSSCertificate::Read

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Scoobidiver (away)]

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:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3c8147998124&tochange=de483d897af4

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 Reason	EXCEPTION_BREAKPOINT
Crash Address	0xa8195d
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x29c2, AdapterSubsysID: 26991019, AdapterDriverVersion: 6.14.10.5218
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:
https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char%20const*%20const%29%20|%20mozalloc_handle_oom%28%29%20|%20nsBinaryInputStream%3A%3AReadBytes%28unsigned%20int%2C%20char**%29

Comment 1

[Benjamin Smedberg]

Caused, somewhat intentionally, by bug 680556. This means that nsMemory::Alloc can't return NULL and we don't hit the check at http://hg.mozilla.org/mozilla-central/annotate/9a230265bad5/xpcom/io/nsBinaryStream.cpp#l729

So presumably either the person is really close to OOM, or we're hitting an unusually or maliciously large certificate...

Comment 2

[(not currently active) Ted Mielczarek]

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

[(not currently active) Ted Mielczarek]

FWIW, from the raw JSON of the crash in comment 0:
"TotalVirtualMemory": "2147352576"
"AvailableVirtualMemory": "1614151680",
"SystemMemoryUsePercentage": "45"

Comment 4

[Benjamin Smedberg]

Attachment: Make nsBinaryInputStream::ReadBytes use the fallible allocator, rev. 1

Comment 5

[(not currently active) Ted Mielczarek]

I filed bug 716638 on the idea in comment 2.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[Alex Keybl [:akeybl]]

Tracking for Firefox 11. Is there any remaining work before landing on m-c and uplifting to Beta 11 and Aurora 12?

Comment 8

[Benjamin Smedberg]

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

[Benjamin Smedberg]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e09c8fd75d4

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7e09c8fd75d4

Comment 11

[Sheila Mooney]

I don't see any of these crashes on the trunk. Should we uplift to 11?

Comment 12

[Scoobidiver (away)]

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

[Sheila Mooney]

Bsmedberg, is there any reason to take this for beta? Risks? Or should we just let it ride the trains.

Comment 14

[Benjamin Smedberg]

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

[Alex Keybl [:akeybl]]

(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

[Benjamin Smedberg]

That's up to you, but I think that's the same amount of change that this is...

Comment 17

[Alex Keybl [:akeybl]]

We ended up backing out bug 680556 for both this bug and bug 723472. 2 birds, one low risk stone.

Comment 18

[u279076]

Is there anything QA can do to verify this fix?