1518960 - AddressSanitizer: heap-buffer-overflow [@ mozilla::a11y::DocAccessibleChildBase::ActorDestroy] with WRITE of size 8

Status: RESOLVED DUPLICATE of bug 1512567

(Core :: Disability Access APIs, defect, P2)

Description

[Christian Holler (:decoder)]

The attached crash information was submitted via the ASan Nightly Reporter on mozilla-central-asan-nightly revision 66.0a1-20190105215256-https://hg.mozilla.org/mozilla-central/rev/c1eea02ba0c0df7868b43ab4c30e57655938969e.

For detailed crash information, see attachment.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Hi Jamie, can you help find someone to look into this?

Comment 4

[James Teh [:Jamie]]

According to the stack, this occurs when the DocAccessibleChild actor tries to clear the IPC doc from its associated DocAccessible:

55 mDoc->SetIPCDoc(nullptr);

If I understand this correctly, it would seem that mDoc is now an invalid pointer. In order for that to happen, the DocAccessible must have been destroyed before ActorDestroy was called. However, when a DocAccessible is destroyed, it calls DocAccessibleChild::Shutdown, which calls DetachDocument and thus sets mDoc to null. So, I don't understand how mDoc could be invalid!

Eitan, any ideas?

Comment 5

[Daniel Veditz [:dveditz]]

the small number of crashes in the wild seemed to start around Dec 11 with Fx65 if that helps point to anything.

Comment 6

[Eitan Isaacson [:eeejay]]

I'll look at this further, fyi I think this is a dub of bug 1512567. Which I have been struggling to reproduce. Didn't actually try to wrap my head around the stack trace.. I'll do that next.

Comment 7

[Christian Holler (:decoder)]

I just received another report from the same reporter, with identical stack, except that the report indicates a heap-use-after-free and not a heap-buffer-overflow. It is plausible that this is the same bug, attaching the stack here for investigation.

Comment 8

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Eitan, did the extra information above help out at all? Do you think this issue is actionable or should we consider it stalled?

Comment 10

[Eitan Isaacson [:eeejay]]

Thanks for the ping Liz. I got back to this. It looks like we get into a condition where we set two IPC docs for a DocAccessible. So the last one set is referenced by the doc and gets shutdown with it, and successfully detaches from the DocAccessible (which is subsequently freed). The second one is just dangling somewhere until its ActorDestroy gets called from its manager, at that point it tries to unset itself from a doc that already doesn't exist.

The condition where a doc gets set with two IPC docs is when NotificationController::WillRefresh creates one before DocAccessible::DoInitialUpdate does. The latter doesn't check to see if an ipc doc is already set.

Comment 11

[Eitan Isaacson [:eeejay]]

The patch in bug 1512567 should remedy this. Once it is confirmed we can close this as dup.

Comment 12

[Pascal Chevrel:pascalc]

I am not seeing crashes anymore, should that bug be closed as fixed?

Comment 13

[Christian Holler (:decoder)]

I have not seen this happening since 2019-02-22 anymore while it was happening once a few days before. So I assume it is plausible that bug 1512567 fixed this.