Create DispatchSourceMach to run a MACH_RECV dispatch source.

This implements RAII semantics so that when the destructor runs, the source
is cancelled and event handler blocks cannot call methods on a destructed
object.

BUG=382931
R=mark@chromium.org

Review URL: https://codereview.chromium.org/392273002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283664 0039d316-1c4b-4281-b951-d872f2087c98

sandbox/mac/BUILD.gn

@@ -6,6 +6,8 @@ component("sandbox") {
   sources = [
     "bootstrap_sandbox.cc",
     "bootstrap_sandbox.h",
+    "dispatch_source_mach.cc",
+    "dispatch_source_mach.h",
     "launchd_interception_server.cc",
     "launchd_interception_server.h",
     "mach_message_server.cc",
@@ -56,6 +58,7 @@ action("generate_stubs") {
 test("sandbox_mac_unittests") {
   sources = [
     "bootstrap_sandbox_unittest.mm",
+    "dispatch_source_mach_unittest.cc",
     "policy_unittest.cc",
   ]
 

sandbox/mac/dispatch_source_mach.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sandbox/mac/dispatch_source_mach.h"
+
+namespace sandbox {
+
+DispatchSourceMach::DispatchSourceMach(const char* name,
+                                       mach_port_t port,
+                                       void (^event_handler)())
+    // TODO(rsesek): Specify DISPATCH_QUEUE_SERIAL, in the 10.7 SDK. NULL
+    // means the same thing but is not symbolically clear.
+    : DispatchSourceMach(dispatch_queue_create(name, NULL),
+                         port,
+                         event_handler) {
+  // Since the queue was created above in the delegated constructor, and it was
+  // subsequently retained, release it here.
+  dispatch_release(queue_);
+}
+
+DispatchSourceMach::DispatchSourceMach(dispatch_queue_t queue,
+                                       mach_port_t port,
+                                       void (^event_handler)())
+    : queue_(queue),
+      source_(dispatch_source_create(DISPATCH_SOURCE_TYPE_MACH_RECV,
+          port, 0, queue_)),
+      source_canceled_(dispatch_semaphore_create(0)) {
+  dispatch_retain(queue);
+
+  dispatch_source_set_event_handler(source_, event_handler);
+  dispatch_source_set_cancel_handler(source_, ^{
+      dispatch_semaphore_signal(source_canceled_);
+  });
+}
+
+DispatchSourceMach::~DispatchSourceMach() {
+  Cancel();
+}
+
+void DispatchSourceMach::Resume() {
+  dispatch_resume(source_);
+}
+
+void DispatchSourceMach::Cancel() {
+  if (source_) {
+    dispatch_source_cancel(source_);
+    dispatch_release(source_);
+    source_ = NULL;
+
+    dispatch_semaphore_wait(source_canceled_, DISPATCH_TIME_FOREVER);
+    dispatch_release(source_canceled_);
+    source_canceled_ = NULL;
+  }
+
+  if (queue_) {
+    dispatch_release(queue_);
+    queue_ = NULL;
+  }
+}
+
+}  // namespace sandbox

sandbox/mac/dispatch_source_mach.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SANDBOX_MAC_DISPATCH_SOURCE_MACH_H_
+#define SANDBOX_MAC_DISPATCH_SOURCE_MACH_H_
+
+#include <dispatch/dispatch.h>
+
+#include "base/basictypes.h"
+#include "sandbox/sandbox_export.h"
+
+namespace sandbox {
+
+// This class encapsulates a MACH_RECV dispatch source. When this object is
+// destroyed, the source will be cancelled and it will wait for the source
+// to stop executing work. The source can run on either a user-supplied queue,
+// or it can create its own for the source.
+class SANDBOX_EXPORT DispatchSourceMach {
+ public:
+  // Creates a new dispatch source for the |port| and schedules it on a new
+  // queue that will be created with |name|. When a Mach message is received,
+  // the |event_handler| will be called.
+  DispatchSourceMach(const char* name,
+                     mach_port_t port,
+                     void (^event_handler)());
+
+  // Creates a new dispatch source with the same semantics as above, but rather
+  // than creating a new queue, it schedules the source on |queue|.
+  DispatchSourceMach(dispatch_queue_t queue,
+                     mach_port_t port,
+                     void (^event_handler)());
+
+  // Cancels the source and waits for it to become fully cancelled before
+  // releasing the source.
+  ~DispatchSourceMach();
+
+  // Resumes the source. This must be called before any Mach messages will
+  // be received.
+  void Resume();
+
+ private:
+  // Cancels the source, after which this class will no longer receive Mach
+  // messages. Calling this more than once is a no-op.
+  void Cancel();
+
+  // The dispatch queue used to service the source_.
+  dispatch_queue_t queue_;
+
+  // A MACH_RECV dispatch source.
+  dispatch_source_t source_;
+
+  // Semaphore used to wait on the |source_|'s cancellation in the destructor.
+  dispatch_semaphore_t source_canceled_;
+
+  DISALLOW_COPY_AND_ASSIGN(DispatchSourceMach);
+};
+
+}  // namespace sandbox
+
+#endif  // SANDBOX_MAC_DISPATCH_SOURCE_MACH_H_

sandbox/mac/dispatch_source_mach_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sandbox/mac/dispatch_source_mach.h"
+
+#include <mach/mach.h>
+
+#include "base/logging.h"
+#include "base/mac/scoped_mach_port.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/test/test_timeouts.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace sandbox {
+
+class DispatchSourceMachTest : public testing::Test {
+ public:
+  virtual void SetUp() OVERRIDE {
+    mach_port_t port = MACH_PORT_NULL;
+    ASSERT_EQ(KERN_SUCCESS, mach_port_allocate(mach_task_self(),
+        MACH_PORT_RIGHT_RECEIVE, &port));
+    receive_right_.reset(port);
+
+    ASSERT_EQ(KERN_SUCCESS, mach_port_insert_right(mach_task_self(), port,
+        port, MACH_MSG_TYPE_MAKE_SEND));
+    send_right_.reset(port);
+  }
+
+  mach_port_t port() { return receive_right_.get(); }
+
+  void WaitForSemaphore(dispatch_semaphore_t semaphore) {
+    dispatch_semaphore_wait(semaphore, dispatch_time(
+        DISPATCH_TIME_NOW,
+        TestTimeouts::action_timeout().InSeconds() * NSEC_PER_SEC));
+  }
+
+ private:
+  base::mac::ScopedMachReceiveRight receive_right_;
+  base::mac::ScopedMachSendRight send_right_;
+};
+
+TEST_F(DispatchSourceMachTest, ReceiveAfterResume) {
+  dispatch_semaphore_t signal = dispatch_semaphore_create(0);
+
+  bool __block did_receive = false;
+  DispatchSourceMach source("org.chromium.sandbox.test.ReceiveAfterResume",
+      port(), ^{
+          mach_msg_empty_rcv_t msg = {{0}};
+          msg.header.msgh_size = sizeof(msg);
+          msg.header.msgh_local_port = port();
+          mach_msg_receive(&msg.header);
+          did_receive = true;
+
+          dispatch_semaphore_signal(signal);
+      });
+
+  mach_msg_empty_send_t msg = {{0}};
+  msg.header.msgh_size = sizeof(msg);
+  msg.header.msgh_remote_port = port();
+  msg.header.msgh_bits = MACH_MSGH_BITS_REMOTE(MACH_MSG_TYPE_COPY_SEND);
+  ASSERT_EQ(KERN_SUCCESS, mach_msg_send(&msg.header));
+
+  EXPECT_FALSE(did_receive);
+
+  source.Resume();
+
+  WaitForSemaphore(signal);
+
+  EXPECT_TRUE(did_receive);
+}
+
+TEST_F(DispatchSourceMachTest, NoMessagesAfterDestruction) {
+  scoped_ptr<int> count(new int(0));
+  int* __block count_ptr = count.get();
+
+  scoped_ptr<DispatchSourceMach> source(new DispatchSourceMach(
+      "org.chromium.sandbox.test.NoMessagesAfterDestruction",
+      port(), ^{
+          mach_msg_empty_rcv_t msg = {{0}};
+          msg.header.msgh_size = sizeof(msg);
+          msg.header.msgh_local_port = port();
+          mach_msg_receive(&msg.header);
+          LOG(INFO) << "Receieve " << *count_ptr;
+          ++(*count_ptr);
+      }));
+  source->Resume();
+
+  dispatch_queue_t queue =
+      dispatch_queue_create("org.chromium.sandbox.test.MessageSend", NULL);
+  dispatch_semaphore_t signal = dispatch_semaphore_create(0);
+  for (int i = 0; i < 30; ++i) {
+    dispatch_async(queue, ^{
+        mach_msg_empty_send_t msg = {{0}};
+        msg.header.msgh_size = sizeof(msg);
+        msg.header.msgh_remote_port = port();
+        msg.header.msgh_bits =
+            MACH_MSGH_BITS_REMOTE(MACH_MSG_TYPE_COPY_SEND);
+        mach_msg_send(&msg.header);
+    });
+
+    // After sending five messages, shut down the source and taint the
+    // pointer the handler dereferences. The test will crash if |count_ptr|
+    // is being used after "free".
+    if (i == 5) {
+      scoped_ptr<DispatchSourceMach>* source_ptr = &source;
+      dispatch_async(queue, ^{
+          source_ptr->reset();
+          count_ptr = reinterpret_cast<int*>(0xdeaddead);
+          dispatch_semaphore_signal(signal);
+      });
+    }
+  }
+
+  WaitForSemaphore(signal);
+  dispatch_release(queue);
+}
+
+}  // namespace sandbox

sandbox/mac/mach_message_server.cc

@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/mac/mach_logging.h"
 #include "base/strings/stringprintf.h"
+#include "sandbox/mac/dispatch_source_mach.h"
 
 namespace sandbox {
 
@@ -21,9 +22,6 @@ MachMessageServer::MachMessageServer(
     mach_msg_size_t buffer_size)
     : demuxer_(demuxer),
       server_port_(server_receive_right),
-      server_queue_(NULL),
-      server_source_(NULL),
-      source_canceled_(dispatch_semaphore_create(0)),
       buffer_size_(
           mach_vm_round_page(buffer_size + sizeof(mach_msg_audit_trailer_t))),
       did_forward_message_(false) {
@@ -31,15 +29,6 @@ MachMessageServer::MachMessageServer(
 }
 
 MachMessageServer::~MachMessageServer() {
-  if (server_source_) {
-    dispatch_source_cancel(server_source_);
-    dispatch_release(server_source_);
-
-    dispatch_semaphore_wait(source_canceled_, DISPATCH_TIME_FOREVER);
-    dispatch_release(source_canceled_);
-  }
-  if (server_queue_)
-    dispatch_release(server_queue_);
 }
 
 bool MachMessageServer::Initialize() {
@@ -78,18 +67,11 @@ bool MachMessageServer::Initialize() {
   reply_buffer_.reset(buffer, buffer_size_);
 
   // Set up the dispatch queue to service the bootstrap port.
-  // TODO(rsesek): Specify DISPATCH_QUEUE_SERIAL, in the 10.7 SDK. NULL means
-  // the same thing but is not symbolically clear.
   std::string label = base::StringPrintf(
       "org.chromium.sandbox.MachMessageServer.%p", demuxer_);
-  server_queue_ = dispatch_queue_create(label.c_str(), NULL);
-  server_source_ = dispatch_source_create(DISPATCH_SOURCE_TYPE_MACH_RECV,
-      server_port_.get(), 0, server_queue_);
-  dispatch_source_set_event_handler(server_source_, ^{ ReceiveMessage(); });
-  dispatch_source_set_cancel_handler(server_source_, ^{
-      dispatch_semaphore_signal(source_canceled_);
-  });
-  dispatch_resume(server_source_);
+  dispatch_source_.reset(new DispatchSourceMach(
+      label.c_str(), server_port_.get(), ^{ ReceiveMessage(); }));
+  dispatch_source_->Resume();
 
   return true;
 }

sandbox/mac/mach_message_server.h

@@ -5,15 +5,17 @@
 #ifndef SANDBOX_MAC_MACH_MESSAGE_SERVER_H_
 #define SANDBOX_MAC_MACH_MESSAGE_SERVER_H_
 
-#include <dispatch/dispatch.h>
 #include <mach/mach.h>
 
 #include "base/mac/scoped_mach_port.h"
 #include "base/mac/scoped_mach_vm.h"
+#include "base/memory/scoped_ptr.h"
 #include "sandbox/mac/message_server.h"
 
 namespace sandbox {
 
+class DispatchSourceMach;
+
 // A Mach message server that operates a receive port. Messages are received
 // and then passed to the MessageDemuxer for handling. The Demuxer
 // can use the server class to send a reply, forward the message to a
@@ -53,16 +55,6 @@ class MachMessageServer : public MessageServer {
   // The Mach port on which the server is receiving requests.
   base::mac::ScopedMachReceiveRight server_port_;
 
-  // The dispatch queue used to service the server_source_.
-  dispatch_queue_t server_queue_;
-
-  // A MACH_RECV dispatch source for the server_port_.
-  dispatch_source_t server_source_;
-
-  // Semaphore used to wait on the |server_source_|'s cancellation in the
-  // destructor.
-  dispatch_semaphore_t source_canceled_;
-
   // The size of the two message buffers below.
   const mach_msg_size_t buffer_size_;
 
@@ -70,6 +62,9 @@ class MachMessageServer : public MessageServer {
   base::mac::ScopedMachVM request_buffer_;
   base::mac::ScopedMachVM reply_buffer_;
 
+  // MACH_RECV dispatch source that handles the |server_port_|.
+  scoped_ptr<DispatchSourceMach> dispatch_source_;
+
   // Whether or not ForwardMessage() was called during ReceiveMessage().
   bool did_forward_message_;
 };

sandbox/mac/sandbox_mac.gypi

@@ -10,6 +10,8 @@
       'sources': [
         'bootstrap_sandbox.cc',
         'bootstrap_sandbox.h',
+        'dispatch_source_mach.cc',
+        'dispatch_source_mach.h',
         'launchd_interception_server.cc',
         'launchd_interception_server.h',
         'mach_message_server.cc',
@@ -79,6 +81,7 @@
       'type': 'executable',
       'sources': [
         'bootstrap_sandbox_unittest.mm',
+        'dispatch_source_mach_unittest.cc',
         'policy_unittest.cc',
       ],
       'dependencies': [