1806247 - Crash in [@ nsMsgComposeSecure::MimeFinishMultipartSigned]

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Wayne Mery (:wsmwk)]

Crash exists in version 91.

Crash report: https://crash-stats.mozilla.org/report/index/807e04ef-7bf6-4dab-93a4-69a220221216

Reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 10 frames of crashing thread:

0  XUL  nsMsgComposeSecure::MimeFinishMultipartSigned  mailnews/extensions/smime/nsMsgComposeSecure.cpp:616
1  XUL  nsMsgComposeSecure::MimeFinishEncryption  mailnews/extensions/smime/nsMsgComposeSecure.cpp:735
2  XUL  _NS_InvokeByIndex  
3  XUL  XPCWrappedNative::CallMethod  js/xpconnect/src/XPCWrappedNative.cpp:1125
4  XUL  XPC_WN_CallMethod  js/xpconnect/src/XPCWrappedNativeJSOps.cpp:963
5  XUL  js::InternalCallOrConstruct  js/src/vm/Interpreter.cpp:507
6  XUL  Interpret  js/src/vm/Interpreter.cpp:3314
7  XUL  js::RunScript  js/src/vm/Interpreter.cpp:389
8  XUL  js::InternalCallOrConstruct  js/src/vm/Interpreter.cpp:539
9  XUL  js::Call  js/src/vm/Interpreter.cpp:605

Comment 1

[Wayne Mery (:wsmwk)]

Some crashes have add-ons, but some don't have addons

Comment 2

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/releases/comm-esr102/file/tip/mailnews/extensions/smime/nsMsgComposeSecure.cpp#l616
mDataHash is nulled out on the next line. So maybe there's some re-entry of nsMsgComposeSecure::MimeFinishMultipartSigned?

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: Bug 1806247 - Fix Crash in [@ nsMsgComposeSecure::MimeFinishMultipartSigned]. r=kaie

Comment 4

[Kai Engert [:KaiE:]]

The bad state could occur if the init code fails, but the parent code continues to use the object with the failed state.

Wayne, do all crashes show a "crash address" of zero, like in the one crash you linked above?
If yes, we don't have memory corruption, but rather have a failed allocation (or a double MIME part destroy call).

I agree with the patch. I agree we should abort the operation if we run into this inconsistency.

If anyone had time available to investigate further, we could check if we ever run through MimeFinishMultipartSigned twice for a single object. And in addition, we could add a state boolean, that remembers whether we had a failed init (any of the init functions), and immediately return from the "update" and "finish" functions if the "failed init" flag is set.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e45e4b7e299d
Fix Crash in [@ nsMsgComposeSecure::MimeFinishMultipartSigned]. r=kaie

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to Kai Engert (:KaiE:) from comment #4)

The bad state could occur if the init code fails, but the parent code continues to use the object with the failed state.

Wayne, do all crashes show a "crash address" of zero, like in the one crash you linked above?
If yes, we don't have memory corruption, but rather have a failed allocation (or a double MIME part destroy call).

Yes, all zero.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9310607 [details]
Bug 1806247 - Fix Crash in [@ nsMsgComposeSecure::MimeFinishMultipartSigned]. r=kaie

[Approval Request Comment]
Safe crash fix

Comment 8

[Wayne Mery (:wsmwk)]

Comment on attachment 9310607 [details]
Bug 1806247 - Fix Crash in [@ nsMsgComposeSecure::MimeFinishMultipartSigned]. r=kaie

[Triage Comment]
Approved for beta

Comment 9

[Daniel Darnell [:dandarnell]]

Thunderbird 109.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/b4191d614857

Comment 10

[Wayne Mery (:wsmwk)]

crash rate is too low to verify effectiveness of patch in beta or nightly builds.

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9310607 [details]
Bug 1806247 - Fix Crash in [@ nsMsgComposeSecure::MimeFinishMultipartSigned]. r=kaie

[Triage Comment]
Approved for esr102

Comment 12

[Rob Lemley [:rjl]]

Thunderbird 102.7.2:
https://hg.mozilla.org/releases/comm-esr102/rev/f4a01622fb9b

Comment 13

[Wayne Mery (:wsmwk)]

v.fixed gone from 102.7.2