Closed Bug 1356516 Opened 2 years ago Closed 2 years ago

Crash in mozilla::ipc::MessageChannel::Clear called from ~PGMPServiceChild()

Categories

(Core :: IPC, defect, critical)

55 Branch
Unspecified
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: calixte, Assigned: gerald)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, Whiteboard: [clouseau])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-0d7b3210-09b3-438c-91ba-de5a70170414.
=============================================================

There are 17 crashes on nightly 55 with buildid 20170413030227. The regression may have been introduced by patch [1] to fix bug 1349699.

[1] https://hg.mozilla.org/mozilla-central/rev?node=8123d3e11f7df7370bddd6605ba48278cc0ead6a
Flags: needinfo?(wmccloskey)
Possible regression window based on comment 0: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f40e24f40b4c4556944c762d4764eace261297f5&tochange=819a666afddc804b6099ee1b3cff3a0fdf35ec15

Every crash report had ~PGMPServiceChild() in the stack.
Summary: Crash in mozilla::ipc::MessageChannel::Clear → Crash in mozilla::ipc::MessageChannel::Clear called from ~PGMPServiceChild()
Gerald, could you find someone to look into this? It's a new assertion I added recently to ensure that IPC channels are Close()ed before they're deleted. Deleting a channel before it's closed can cause UAF bugs if any messages arrive on the channel after the memory has been freed.

All these crashes happen on shutdown and were submitted through the infobar (and thus would be invisible to release channel users). But I think it would be good to fix them nonetheless. It may be possible to trigger the same situation in a different way.
Flags: needinfo?(wmccloskey) → needinfo?(gsquelart)
Top #3 of Nightly 20170416100136 on Linux.
Crash Signature: [@ mozilla::ipc::MessageChannel::Clear] → [@ mozilla::ipc::MessageChannel::Clear] [libxul.so@0xc37b03 | mozilla::ipc::MessageChannel::~MessageChannel | mozilla::gmp::PGMPServiceChild::~PGMPServiceChild]
Crash Signature: [@ mozilla::ipc::MessageChannel::Clear] [libxul.so@0xc37b03 | mozilla::ipc::MessageChannel::~MessageChannel | mozilla::gmp::PGMPServiceChild::~PGMPServiceChild] → [@ mozilla::ipc::MessageChannel::Clear] [@ libxul.so@0xc37b03 | mozilla::ipc::MessageChannel::~MessageChannel | mozilla::gmp::PGMPServiceChild::~PGMPServiceChild]
Assignee: nobody → gsquelart
Flags: needinfo?(gsquelart)
Comment on attachment 8859441 [details]
Bug 1356516 - Close channel before destroying GMPServiceChild -

https://reviewboard.mozilla.org/r/131484/#review134192

Thanks!

::: commit-message-3f9f6:4
(Diff revision 1)
> +Bug 1356516 - Close channel before destroying GMPServiceChild - r?billm
> +
> +mServiceChild is a UniquePtr, so nulling it will destroy the GMPServiceChild,
> +which will destroy the associated message channel. So we need to destroy the

Please change "destroy" to "close".
Attachment #8859441 - Flags: review?(wmccloskey) → review+
Comment on attachment 8859441 [details]
Bug 1356516 - Close channel before destroying GMPServiceChild -

https://reviewboard.mozilla.org/r/131484/#review134192

> Please change "destroy" to "close".

Good catch! Thank you for the review.
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cc4fc5f57503
Close channel before destroying GMPServiceChild - r=billm
https://hg.mozilla.org/mozilla-central/rev/cc4fc5f57503
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1364193
Crash Signature: [@ mozilla::ipc::MessageChannel::Clear] [@ libxul.so@0xc37b03 | mozilla::ipc::MessageChannel::~MessageChannel | mozilla::gmp::PGMPServiceChild::~PGMPServiceChild] → [@ mozilla::ipc::MessageChannel::Clear] [@ libxul.so@0xc37b03 | mozilla::ipc::MessageChannel::~MessageChannel | mozilla::gmp::PGMPServiceChild::~PGMPServiceChild] [@ mozilla::ipc::MessageChannel::Clear | mozilla::ipc::MessageChannel::~MessageChannel | mo…
You need to log in before you can comment on or make changes to this bug.