Closed Bug 619201 Opened 14 years ago Closed 14 years ago

nsCertOverrideService needs to initialize on the main thread to use the directory service

Categories

(Core :: Security: PSM, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7
Tracking Status
firefox6 --- wontfix
firefox7 --- wontfix
firefox8 + fixed

People

(Reporter: jdm, Assigned: timeless)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

Running Fennec on a restricted wireless network currently brings up an SSL cert error which triggers this assertion: #0 RealBreak () at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:407 #1 0x0258d684 in Break (aMsg= 0xb00fe35c "###!!! ASSERTION: mozHunspellDirProvider not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspe"...) at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:503 #2 0x0258d649 in NS_DebugBreak_P (aSeverity=1, aStr=0x30169e8 "mozHunspellDirProvider not thread-safe", aExpr=0x30169b4 "_mOwningThread.GetThread() == PR_GetCurrentThread()", aFile= 0x30168cc "/home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspellDirProvider.cpp", aLine=45) at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:371 #3 0x022f7367 in mozHunspellDirProvider::AddRef (this=0xb40e01c0) at /home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspellDirProvider.cpp:43 #4 0x02508df3 in NS_TableDrivenQI (aThis=0xb40e01c0, entries=0x328db20, aIID=..., aInstancePtr=0xb00fe86c) at nsISupportsImpl.cpp:49 #5 0x022f75ec in mozHunspellDirProvider::QueryInterface (this=0xb40e01c0, aIID=..., aInstancePtr=0xb00fe86c) at /home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspellDirProvider.cpp:43 #6 0x025053c9 in nsQueryInterface::operator() (this=0xb00fe884, aIID=..., answer=0xb00fe86c) at nsCOMPtr.cpp:47 #7 0x00ff5136 in nsCOMPtr<nsIDirectoryServiceProvider>::assign_from_qi (this=0xb00fe8c8, qi=..., aIID=...) at ../../dist/include/nsCOMPtr.h:1212 #8 0x00ff494c in nsCOMPtr<nsIDirectoryServiceProvider>::nsCOMPtr (this=0xb00fe8c8, qi=...) at ../../dist/include/nsCOMPtr.h:595 #9 0x02548f6a in FindProviderFile (aElement=0xb40e01c0, aData=0xb00fe974) at /home/t_mattjo/src/firefox/mobilebase/xpcom/io/nsDirectoryService.cpp:433 #10 0x02536465 in nsSupportsArray::EnumerateBackwards (this=0xb7dd0d00, aFunc=0x2548d18 <FindProviderFile(nsISupports*, void*)>, aData=0xb00fe974) at /home/t_mattjo/src/firefox/mobilebase/xpcom/ds/nsSupportsArray.cpp:639 #11 0x0254922a in nsDirectoryService::Get (this=0xb7de5100, prop=0x2f3440c "ProfD", uuid=..., result=0xabb2d0cc) at /home/t_mattjo/src/firefox/mobilebase/xpcom/io/nsDirectoryService.cpp:468 #12 0x00fea1df in NS_GetSpecialDirectory (specialDirName=0x2f3440c "ProfD", result=0xabb2d0cc) at ../../dist/include/nsDirectoryServiceUtils.h:58 #13 0x01fc5f97 in nsCertOverrideService::Init (this=0xabb2d0b0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsCertOverrideService.cpp:152 #14 0x01ff0881 in nsCertOverrideServiceConstructor (aOuter=0x0, aIID=..., aResult=0xb00feb38) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsNSSModule.cpp:259 #15 0x02514d28 in mozilla::GenericFactory::CreateInstance (this=0xae173750, aOuter=0x0, aIID=..., aResult=0xb00feb38) at GenericFactory.cpp:48 #16 0x0257244f in nsComponentManagerImpl::CreateInstanceByContractID (this=0xb7dc3220, aContractID=0x2f3c3d4 "@mozilla.org/security/certoverride;1", aDelegate=0x0, aIID=..., aResult=0xb00feb38) at /home/t_mattjo/src/firefox/mobilebase/xpcom/components/nsComponentManager.cpp:1315 #17 0x025730c2 in nsComponentManagerImpl::GetServiceByContractID (this=0xb7dc3220, aContractID=0x2f3c3d4 "@mozilla.org/security/certoverride;1", aIID=..., result=0xb00fec2c) at /home/t_mattjo/src/firefox/mobilebase/xpcom/components/nsComponentManager.cpp:1676 #18 0x025063a2 in CallGetService (aContractID=0x2f3c3d4 "@mozilla.org/security/certoverride;1", aIID=..., aResult=0xb00fec2c) at nsComponentManagerUtils.cpp:94 #19 0x02506890 in nsGetServiceByContractID::operator() (this=0xb00fec44, aIID=..., aInstancePtr=0xb00fec2c) at nsComponentManagerUtils.cpp:278 #20 0x01fdfb48 in nsCOMPtr<nsICertOverrideService>::assign_from_gs_contractid (this=0xb00fed70, gs=..., aIID=...) at ../../../../dist/include/nsCOMPtr.h:1252 #21 0x01fdf504 in nsCOMPtr<nsICertOverrideService>::nsCOMPtr (this=0xb00fed70, gs=...) at ../../../../dist/include/nsCOMPtr.h:627 #22 0x01fe9efa in nsNSSBadCertHandler (arg=0xb058f710, sslSocket=0xae1bf0e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsNSSIOLayer.cpp:3519 #23 0x0036aaa6 in ssl3_HandleCertificate (ss=0xae162000, b= 0xae1a0388 "\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245"..., length=0) at ssl3con.c:7911 #24 0x0036c49a in ssl3_HandleHandshakeMessage (ss=0xae162000, b=0xae1a0004 "", length=900) at ssl3con.c:8603 #25 0x0036c928 in ssl3_HandleHandshake (ss=0xae162000, origBuf=0xae162250) at ssl3con.c:8727 #26 0x0036d40a in ssl3_HandleRecord (ss=0xae162000, cText=0xb00ff08c, databuf=0xae162250) at ssl3con.c:9066 #27 0x0036e527 in ssl3_GatherCompleteHandshake (ss=0xae162000, flags=0) at ssl3gthr.c:209 #28 0x0037117b in ssl_GatherRecord1stHandshake (ss=0xae162000) at sslcon.c:1258 #29 0x0037c824 in ssl_Do1stHandshake (ss=0xae162000) at sslsecur.c:151 #30 0x0037ea68 in ssl_SecureSend (ss=0xae162000, buf= 0xae110000 "GET /en-US/mobile/api/1.5/list/featured/all/4/Linux/4.0b3pre HTTP/1.1\r\nHost: services.addons.mozilla.org\r\nUser-Agent: Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101213 Firefox/4.0b8pre Fennec/"..., len=631, flags=0) at sslsecur.c:1213 #31 0x0037ec22 in ssl_SecureWrite (ss=0xae162000, buf= 0xae110000 "GET /en-US/mobile/api/1.5/list/featured/all/4/Linux/4.0b3pre HTTP/1.1\r\nHost: services.addons.mozilla.org\r\nUser-Agent: Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101213 Firefox/4.0b8pre Fennec/"..., len=631) at sslsecur.c:1258 #32 0x0038628f in ssl_Write (fd=0xae1bf0e0, buf=0xae110000, len=631) at sslsock.c:1652 #33 0x01fccf3c in nsSSLThread::Run (this=0xb01705e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsSSLThread.cpp:1045 #34 0x01fcb26b in nsPSMBackgroundThread::nsThreadRunner (arg=0xb01705e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsPSMBackgroundThread.cpp:44 ---Type <return> to continue, or q <return> to quit--- #35 0x00182797 in _pt_root (arg=0xb037fa80) at /home/t_mattjo/src/firefox/mobilebase/nsprpub/pr/src/pthreads/ptthread.c:187 #36 0x008ba925 in start_thread () from /lib/libpthread.so.0 #37 0x007e407e in clone () from /lib/libc.so.6 (gdb) fr 13 #13 0x01fc5f97 in nsCertOverrideService::Init (this=0xabb2d0b0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsCertOverrideService.cpp:152 152 NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(mSettingsFile)); (gdb) list 147 148 mDottedOidForStoringNewHashes = dotted_oid; 149 PR_smprintf_free(dotted_oid); 150 151 // cache mSettingsFile 152 NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(mSettingsFile)); 153 if (mSettingsFile) { 154 mSettingsFile->AppendNative(NS_LITERAL_CSTRING(kCertOverrideFileName)); 155 } 156
Yeah, quite clearly nsCertOverrideService::Init should not be calling into the directory service off the main thread. This can be a cause of crashes. The possibilities are: * We should pre-create nsCertOverrideService on the main thread * We should not be creating the nsCertOverrideService off the main thread * ? Who knows the code?
Component: Security → Security: PSM
QA Contact: toolkit → psm
Summary: ASSERTION: mozHunspellDirProvider not thread-safe → nsCertOverrideService uses directory service off the main thread
Summary: nsCertOverrideService uses directory service off the main thread → nsCertOverrideService needs to initialize on the main thread to use the directory service
Attached patch add to the ball of wax (deleted) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #497777 - Flags: review?(kaie)
Attachment #497777 - Flags: approval2.0?
Comment on attachment 497777 [details] [diff] [review] add to the ball of wax Is it sufficent to create the service once, because XPCOM will keep it alive? If yes, r=kaie Or will the service be deleted on leaving the scope, and will need to be constructed again later? If yes, r- and please add a member variable nsNSSComponent::mCertOverrServ to keep a reference.
Attachment #497777 - Flags: review?(kaie) → review+
question has been answered, once a service is alive, XPCOM will keep it alive until shutdown. patch is sufficient.
Comment on attachment 497777 [details] [diff] [review] add to the ball of wax If Fennec wants this, Fennec drivers can ask for it.
Attachment #497777 - Flags: approval2.0? → approval2.0-
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
Depends on: 650892
Depends on: 650858
If we implement the design in bug 511393 comment 14, then we can make nsICertOverrideService main-thread only, which will provide an alternate solution for this bug. Besides causing bug 650858, the current solution to this bug has the added disadvantage of adding disk I/O on the main thread at startup. So, my recommendation is to backout this patch on all trees (including mozilla-central) and then fix bug 511393. If backing out the patch is problematic (i.e. we have some evidence that it really does cause crashes), then I will hack together an alternate fix for this bug that avoids causing bug 650858.
I backed this out of beta6 too: http://hg.mozilla.org/releases/mozilla-beta/rev/a9bc7596542b So bug 650858 doesn't affect it anymore. I have not backed it out of aurora or central.
Target Milestone: mozilla5 → mozilla7
qa- as QA does not need to verify this fix.
Whiteboard: [qa-]
Also backed out of beta-8: https://hg.mozilla.org/releases/mozilla-beta/rev/dc9713bcaa93 The patch for bug 679140 fixes this in a different way, by ensuring that we only use the cert override service on the main thread, so that it is guaranteed to be initialized on the main thread without having to do so in nsNSSComponent::Init().
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: