Bug 1363378 - Set close-on-exec in sandbox-related sockets held by parent process. r=gcp
authorJed Davis <jld@mozilla.com>
Wed, 13 Sep 2017 12:25:35 -0600
changeset 430359 b549b2ed2efcc38ec69b87ab626ca9c232738259
parent 430358 45fe1c0506456d86919732e4223c1d0b2772e7f3
child 430360 d5dc76a1482891edaced2f77d2ee86d58b55b29c
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1363378
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1363378 - Set close-on-exec in sandbox-related sockets held by parent process. r=gcp If these aren't close-on-exec, they can be inherited by the crash reporter process after the parent process has crashed and exited, causing child processes to continue running when the IPC I/O thread blocks in the file broker trying to open a GeckoChildCrash temp file. (Empirically, the main thread then blocks waiting for the I/O thread.) Operations that run on dedicated threads, like playing media, may continue even though the main and IPC threads are locked up, resulting in videos that keep playing sound even though the browser seems to no longer exist. If the broker socket is closed as expected when the parent process exits, the child will return failure from the brokered file operation and then go on to get an IPC error due to the parent process's nonexistence, and will exit as normal. This patch makes the same change to rejected syscall reporting, even though that's a one-way asynchronous message with no response to wait for, just in case something goes wrong enough to fill the entire socket buffer but not so badly broken that it would wind up in an infinite loop anyway. SOCK_CLOEXEC has been present since Linux 2.6.26, and it would be used only if seccomp-bpf is available, so it should be safe to use unconditionally. MozReview-Commit-ID: 7tDPBJILzlj
security/sandbox/linux/broker/SandboxBroker.cpp
security/sandbox/linux/reporter/SandboxReporter.cpp
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -33,17 +33,17 @@
 namespace mozilla {
 
 // This constructor signals failure by setting mFileDesc and aClientFd to -1.
 SandboxBroker::SandboxBroker(UniquePtr<const Policy> aPolicy, int aChildPid,
                              int& aClientFd)
   : mChildPid(aChildPid), mPolicy(Move(aPolicy))
 {
   int fds[2];
-  if (0 != socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds)) {
+  if (0 != socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, fds)) {
     SANDBOX_LOG_ERROR("SandboxBroker: socketpair failed: %s", strerror(errno));
     mFileDesc = -1;
     aClientFd = -1;
     return;
   }
   mFileDesc = fds[0];
   aClientFd = fds[1];
 
--- a/security/sandbox/linux/reporter/SandboxReporter.cpp
+++ b/security/sandbox/linux/reporter/SandboxReporter.cpp
@@ -43,17 +43,17 @@ SandboxReporter::SandboxReporter()
 {
 }
 
 bool
 SandboxReporter::Init()
 {
   int fds[2];
 
-  if (0 != socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds)) {
+  if (0 != socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, fds)) {
     SANDBOX_LOG_ERROR("SandboxReporter: socketpair failed: %s",
 		      strerror(errno));
     return false;
   }
   mClientFd = fds[0];
   mServerFd = fds[1];
 
   if (!PlatformThread::Create(0, this, &mThread)) {