885292 - crash in nsSMimeVerificationListener::Notify @ nsXPCWrappedJS::AddRef when opening a signed email with S/mime

Status: RESOLVED WORKSFORME

(Thunderbird :: Security, defect)

Description

[Ludovic Hirlimann [:Usul]]

This bug was filed from the Socorro interface and is 
report bp-06ee5cc7-b2e5-4025-9ba3-295482130620 .
============================================================= 
0 	XUL 	nsXPCWrappedJS::AddRef 	js/xpconnect/src/XPCWrappedJS.cpp:158
1 	XUL 	nsSMimeVerificationListener::Notify 	objdir-tb/x86_64/mozilla/dist/include/nsCOMPtr.h:578
2 	XUL 	NS_TableDrivenQI 	objdir-tb/x86_64/mozilla/xpcom/build/nsISupportsImpl.cpp:16
3 	XUL 	nsSMimeVerificationJob::Run 	security/manager/ssl/src/nsCertVerificationThread.cpp:85
4 	XUL 	nsCertVerificationThread::Run 	security/manager/ssl/src/nsCertVerificationThread.cpp:143
5 	libnspr4.dylib 	libnspr4.dylib@0x1dad0 	
6 	libnspr4.dylib 	_pt_root 	nsprpub/pr/src/pthreads/ptthread.c:204
7 	libsystem_c.dylib 	libsystem_c.dylib@0x4e8bf 	
8 	libsystem_c.dylib 	libsystem_c.dylib@0x51b75 	
9 	libnspr4.dylib 	

Looking at https://hg.mozilla.org/comm-central I see 867437 and 882424 as potential regrssions.

Comment 1

[Scoobidiver (away)]

The stack trace says it's caused by the feature of bug 770840.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

I think mListener in nsSMimeVerificationJob has to be nsMainThreadPtrHandle too like the patch for bug 873615 does.

Comment 3

[Josh Matthews [:jdm]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> I think mListener in nsSMimeVerificationJob has to be nsMainThreadPtrHandle
> too like the patch for bug 873615 does.

I am doubtful that there are any JS implementors of nsISmimeVerificationListener. mxr.mozilla.org/addons/ certainly doesn't find any.

Comment 4

[Josh Matthews [:jdm]]

It looks more likely to me that one of these lines in Notify is triggering the problem:

>318   nsCOMPtr<nsICMSMessage> msg = do_QueryInterface(aVerifiedMessage);
>319   NS_ENSURE_TRUE(msg, NS_ERROR_FAILURE);
>320   
>321   nsCOMPtr<nsIX509Cert> signerCert;
>322   msg->GetSignerCert(getter_AddRefs(signerCert));

Comment 5

[Mark Banner (:standard8)]

Is this still happening? and is this really a core issue?

Comment 6

[Ludovic Hirlimann [:Usul]]

(In reply to Mark Banner (:standard8) from comment #5)
> Is this still happening? and is this really a core issue?

Nope - don't know.

Comment 7

[Mark Banner (:standard8)]

Not tracking for TB 24, as we don't seem to be hitting this any more. Could be a WFM?

Comment 8

[Wayne Mery (:wsmwk)]

(In reply to Mark Banner (:standard8) from comment #7)
> Not tracking for TB 24, as we don't seem to be hitting this any more. Could
> be a WFM?

last crashes are tb24.0a1 - approx 10 users in last 8 weeks. All with nsSMimeVerificationListener::Notif on the stack. i.e. zero TB24 beta crashes

Quite unclear to me which bug fixed this. None were using calendar - fixed Bug 870887, and all other bugs with this signature** have different stacks - which coincidentally have all been fixed in this general time period.

bug 870916 RESOLVED FIXED crash in nsSocketTransport::BuildSocket @ nsXPTCStubBase::AddRef
bug 882125 RESOLVED DUPLICATE Remove Off-Main-Thread XPCWrappedJS refcounting from nsLDAPOperation
bug 882196 RESOLVED FIXED Android crash in nsXPCWrappedJS::AddRef
bug 882200 RESOLVED FIXED crash in nsLDAPOperation::GetMessageListener @ nsXPCWrappedJS::AddRef
bug 882328 RESOLVED FIXED nsICameraGetCameraCallback addref-ed on non-Main thread
bug 882424 RESOLVED FIXED crash in SignedStatusRunnable::SignedStatusRunnable @ nsXPCWrappedJS::AddRef
bug 882637 RESOLVED DUPLICATE crash in mozilla::nsDOMCameraControl::Result @ nsXPCWrappedJS::AddRef