startup crash in nsZipHandle::findDataStart

RESOLVED FIXED in Firefox 47

Status

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: philipp, Assigned: bytesized)

Tracking

({crash, regression})

47 Branch
mozilla49
x86
Windows NT
Dependency tree / graph

Firefox Tracking Flags

(firefox47+ fixed, firefox48 fixed, firefox49 fixed)

Details

(crash signature)

Attachments

(1 attachment)

Reporter

Description

3 years ago
[Tracking Requested - why for this release]:
nominating for tracking since it's a recent regression

This bug was filed from the Socorro interface and is 
report bp-43fd92e8-1556-4540-ac47-891212160428.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	nsZipHandle::findDataStart() 	modules/libjar/nsZipArchive.cpp
1 	xul.dll 	nsZipHandle::Init(nsIFile*, nsZipHandle**, PRFileDesc**) 	modules/libjar/nsZipArchive.cpp
2 	xul.dll 	nsZipArchive::OpenArchive(nsIFile*) 	modules/libjar/nsZipArchive.cpp
3 	xul.dll 	mozilla::scache::StartupCache::LoadArchive(mozilla::scache::StartupCache::TelemetrifyAge) 	startupcache/StartupCache.cpp
4 	xul.dll 	mozilla::scache::StartupCache::Init() 	startupcache/StartupCache.cpp
5 	xul.dll 	mozilla::scache::StartupCache::InitSingleton() 	startupcache/StartupCache.cpp
6 	xul.dll 	mozilla::scache::StartupCache::GetSingleton() 	startupcache/StartupCache.cpp
7 	xul.dll 	NS_InitXPCOM2 	xpcom/build/XPCOMInit.cpp
8 	xul.dll 	ScopedXPCOMStartup::Initialize() 	toolkit/xre/nsAppRunner.cpp
9 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
10 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp
11 	xul.dll 	nsAString_internal::ReplacePrep(unsigned int, unsigned int, unsigned int) 	xpcom/string/nsTSubstring.cpp
12 	firefox.exe 	firefox.exe@0x151d3

this new crash signature starts to show up in 47.0b1 and it's occurring on startup. it looks related to the work done in bug 1260836.
Assignee: nobody → ksteuber

Comment 1

3 years ago
In some parts of libjar, we use Windows SEH to trap reads from mapped memory and turn those into errors: see e.g. https://hg.mozilla.org/releases/mozilla-beta/annotate/5bbf2e7c2fc6/modules/libjar/nsZipArchive.cpp#l447

I don't know whether, during this refactoring, you just moved an existing crash to another function (which is probably not a big deal). But if you moved access outside of an SEH block, it may be simple to re-add that and propagate errors instead of crashing. Antivirus is a common cause for memory-mapped files to "disappear" causing these IN_PAGE_ERROR exceptions.
That explains a lot. I was just in the middle of writing a comment noting how these crashes have been seen before (in a different function), but mostly in OSX whereas now we are seeing them in Windows.

I will write a patch to add the windows exception handling. Currently, all crashes with the signature nsZipHandle::findDataStart are in Windows, so this should fix that problem. However, I feel that it is worth pointing out that this bug still gets hit in other operating systems. Before I added nsZipHandle::findDataStart, the first place where the zip file is read is in nsZipArchive::BuildFileList [1]. Looking for crashes with this signature yields a number of crashes in OSX [2]. These have the same stack trace, just crashing in nsZipArchive::BuildFileList instead of nsZipHandle::findDataStart.

[1] https://hg.mozilla.org/releases/mozilla-release/annotate/e35da3da61cb/modules/libjar/nsZipArchive.cpp#l625
[2] https://crash-stats.mozilla.com/report/index/6ebcea83-ce6b-481f-b14b-181572160428
Attachment #8748827 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 8748827 [details]
MozReview Request: Bug 1269185 - Prevent crashes in Windows when zip files cannot be read

https://reviewboard.mozilla.org/r/50551/#review47517
Attachment #8748827 - Flags: review?(spohl.mozilla.bugs) → review+
Any interest in uplifting this, given that Bug 1260836 (Add support for CRX) was uplifted?
Flags: needinfo?(cpearce)
(In reply to Kirk Steuber [:ksteuber] from comment #5)
> Any interest in uplifting this, given that Bug 1260836 (Add support for CRX)
> was uplifted?

Yes! Thanks for the ni?

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b3acf01a84cb
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8748827 [details]
MozReview Request: Bug 1269185 - Prevent crashes in Windows when zip files cannot be read

Approval Request Comment
[Feature/regressing bug #]: Widevine EME
[User impact if declined]: Crashes when we fail to install the Widevine CDM
[Describe test coverage new/current, TreeHerder]: We have tests covering the Widevine CDM installer
[Risks and why]: Low; just adding error checking
[String/UUID change made/needed]: None.
Attachment #8748827 - Flags: approval-mozilla-beta?
Attachment #8748827 - Flags: approval-mozilla-aurora?
Comment on attachment 8748827 [details]
MozReview Request: Bug 1269185 - Prevent crashes in Windows when zip files cannot be read

Fix a crash during Widevine CDM installation, Aurora48+, Beta47+
Attachment #8748827 - Flags: approval-mozilla-beta?
Attachment #8748827 - Flags: approval-mozilla-beta+
Attachment #8748827 - Flags: approval-mozilla-aurora?
Attachment #8748827 - Flags: approval-mozilla-aurora+
Flags: needinfo?(cpearce)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.