Closed Bug 595924 Opened 14 years ago Closed 14 years ago

Firefox 4.0b5 Crash Report [@ crc32_little ]

Categories

(Core :: Networking: JAR, defect)

x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: cbook, Assigned: taras.mozilla)

References

Details

(Keywords: crash, Whiteboard: [crashkill])

Crash Data

Attachments

(1 file, 3 obsolete files)

Crash on Firefox 4.0Beta5 and older branches from Chofmann's Crashstats list. It seems this is Windows only http://crash-stats.mozilla.com/report/list?signature=crc32_little Frame Module Signature [Expand] Source 0 xul.dll crc32_little modules/zlib/src/crc32.c:279 1 xul.dll MOZ_Z_crc32 modules/zlib/src/crc32.c:237 2 xul.dll nsZipArchive::CheckCRC modules/libjar/nsZipArchive.cpp:741 3 xul.dll mozilla::scache::StartupCache::GetBuffer 4 xul.dll mozJSComponentLoader::ReadScript js/src/xpconnect/loader/mozJSComponentLoader.cpp:895 5 xul.dll mozJSComponentLoader::GlobalForLocation js/src/xpconnect/loader/mozJSComponentLoader.cpp:1036 6 xul.dll mozJSComponentLoader::LoadModuleImpl js/src/xpconnect/loader/mozJSComponentLoader.cpp:727 7 xul.dll mozJSComponentLoader::LoadModuleFromJAR js/src/xpconnect/loader/mozJSComponentLoader.cpp:692 8 xul.dll nsComponentManagerImpl::KnownModule::Load
It is #6 top crasher in b7pre/20100920 build.
Component: General → Networking: JAR
QA Contact: general → networking.jar
ccing startup cache author, since it seems at least plausible that it's just feeding bogus data into libjar.
blocking2.0: --- → ?
Assignee: nobody → bhsieh
blocking2.0: ? → final+
Could we email the affected users for their startupCache.4.little file? (In reply to comment #2) > ccing startup cache author, since it seems at least plausible that it's just > feeding bogus data into libjar. Sure but we shouldn't crash on bogus zips.
Is it a bogus zip, or bogus other data?
(In reply to comment #4) > Is it a bogus zip, or bogus other data? http://msdn.microsoft.com/en-us/library/aa366801%28VS.85%29.aspx Looks this isn't as much of a weird corner case as I originally expected. Windows does this if a network resource disappears(i'm guessing there may be other more reasons for it too). Would be nice to email users to see if they have any network shares that their profiles or startup cache are residing on. To confirm this. IF this turns out to be a problem as described in docs, we can either pepper our jar code with try__ stuff OR just detect files on removable drives + network shares and memcpy those files into our own buffer at expense of increased memory usage. Both seem reasonable to me if we can track down the cause.
Depends on: 598416
I think the approach will be: get the _try around nsZipCursor::Read in Bug 598416, then change startupCache to use nsZipCursor::Read and remove the CheckCrc method from the nsZipArchive class entirely.
Attachment #479233 - Flags: review? → review?(tglek)
Comment on attachment 479233 [details] [diff] [review] removes checkCRC, uses zipcursor in startupcache >+ nsZipCursor cursor(zipItem, mArchive, NULL, 0, true); >+ data = (char*) cursor.Read(&len); >+ NS_ASSERTION(len == zipItem->Size(), "Amount read doesn't match the size"); > } > } > > if (data) { > *outbuf = new char[len]; > memcpy(*outbuf, data, len); > *length = len; > return NS_OK; Say no to scoping nsZipCursor and data it holds differently, this is guaranteed to deref free()ed memory in the case of reading from a compressed jar.
Attachment #479233 - Flags: review?(tglek) → review-
Attached patch scopes cursor stuff correctly (obsolete) — Splinter Review
Oh, that makes sense.
Attachment #479233 - Attachment is obsolete: true
Attachment #479238 - Flags: review?(tglek)
Comment on attachment 479238 [details] [diff] [review] scopes cursor stuff correctly Thanks for the quick fix. Can you check this in along with my bug that this depends on?
Attachment #479238 - Flags: review?(tglek) → review+
Yep, will do once the tree opens.
Blocks: 586859
Blocks: 601682
Comment on attachment 479238 [details] [diff] [review] scopes cursor stuff correctly >+ nsZipCursor cursor(zipItem, mArchive, NULL, 0, true); >+ data = (char*) cursor.Read(&len); this returns a null on error. Need to check it. Just got burned by that.
Attachment #479238 - Flags: review+ → review-
Attached patch checks data for null (obsolete) — Splinter Review
Thanks for the catch.
Attachment #479238 - Attachment is obsolete: true
Attachment #481673 - Flags: review?(tglek)
Assignee: bhsieh → tglek
Attachment #481673 - Attachment is obsolete: true
Attachment #481931 - Flags: review?(mwu)
Attachment #481673 - Flags: review?(tglek)
Blocks: 562406
Comment on attachment 481931 [details] [diff] [review] Use a convenience class to access zip Would be nice if startup cache could do zerocopy, but it probably doesn't matter if we go for compressed startupcache.
Attachment #481931 - Flags: review?(mwu) → review+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Crash Signature: [@ crc32_little ]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: