574961 - crash [@ MimeHeaders_get(MimeHeaders*, char const*, int, int)] [@ MimeHeaders_get_name] [@ @0x0 | MimeHeaders_get_name(MimeHeaders*, MimeDisplayOptions*)]

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Wayne Mery (:wsmwk)]

crash [@ MimeHeaders_get(MimeHeaders*, char const*, int, int)]

~#10 crash for 3.1, but it's still early to characterize as a long term topcrash - not a top 300 for 3.0.4

no crashes on trunk builds. 
stacks vary slightly, second frames being MimeInlineTextHTML_parse_begin, MimeHeaders_get_name, MimeObjectChildIsMessageBody, mime_create,   
but top frame for 3.1 the top frame is all mailnews/mime/src/mimehdrs.cpp:413


earliest crash is bp-175c0837-ea4d-4487-8f7b-8608f2100111 v3.0
MimeHeaders_get	 mailnews/mime/src/mimehdrs.cpp:368
MimeHeaders_get_name	mailnews/mime/src/mimehdrs.cpp:706 


bp-c613741c-21b0-420e-88e9-5f3cf2100626 (rg) 3.1 
MimeHeaders_get	 mailnews/mime/src/mimehdrs.cpp:413
MimeInlineTextHTML_parse_begin	mailnews/mime/src/mimethtm.cpp:114 
...
nsStreamConverter::OnDataAvailable	 mailnews/mime/src/nsStreamConverter.cpp:979
nsMailboxProtocol::ReadMessageResponse	mailnews/local/src/nsMailboxProtocol.cpp:586 


bp-51ba485c-0201-4e73-a1a1-44c062100625 (ab) 3.1
MimeHeaders_get	 mailnews/mime/src/mimehdrs.cpp:413
MimeObjectChildIsMessageBody	mailnews/mime/src/mimemoz2.cpp:1334
MimeMultipart_parse_line	mailnews/mime/src/mimemult.cpp:335 
...
nsStreamConverter::OnDataAvailable	 mailnews/mime/src/nsStreamConverter.cpp:979
nsImapCacheStreamListener::OnDataAvailable	mailnews/imap/src/nsImapProtocol.cpp:8555 


bp-d9ce78c3-8f30-4bd0-bff6-bd50d2100626 (gerd) 3.1 
MimeHeaders_get	 mailnews/mime/src/mimehdrs.cpp:413
mime_create	mailnews/mime/src/mimei.cpp:909 
...
nsStreamConverter::OnDataAvailable	 mailnews/mime/src/nsStreamConverter.cpp:979
nsMailboxProtocol::ReadMessageResponse	mailnews/local/src/nsMailboxProtocol.cpp:586 
(another gerd crash is not the same stack bp-1b13e3a4-5604-4565-ae72-1b1f12100626 )


bp-28d08b09-88c2-4892-9a3f-95ef92100626 (joseurcola)
MimeHeaders_get	 mailnews/mime/src/mimehdrs.cpp:413
MimeMultipart_parse_line	mailnews/mime/src/mimemult.cpp:349
convert_and_send_buffer	mailnews/mime/src/mimebuf.cpp:184 
...
nsStreamConverter::OnDataAvailable	 mailnews/mime/src/nsStreamConverter.cpp:979
nsMailboxProtocol::ReadMessageResponse	mailnews/local/src/nsMailboxProtocol.cpp:586

Comment 1

[Wayne Mery (:wsmwk)]

#8 crash for v3.1.2
there is a huge uptick in crashes starting with v3.1 (prior to 3.1 there is only a couple dozen crashes per month). Whether the increase was caused by a regression in thunderbird or change in some extension is yet to be determined.
 
bp-83c3f663-78e8-4db1-8ce2-6afec2100813  (dave) an event reminder appeared, then crash
bp-a089ebbc-ff23-4f6f-aa97-ac2f42100813  (dnwidmer)
bp-79ec0b91-6f65-4db5-95c8-369a72100823  (sjl2004)
bp-89d96ef4-b3aa-4104-99a6-da5762100814  (siva)

The more common stack seems to have MimeHeaders_get_name  as in ....
bp-5bb2d1c9-7f60-4447-adfb-ddc2b2100709  (mail)
0	thunderbird.exe	MimeHeaders_get	 mailnews/mime/src/mimehdrs.cpp:413
1	thunderbird.exe	MimeHeaders_get_name	mailnews/mime/src/mimehdrs.cpp:717
2	thunderbird.exe	MimeObject_write	mailnews/mime/src/mimei.cpp:1761
3	thunderbird.exe	MimeInlineTextHTML_parse_line	mailnews/mime/src/mimethtm.cpp:208
4	thunderbird.exe	MimeInlineText_convert_and_parse_line	mailnews/mime/src/mimetext.cpp:442
5	thunderbird.exe	MimeInlineText_rotate_convert_and_parse_line	mailnews/mime/src/mimetext.cpp:570
6	thunderbird.exe	convert_and_send_buffer	mailnews/mime/src/mimebuf.cpp:184
7	thunderbird.exe	mime_LineBuffer	mailnews/mime/src/mimebuf.cpp:272
8	thunderbird.exe	MimeInlineText_parse_decoded_buffer	mailnews/mime/src/mimetext.cpp:358
9	thunderbird.exe	mime_decode_qp_buffer	mailnews/mime/src/mimeenc.cpp:199
10	thunderbird.exe	MimeDecoderWrite	mailnews/mime/src/mimeenc.cpp:840
11	thunderbird.exe	MimeLeaf_parse_buffer	mailnews/mime/src/mimeleaf.cpp:174
12	thunderbird.exe	MimeMultipart_parse_child_line	mailnews/mime/src/mimemult.cpp:740
13	thunderbird.exe	MimeMultipart_parse_line	mailnews/mime/src/mimemult.cpp:427
14	thunderbird.exe	convert_and_send_buffer	mailnews/mime/src/mimebuf.cpp:184
15	thunderbird.exe	mime_LineBuffer	mailnews/mime/src/mimebuf.cpp:272
16	thunderbird.exe	MimeObject_parse_buffer	mailnews/mime/src/mimeobj.cpp:275
17	thunderbird.exe	MimeMultipart_parse_child_line	mailnews/mime/src/mimemult.cpp:735
18	thunderbird.exe	MimeMultipart_parse_line	mailnews/mime/src/mimemult.cpp:427
19	thunderbird.exe	convert_and_send_buffer	mailnews/mime/src/mimebuf.cpp:184
20	thunderbird.exe	mime_LineBuffer	mailnews/mime/src/mimebuf.cpp:272
21	thunderbird.exe	MimeObject_parse_buffer	mailnews/mime/src/mimeobj.cpp:275
22	thunderbird.exe	MimeMessage_parse_line	mailnews/mime/src/mimemsg.cpp:232
23	thunderbird.exe	convert_and_send_buffer	mailnews/mime/src/mimebuf.cpp:184
24	thunderbird.exe	mime_LineBuffer	mailnews/mime/src/mimebuf.cpp:272
25	thunderbird.exe	MimeObject_parse_buffer	mailnews/mime/src/mimeobj.cpp:275
26	thunderbird.exe	mime_display_stream_write	mailnews/mime/src/mimemoz2.cpp:944
27	thunderbird.exe	nsStreamConverter::OnDataAvailable	mailnews/mime/src/nsStreamConverter.cpp:979
28	thunderbird.exe	nsMailboxProtocol::ReadMessageResponse	mailnews/local/src/nsMailboxProtocol.cpp:586

Comment 2

[Wayne Mery (:wsmwk)]

dave, reports "an event reminder appeared, running Windows 7 Ultimate 64.   My wife's PC has had no problems running Thunderbird on XP SP3. ... event reminders always appear in duplicate and the reminder window now appears behind other windows so you do not see it!" bp-83c3f663-78e8-4db1-8ce2-6afec2100813  

oddly, this crash sig has dropped to #98 in v3.1.3, from #18 in v3.1.2. I haven't managed to get any sample messages, and everyone who wrote to me has indicated they can't reproduce the crash. However, there are two new reporters who say they keep crashing after updating to v3.1.2 (in august). attempting to contact them.

adding @0x0 | MimeHeaders_get_name(MimeHeaders*, MimeDisplayOptions*)
bp-a933232b-4710-4007-8406-30bc32100917
0		@0x0	
1	thunderbird.exe	MimeHeaders_get_name	mailnews/mime/src/mimehdrs.cpp:717
2	thunderbird.exe	MimeObject_write	mailnews/mime/src/mimei.cpp:1761
3	thunderbird.exe	MimeInlineTextHTML_parse_line	mailnews/mime/src/mimethtm.cpp:208
4	thunderbird.exe	mailnews/mime/src/mimetext.cpp:442
5	thunderbird.exe	mailnews/mime/src/mimetext.cpp:570
6	thunderbird.exe
7	thunderbird.exe	mime_LineBuffer
8	thunderbird.exe	MimeInlineText_parse_decoded_buffer
9	thunderbird.exe	mime_decode_qp_buffer
10	thunderbird.exe	MimeDecoderWrite	mailnews/mime/src/mimeenc.cpp:840

Comment 3

[Wayne Mery (:wsmwk)]

timeless, does anything obviously pop out from the lovely mime dumps?

I was in contact with 5 the crash reporters, but most report it was a one time crash. and couldn't identify a specific message as causing the crash

Comment 4

[Makoto Kato [:m_kato]]

When last of buffer is CRLF and last address is data boundary, this may occurs.

Comment 5

[Jason Oster (:Parasyte)]

Attachment: Probable fix

This looks like an off-by-one that has always been there, my patch in Bug 538641 just makes it more obvious?

Analysis: `end' always points at the *next header*. Or if processing the last header, `end' points at the character directly following the last character in the buffer: http://mxr.mozilla.org/comm-1.9.2/source/mailnews/mime/src/mimehdrs.cpp#367

Fix: Don't read the character pointed by `end'.

Looking for a review on this from timeless. The xpc-shell tests in MIME still pass with the patch applied.

Comment 6

[Jason Oster (:Parasyte)]

(In reply to comment #5)
> This looks like an off-by-one that has always been there, my patch in Bug
> 538641 just makes it more obvious?

Should have been: Bug 543813.

Also I cannot reproduce the crash, so cannot tell if this is an actual solution.

Comment 7

[Ludovic Hirlimann [:Usul]]

Comment on attachment 498373 [details] [diff] [review]
Probable fix

I don't think Timeless is a Mailnews peer :D

Comment 8

[Makoto Kato [:m_kato]]

(In reply to comment #6)
> (In reply to comment #5)
> > This looks like an off-by-one that has always been there, my patch in Bug
> > 538641 just makes it more obvious?
> 
> Should have been: Bug 543813.
> 
> Also I cannot reproduce the crash, so cannot tell if this is an actual
> solution.

I will add test case after this bug is resolved.  Please file a bug after you fix this.

Comment 9

[Jason Oster (:Parasyte)]

Ludovic: Thanks.  :)

Makoto: It would be very helpful to have a test-case that reproduces the crash.  I can setup a Windows build environment to verify the patch fixes it.  What other bug do you want filed?

Comment 10

[David :Bienvenu]

Comment on attachment 498373 [details] [diff] [review]
Probable fix

this passes all existing xpcshell tests. I think the new test case should just be added to this bug; we shouldn't need a new bug for just the test case.

Comment 11

[Ludovic Hirlimann [:Usul]]

(In reply to comment #10)
> Comment on attachment 498373 [details] [diff] [review]
> Probable fix
> 
> this passes all existing xpcshell tests. I think the new test case should just
> be added to this bug; we shouldn't need a new bug for just the test case.

Makoto can you work on the adding the testcase ?

David should we wait for the testcase before landing this ?

Comment 12

[Makoto Kato [:m_kato]]

(In reply to comment #11)
> (In reply to comment #10)
> > Comment on attachment 498373 [details] [diff] [review] [details]
> > Probable fix
> > 
> > this passes all existing xpcshell tests. I think the new test case should just
> > be added to this bug; we shouldn't need a new bug for just the test case.
> 
> Makoto can you work on the adding the testcase ?

After I land some fixes, I will add test case at next week.  So please keep open even if fix is landed.

Comment 13

[David :Bienvenu]

I think it's fine to land now since I'd like to see some baking and I trust Makoto to add a testcase.

Comment 14

[Mark Banner (:standard8)]

Checked in to trunk: http://hg.mozilla.org/comm-central/rev/037132342f97

Over to Makoto for the test cases.

Comment 15

[Makoto Kato [:m_kato]]

Humm, since MimeHeader_get cannot use from test harness, we cannot craete 100% repro test case.

Comment 16

[Makoto Kato [:m_kato]]

Attachment: test case

This is not 100% repro case, but this test is that MimeHeader_get will access unallocation area.

Comment 17

[David :Bienvenu]

Comment on attachment 502434 [details] [diff] [review]
test case

thx for the test case...even if not 100% reliable, it should still be useful.

Comment 18

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/comm-central/rev/4bcbe3004079

Comment 19

[Mark Banner (:standard8)]

I've spoken to David and we're not going to take this on the branch at this time - we consider that it may be a bit risky especially considering the follow up. It is also low in the crash stats ranking at the moment.