diff options
author | rsesek@chromium.org <rsesek@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-29 19:29:31 +0000 |
---|---|---|
committer | rsesek@chromium.org <rsesek@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-29 19:29:31 +0000 |
commit | 40ec9f75f3a2b16d6ce2ef93cff67637aaa8928d (patch) | |
tree | 0ba43d2527fb509c68c64014e3dd8e9ca7c3778a /sandbox | |
parent | b33051fb01cb439c0bc5c7bf21748db9463acf1f (diff) | |
download | chromium_src-40ec9f75f3a2b16d6ce2ef93cff67637aaa8928d.zip chromium_src-40ec9f75f3a2b16d6ce2ef93cff67637aaa8928d.tar.gz chromium_src-40ec9f75f3a2b16d6ce2ef93cff67637aaa8928d.tar.bz2 |
Do not destroy Mach messages that are forwarded out of the the bootstrap sandbox.
For the process hosting the sandbox, doing so could result in over-unrefing send
rights. It's not necessary to destroy forwarded messages because any rights
copied into the process will be copied out on send, using move semantics.
BUG=367863
R=mark@chromium.org
Review URL: https://codereview.chromium.org/306543005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273545 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sandbox')
-rw-r--r-- | sandbox/mac/bootstrap_sandbox_unittest.mm | 48 | ||||
-rw-r--r-- | sandbox/mac/launchd_interception_server.cc | 18 | ||||
-rw-r--r-- | sandbox/mac/launchd_interception_server.h | 3 |
3 files changed, 65 insertions, 4 deletions
diff --git a/sandbox/mac/bootstrap_sandbox_unittest.mm b/sandbox/mac/bootstrap_sandbox_unittest.mm index 0b1e279..fb389237 100644 --- a/sandbox/mac/bootstrap_sandbox_unittest.mm +++ b/sandbox/mac/bootstrap_sandbox_unittest.mm @@ -266,4 +266,52 @@ MULTIPROCESS_TEST_MAIN(PolicySubstitutePort) { return 0; } +TEST_F(BootstrapSandboxTest, ForwardMessageInProcess) { + mach_port_t task = mach_task_self(); + + mach_port_t port; + ASSERT_EQ(KERN_SUCCESS, mach_port_allocate(task, MACH_PORT_RIGHT_RECEIVE, + &port)); + base::mac::ScopedMachReceiveRight scoped_port_recv(port); + + mach_port_urefs_t send_rights = 0; + ASSERT_EQ(KERN_SUCCESS, mach_port_get_refs(task, port, MACH_PORT_RIGHT_SEND, + &send_rights)); + EXPECT_EQ(0u, send_rights); + + ASSERT_EQ(KERN_SUCCESS, mach_port_insert_right(task, port, port, + MACH_MSG_TYPE_MAKE_SEND)); + base::mac::ScopedMachSendRight scoped_port_send(port); + + send_rights = 0; + ASSERT_EQ(KERN_SUCCESS, mach_port_get_refs(task, port, MACH_PORT_RIGHT_SEND, + &send_rights)); + EXPECT_EQ(1u, send_rights); + + mach_port_t bp; + ASSERT_EQ(KERN_SUCCESS, task_get_bootstrap_port(task, &bp)); + base::mac::ScopedMachSendRight scoped_bp(bp); + + char service_name[] = "org.chromium.sandbox.test.ForwardMessageInProcess"; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + kern_return_t kr = bootstrap_register(bp, service_name, port); +#pragma GCC diagnostic pop + EXPECT_EQ(KERN_SUCCESS, kr); + + send_rights = 0; + ASSERT_EQ(KERN_SUCCESS, mach_port_get_refs(task, port, MACH_PORT_RIGHT_SEND, + &send_rights)); + EXPECT_EQ(1u, send_rights); + + mach_port_t service_port; + EXPECT_EQ(KERN_SUCCESS, bootstrap_look_up(bp, service_name, &service_port)); + base::mac::ScopedMachSendRight scoped_service_port(service_port); + + send_rights = 0; + ASSERT_EQ(KERN_SUCCESS, mach_port_get_refs(task, port, MACH_PORT_RIGHT_SEND, + &send_rights)); + EXPECT_EQ(2u, send_rights); +} + } // namespace sandbox diff --git a/sandbox/mac/launchd_interception_server.cc b/sandbox/mac/launchd_interception_server.cc index d1495dfe..919f207 100644 --- a/sandbox/mac/launchd_interception_server.cc +++ b/sandbox/mac/launchd_interception_server.cc @@ -25,6 +25,7 @@ LaunchdInterceptionServer::LaunchdInterceptionServer( server_port_(MACH_PORT_NULL), server_queue_(NULL), server_source_(NULL), + did_forward_message_(false), sandbox_port_(MACH_PORT_NULL), compat_shim_(GetLaunchdCompatibilityShim()) { } @@ -102,6 +103,7 @@ void LaunchdInterceptionServer::ReceiveMessage() { // Zero out the buffers from handling any previous message. bzero(request, kBufferSize); bzero(reply, kBufferSize); + did_forward_message_ = false; // A Mach message server-once. The system library to run a message server // cannot be used here, because some requests are conditionally forwarded @@ -125,9 +127,15 @@ void LaunchdInterceptionServer::ReceiveMessage() { // Process the message. DemuxMessage(request, reply); - // Free any descriptors in the message body. - mach_msg_destroy(request); - mach_msg_destroy(reply); + // Free any descriptors in the message body. If the message was forwarded, + // any descriptors would have been moved out of the process on send. If the + // forwarded message was sent from the process hosting this sandbox server, + // destroying the message could also destroy rights held outside the scope of + // this message server. + if (!did_forward_message_) { + mach_msg_destroy(request); + mach_msg_destroy(reply); + } } void LaunchdInterceptionServer::DemuxMessage(mach_msg_header_t* request, @@ -254,7 +262,9 @@ void LaunchdInterceptionServer::ForwardMessage(mach_msg_header_t* request, request->msgh_bits = (request->msgh_bits & ~MACH_MSGH_BITS_PORTS_MASK) | MACH_MSGH_BITS(MACH_MSG_TYPE_COPY_SEND, MACH_MSG_TYPE_MOVE_SEND_ONCE); kern_return_t kr = mach_msg_send(request); - if (kr != KERN_SUCCESS) { + if (kr == KERN_SUCCESS) { + did_forward_message_ = true; + } else { MACH_LOG(ERROR, kr) << "Unable to forward message to the real launchd."; } } diff --git a/sandbox/mac/launchd_interception_server.h b/sandbox/mac/launchd_interception_server.h index 8125d00..6b92e9f 100644 --- a/sandbox/mac/launchd_interception_server.h +++ b/sandbox/mac/launchd_interception_server.h @@ -82,6 +82,9 @@ class LaunchdInterceptionServer { base::mac::ScopedMachVM request_buffer_; base::mac::ScopedMachVM reply_buffer_; + // Whether or not ForwardMessage() was called during ReceiveMessage(). + bool did_forward_message_; + // The Mach port handed out in reply to denied look up requests. All denied // requests share the same port, though nothing reads messages from it. base::mac::ScopedMachReceiveRight sandbox_port_; |