Allow PR_SET_PTRACER in Linux sandbox policies to avoid recursive crash

RESOLVED FIXED in Firefox 49

Status

()

Product:
Core
Component:
Security: Process Sandboxing
Type:
defect
Status:
RESOLVED FIXED
Opened:
3 years ago
Updated:
3 years ago

People

(Reporter: jld, Assigned: jld)

Tracking

Version:
36 Branch
Target:
mozilla51
Platform:
Unspecified
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed, firefox50 fixed, firefox51 fixed)

Details

Attachments

(1 attachment)

bug1290618-allow-ptracer-hg0.diff
1.71 KB, patch
tedd
: review+
ritu
: approval-mozilla-aurora+
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review 
Bug 1090598 added a call to prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY) in the crash signal handlers used when the crash reporter is disabled (e.g., desktop debug builds) so that gdb can attach even if the Yama LSM would otherwise prevent that.  Content processes allow all prctl()s for now (although we'd like to restrict that eventually), but GMP processes have a small allowed list and PR_SET_PTRACER isn't on it.

The result is that if a GMP process segfaults (or similar) on Linux with the crash reporter disabled, it will take SIGSYS in the crash handler and the SIGSYS handler will complain and immediately terminate the process, which prevents debugging it.

Adding PR_SET_PTRACER to the policy is simple (and reasonably safe — prctl affects only the calling thread, and we already allow PR_SET_DUMPABLE, which is similar, for the crash reporter).
…except that even with PR_SET_PTRACER allowed, the process gets immediately killed anyway, because the IPC channel observes an error and is destroyed, for reasons that aren't immediately clear.  But I already have a patch for this problem, so I might as well.
Assignee: nobody → jld
Comment on attachment 8776732 [details] [diff] [review]
bug1290618-allow-ptracer-hg0.diff

Review of attachment 8776732 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8776732 - Flags: review?(julian.r.hector) → review+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0143eed90884
Allow PR_SET_PTRACER in Linux sandbox policies to avoid recursive crash. r=tedd
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0143eed90884
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment on attachment 8776732 [details] [diff] [review]
bug1290618-allow-ptracer-hg0.diff

Approval Request Comment
[Feature/regressing bug #]: Widevine EME on Linux
[User impact if declined]: Without this patch, debugging the sandbox issues fixed in other uplifts I've just requested is very hard.
[Describe test coverage new/current, TreeHerder]: We have plenty of EME mochitests, which run on Linux inside the sandbox.
[Risks and why]: Low; this is tweaking how the sandbox interacts with debuggers
[String/UUID change made/needed]: None
Attachment #8776732 - Flags: approval-mozilla-beta?
Attachment #8776732 - Flags: approval-mozilla-aurora?
Comment on attachment 8776732 [details] [diff] [review]
bug1290618-allow-ptracer-hg0.diff

Widevine EME on Linux, Aurora50+
Attachment #8776732 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8776732 [details] [diff] [review]
bug1290618-allow-ptracer-hg0.diff

Uplifting Linux Widevine support to beta; this should avoid a crash.
Attachment #8776732 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.