Closed Bug 1331787 Opened 3 years ago Closed 3 years ago

Make IPDL discriminated union checks fatal in release builds

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

Details

Attachments

(1 file)

Bug 1210099 made these checks happen in non-release builds, but we should just do it there, too. These conversions can run on untrusted input from a possibly-compromised process, and there is likely not much overhead. There were no regressions on the initial landing that I could see. It seems better to take a tiny hit than have to audit every bit of code that receives a union.
_abortIfFalse is also used for checks like this:
  MOZ_DIAGNOSTIC_ASSERT((container).Contains(actor), "actor not managed by this!");
...but adding a single hashtable lookup when we create a protocol seems cheap.
Comment on attachment 8827687 [details]
Bug 1331787 - Make IPDL aborts fatal in release builds.

https://reviewboard.mozilla.org/r/105314/#review106392
Attachment #8827687 - Flags: review?(wmccloskey) → review+
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/18b0624ece02
Make IPDL aborts fatal in release builds. r=billm
I think it would make sense to backport this, given that it only does anything new on Beta.
https://hg.mozilla.org/mozilla-central/rev/18b0624ece02
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8827687 [details]
Bug 1331787 - Make IPDL aborts fatal in release builds.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: This will make certain IPC crashes easier to diagnose, if they are happening. Any such crashes could be security issues.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: This code has been running in Nightly and Aurora for about a year.
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: This just makes it so that certain validity checks are run in Beta and release. We have been running these checks in Nightly and Aurora for almost a year. I'd like to get these checks on release as quickly as possible.
[String changes made/needed]: None.
Attachment #8827687 - Flags: approval-mozilla-aurora?
Comment on attachment 8827687 [details]
Bug 1331787 - Make IPDL aborts fatal in release builds.

make some ipc errors fatal on beta/release, aurora52+
Attachment #8827687 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.