1840013 - [asan] Intermittent SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/checkouts/gecko/comm/mailnews/base/src/nsMsgCopyService.cpp:291 in nsMsgCopyService::DoNextCopy(void)

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Treeherder Bug Filer]

Filed by: mkmelin [at] iki.fi
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=420372838&repo=comm-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/FcVub1xsRKWMHlmLaPO0jA/runs/0/artifacts/public/logs/live_backing.log

---

Comment 1

[Intermittent Failures Robot]

6 failures in 3880 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 6

Platform and build breakdown:

• windows11-64-2009-asan-qr: 5
  • opt: 5
• windows11-64-2009-shippable-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1840013&startday=2023-06-19&endday=2023-06-25&tree=all

Comment 2

[Ben Campbell]

Quick superficial analysis of this one:

I'm pretty sure that nsMsgCopyService and all the folder code should be running entirely on the main thread.
But I think DoNextCopy() is being invoked by another thread (IMAP, I'm looking at you!).

nsMsgCopyService::DoNextCopy() is called from nsMsgCopyService::NotifyCompletion(), which is itself called from two places in the IMAP folder code (see https://searchfox.org/comm-central/search?q=NotifyCompletion&path=&case=true&regexp=false ).

These are:

• nsImapMailFolder::OnStopRunningUrl()
• nsImapMailFolder::OnCopyCompleted()

I think OnStopRunningUrl() is OK.. it's used for so many other things without problem

OnCopyCompleted() is called from a bunch of different places in nsImapMailfolder. My guess is that one of those places is being called by the IMAP thread.

Quick way to test this: add a MOZ_ASSERT inside DoNextCopy() to fail if we're not on the main thread. If there is a invocation from another thread, it shouldn't be too hard to trigger the assert by just copying some messages about. Then check out the call stack and see where the call came from (somewhere in nsIImapProtocol, most likely).

Comment 3

[Ben Campbell]

OR....

Just look at the logs of the ASAN failures and notice that all the failures seem to be during a call to nsMsgLocalMailFolder::DeleteSelf(), and thus not a threading issue at all!

The stack traces all look like this:

[task 2023-09-06T23:25:56.115Z] 23:25:56    ERROR - GECKO(6904) | ==6904==ERROR: AddressSanitizer: heap-use-after-free on address 0x12316718f5cc at pc 0x7ffe2f894bd1 bp 0x00d6cc1f7fc0 sp 0x00d6cc1f8008
[task 2023-09-06T23:25:56.115Z] 23:25:56     INFO - GECKO(6904) | READ of size 1 at 0x12316718f5cc thread T0
[task 2023-09-06T23:25:56.489Z] 23:25:56     INFO - GECKO(6904) |     #0 0x7ffe2f894bd0 in nsMsgCopyService::DoNextCopy(void) /builds/worker/checkouts/gecko/comm/mailnews/base/src/nsMsgCopyService.cpp:291
[task 2023-09-06T23:25:56.505Z] 23:25:56     INFO - GECKO(6904) |     #1 0x7ffe2f897b5e in nsMsgCopyService::DoCopy /builds/worker/checkouts/gecko/comm/mailnews/base/src/nsMsgCopyService.cpp:220
[task 2023-09-06T23:25:56.505Z] 23:25:56     INFO - GECKO(6904) |     #2 0x7ffe2f897b5e in nsMsgCopyService::CopyFolder(class nsIMsgFolder *, class nsIMsgFolder *, bool, class nsIMsgCopyServiceListener *, class nsIMsgWindow *) /builds/worker/checkouts/gecko/comm/mailnews/base/src/nsMsgCopyService.cpp:497
[task 2023-09-06T23:25:56.506Z] 23:25:56     INFO - GECKO(6904) |     #3 0x7ffe2fef8cad in nsMsgLocalMailFolder::DeleteSelf(class nsIMsgWindow *) /builds/worker/checkouts/gecko/comm/mailnews/local/src/nsLocalMailFolder.cpp:650
[task 2023-09-06T23:25:56.506Z] 23:25:56     INFO - GECKO(6904) |     #4 0x7ffe33944f71 in XPTC__InvokebyIndex (Z:\task_169404084574788\build\application\thunderbird\xul.dll+0x193774f71)
...

Comment 4

[Ben Campbell]

OK, figured it out. DoNextCopy() pulls up the next queued nsCopyRequest and then calls folder->CopyFolder() to kick it off.
see https://searchfox.org/comm-central/source/mailnews/base/src/nsMsgCopyService.cpp#279
This is async - when the folder has done it's thing, it's meant to call NotifyCompletion() on the nsIMsgCopyService to let it know the copy request is done. NotifyCompletion() deletes the request and does whatever other housekeeping is required.

Immediately after, calling CopyFolder(), DoNextCopy() tries to clear the request m_isMoveOrDraftOrTemplate field, if the copy is across servers.
I'm not 100% sure why it does that... my reading is that it's something to do with what notifications nsIMsgFolderNotificationService is supposed to produce... but really it's all a bit of a confused mess.

BUT. The problem is that the localfolder implementation of CopyFolder() completes immediately, and calls NotifyCompletion() before returning, which deletes the copy request, which means that the m_isMoveOfDraftOrTemplate fiddling done in DoNextCopy() is now operating on freed memory. ugh.

Comment 5

[Ben Campbell]

Attachment: Bug 1840013 - Fix use-after-free bug in local folder move. r=#thunderbird-reviewers

Comment 6

[Intermittent Failures Robot]

23 failures in 3267 pushes (0.007 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 23

Platform and build breakdown:

• linux1804-64-asan-qr: 3
  • opt: 3
• windows11-64-2009-asan-qr: 18
  • opt: 18
• linux1804-64-tsan-qr: 2
  • opt: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1840013&startday=2023-09-04&endday=2023-09-10&tree=all

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/853f76daa15a
Fix use-after-free bug in local folder move. r=leftmostcat

Comment 8

[Intermittent Failures Robot]

5 failures in 3337 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 5

Platform and build breakdown:

• windows11-64-2009-asan-qr: 5
  • opt: 5

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1840013&startday=2023-09-11&endday=2023-09-17&tree=all

Comment 9

[Wayne Mery (:wsmwk)]

Comment on attachment 9352324 [details]
Bug 1840013 - Fix use-after-free bug in local folder move. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr115 per matrix

Comment 10

[Daniel Darnell [:dandarnell]]

Thunderbird 115.4.0:
https://hg.mozilla.org/releases/comm-esr115/rev/e7117f386471