1424490 - Crash in PLDHashTable::Search | mozilla::dom::ContentParent::EnsurePermissionsByKey

Status: RESOLVED WORKSFORME

(Core :: Permission Manager, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-dcb0bc9e-bb41-4eaa-bb50-0114d0171207.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll PLDHashTable::Search xpcom/ds/PLDHashTable.cpp:531
1 xul.dll mozilla::dom::ContentParent::EnsurePermissionsByKey dom/ipc/ContentParent.cpp:5173
2 xul.dll mozilla::dom::ContentParent::TransmitPermissionsForPrincipal dom/ipc/ContentParent.cpp:5154
3 xul.dll mozilla::dom::ContentParent::AboutToLoadHttpFtpWyciwygDocumentForChild dom/ipc/ContentParent.cpp:5133
4 xul.dll mozilla::net::WyciwygChannelParent::OnStartRequest netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp:329
5 xul.dll nsWyciwygChannel::NotifyListener netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp:807
6 xul.dll mozilla::detail::RunnableMethodImpl<nsIDocument* const, void  xpcom/threads/nsThreadUtils.h:1192
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1039
8 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:97
9 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:319

=============================================================

there was an attempt to fix crashes with this signature in bug 1349634 already, but they haven't gone away for good. since firefox 57 they seem to be back in greater volume as well.

https://mozilla.github.io/stab-crashes/supergraph.html?s=PLDHashTable%3A%3ASearch+%7C+mozilla%3A%3Adom%3A%3AContentParent%3A%3AEnsurePermissionsByKey

Comment 1

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

Marking security-sensitive from the crash-stats info.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Dump of one for PLDHashTable::Search | mozilla::dom::ContentParent::EnsurePermissionsByKey shows this=e5e5 starting in ContentParent::AboutToLoad(), FWIW.

[@ nsTHashtable<T>::Contains | mozilla::dom::ContentParent::EnsurePermissionsByKey ] are all EXEC failures... :-(  
They're called from the ::Search above, and mOps is part of 'this', which isn't e5 in this case, but garbage (0x9595966a) or thereabouts, depending on the offset of hashKey()

Together, these say to me "something on another thread freed these (or less likely, horribly scribbled over freed memory" - but e5's aren't likely for scribbling using a UAF ptr; those get written on free itself).

Who should be looking at this?

Comment 3

[Andrew Overholt [:overholt]]

Nika loves it when I ask her about the permission manager.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

The problem here doesn't look like it's actually caused by the permission manager. In the WyciwygChannelParent code we check mIPCClosed and then fetch Manager() only if that variable is not set

https://searchfox.org/mozilla-central/rev/c56f656febb1aa58c72b218699e24a76425bd284/netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp#329-334

From :jesup's comment it sounds like Manager()->Manager() is 0xe5e5 meaning that our Manager(), PNeckoParent, has been freed, however our mIPCClosed has not been set.

I don't know of any way for our manager to be freed without ActorDestroy() being called on WyciwygChannelParent, so I'm stumped. mccr8, you know more about IPC stuff than me, have any ideas?

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Note this stack was rather different than the comment 0 one...  https://crash-stats.mozilla.com/report/index/e9d3cc1f-80e9-466d-853a-e29380180131 and https://crash-stats.mozilla.com/report/index/f291ff3a-2752-4262-bf72-601090180128

Comment 6

[Andrew McCreight [:mccr8]]

I'm not really sure. Aren't some of these actors refcounted? Maybe something went wrong with that...

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Exec wildptr crash in 58, though not in the has search: https://crash-stats.mozilla.com/report/index/27dead96-5b47-4de6-8be2-890140180203
Low volume now -- virtually all crashes stopped after 57 (searching for 58/59/60 including betas finds 3 in total, and 3 on the permManager->GetPermissionsWithKey(aKey, perms); right after the hash access.  That signature also existed in 57 and before, again at very low frequency, so I think there were two bugs here - one is now fixed.  The other does not have the hash signature; I'll clone this for that bug.