Use the new ScopedMachVM class and the MACH_LOG family of logging macros

content/browser/mach_broker_mac.mm

 // Copyright (c) 2011 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 "content/browser/mach_broker_mac.h"
 
 #include <bsm/libbsm.h>
 #include <servers/bootstrap.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/mac/foundation_util.h"
+#include "base/mac/mach_logging.h"
 #include "base/mac/scoped_mach_port.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/threading/platform_thread.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/common/content_switches.h"
 
 namespace content {
 
 namespace {
 
-// Prints a string representation of a Mach error code.
-std::string MachErrorCode(kern_return_t err) {
-  return base::StringPrintf("0x%x %s", err, mach_error_string(err));
-}
-
 // Mach message structure used in the child as a sending message.
 struct MachBroker_ChildSendMsg {
   mach_msg_header_t header;
   mach_msg_body_t body;
   mach_msg_port_descriptor_t child_task_port;
 };
 
 // Complement to the ChildSendMsg, this is used in the parent for receiving
 // a message. Contains a message trailer with audit information.
 struct MachBroker_ParentRecvMsg : public MachBroker_ChildSendMsg {
@@ skipping 11 matching lines @@
   }
 
   bool Init() {
     DCHECK(server_port_ == MACH_PORT_NULL);
 
     mach_port_t port;
     kern_return_t kr = mach_port_allocate(mach_task_self(),
                                           MACH_PORT_RIGHT_RECEIVE,
                                           &port);
     if (kr != KERN_SUCCESS) {
-      LOG(ERROR) << "Failed to allocate MachBroker server port: "
-                 << MachErrorCode(kr);
+      MACH_LOG(ERROR, kr) << "mach_port_allocate";
       return false;
     }
 
     // Allocate a send right for the server port.
     kr = mach_port_insert_right(
         mach_task_self(), port, port, MACH_MSG_TYPE_MAKE_SEND);
     if (kr != KERN_SUCCESS) {
-      LOG(ERROR) << "Failed to insert send right for MachBroker server port: "
-                 << MachErrorCode(kr);
+      MACH_LOG(ERROR, kr) << "mach_port_insert_right";
       return false;
     }
 
     server_port_.reset(port);
 
     // Register the port with the bootstrap server. Because bootstrap_register
     // is deprecated, this has to be wraped in an ObjC interface.
     NSPort* ns_port = [NSMachPort portWithMachPort:port
                                            options:NSMachPortDeallocateNone];
     NSString* name = base::SysUTF8ToNSString(broker_->GetMachPortName());
     return [[NSMachBootstrapServer sharedInstance] registerPort:ns_port
                                                            name:name];
   }
 
   // Implement |PlatformThread::Delegate|.
   virtual void ThreadMain() OVERRIDE {
     MachBroker_ParentRecvMsg msg;
     bzero(&msg, sizeof(msg));
     msg.header.msgh_size = sizeof(msg);
     msg.header.msgh_local_port = server_port_.get();
 
+    const mach_msg_option_t options = MACH_RCV_MSG |
+        MACH_RCV_TRAILER_TYPE(MACH_RCV_TRAILER_AUDIT) |
+        MACH_RCV_TRAILER_ELEMENTS(MACH_RCV_TRAILER_AUDIT);
+
     kern_return_t kr;
-    do {
+    while ((kr = mach_msg(&msg.header, options, 0, sizeof(msg), server_port_,

[Mark Mentovai, 2014/05/09 19:06:20]

It occurs to me that this sucks. A compromised child can send an
intentionally-large message to this port to force this mach_msg to return
MACH_RCV_TOO_LARGE, which will terminate this loop and thread.

Do you know how the libdispatch-based Mach message receiver deals with this kind
of thing?

[Robert Sesek, 2014/05/09 20:40:01]

Switching this to libdispatch would be better. Dispatch just has an
EVFILT_MACHPORT and calls out to the event handler block. If the message failed
to be received, then returning out of the block would be fine and the server
would keep servicing its port.

I have an item on my todo list to switch this from a thread to a queue. Then it
can actually be joinable (if we want).

+                          MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL)) ==
+           KERN_SUCCESS) {

[Robert Sesek, 2014/05/09 20:40:01]

nit? should this be indented +4

[Mark Mentovai, 2014/05/09 21:09:35]

rsesek wrote:
I don’t think so, but I struggled with this too. I made it tall instead of wide,
how ’bout that?

       // Use the kernel audit information to make sure this message is from
       // a task that this process spawned. The kernel audit token contains the
       // unspoofable pid of the task that sent the message.
-      mach_msg_option_t options = MACH_RCV_MSG |
-          MACH_RCV_TRAILER_TYPE(MACH_RCV_TRAILER_AUDIT) |
-          MACH_RCV_TRAILER_ELEMENTS(MACH_RCV_TRAILER_AUDIT);
+      //
+      // TODO(rsesek): In the 10.7 SDK, there's audit_token_to_pid().
+      pid_t child_pid;
+      audit_token_to_au32(msg.trailer.msgh_audit,
+          NULL, NULL, NULL, NULL, NULL, &child_pid, NULL, NULL);
 
-      kr = mach_msg(&msg.header, options, 0, sizeof(msg), server_port_,
-          MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
-      if (kr == KERN_SUCCESS) {
-        // TODO(rsesek): In the 10.7 SDK, there's audit_token_to_pid().
-        pid_t child_pid;
-        audit_token_to_au32(msg.trailer.msgh_audit,
-            NULL, NULL, NULL, NULL, NULL, &child_pid, NULL, NULL);
+      mach_port_t child_task_port = msg.child_task_port.name;
 
-        mach_port_t child_task_port = msg.child_task_port.name;
+      // Take the lock and update the broker information.
+      base::AutoLock lock(broker_->GetLock());
+      broker_->FinalizePid(child_pid, child_task_port);
+    }
 
-        // Take the lock and update the broker information.
-        base::AutoLock lock(broker_->GetLock());
-        broker_->FinalizePid(child_pid, child_task_port);
-      }
-    } while (kr == KERN_SUCCESS);
-
-    LOG(ERROR) << "MachBroker thread exiting; mach_msg() likely failed: "
-               << MachErrorCode(kr);
+    MACH_LOG(ERROR, kr) << "mach_msg";
   }
 
  private:
   // The MachBroker to use when new child task rights are received.  Can be
   // NULL.
   MachBroker* broker_;  // weak
 
   base::mac::ScopedMachPort server_port_;
 
   DISALLOW_COPY_AND_ASSIGN(MachListenerThreadDelegate);
 };
 
 bool MachBroker::ChildSendTaskPortToParent() {
   // Look up the named MachBroker port that's been registered with the
   // bootstrap server.
   mach_port_t bootstrap_port;
   kern_return_t kr = task_get_bootstrap_port(mach_task_self(), &bootstrap_port);
   if (kr != KERN_SUCCESS) {
-    LOG(ERROR) << "Failed to look up bootstrap port: " << MachErrorCode(kr);
+    MACH_LOG(ERROR, kr) << "task_get_bootstrap_port";
     return false;
   }
 
   mach_port_t parent_port;
   kr = bootstrap_look_up(bootstrap_port,
       const_cast<char*>(GetMachPortName().c_str()), &parent_port);
   if (kr != KERN_SUCCESS) {
-    LOG(ERROR) << "Failed to look up named parent port: " << MachErrorCode(kr);
+    BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_look_up";
     return false;
   }
 
   // Create the check in message. This will copy a send right on this process'
   // (the child's) task port and send it to the parent.
   MachBroker_ChildSendMsg msg;
   bzero(&msg, sizeof(msg));
   msg.header.msgh_bits = MACH_MSGH_BITS_REMOTE(MACH_MSG_TYPE_COPY_SEND) |
                          MACH_MSGH_BITS_COMPLEX;
   msg.header.msgh_remote_port = parent_port;
   msg.header.msgh_size = sizeof(msg);
   msg.body.msgh_descriptor_count = 1;
   msg.child_task_port.name = mach_task_self();
   msg.child_task_port.disposition = MACH_MSG_TYPE_PORT_SEND;
   msg.child_task_port.type = MACH_MSG_PORT_DESCRIPTOR;
 
   kr = mach_msg(&msg.header, MACH_SEND_MSG | MACH_SEND_TIMEOUT, sizeof(msg),
       0, MACH_PORT_NULL, 100 /*milliseconds*/, MACH_PORT_NULL);
   if (kr != KERN_SUCCESS) {
-    LOG(ERROR) << "Failed to send task port to parent: " << MachErrorCode(kr);
+    MACH_LOG(ERROR, kr) << "mach_msg";
     return false;
   }
 
   return true;
 }
 
 MachBroker* MachBroker::GetInstance() {
   return Singleton<MachBroker, LeakySingletonTraits<MachBroker> >::get();
 }
 
@@ skipping 89 matching lines @@
 }
 
 void MachBroker::InvalidatePid(base::ProcessHandle pid) {
   base::AutoLock lock(lock_);
   MachBroker::MachMap::iterator it = mach_map_.find(pid);
   if (it == mach_map_.end())
     return;
 
   kern_return_t kr = mach_port_deallocate(mach_task_self(),
                                           it->second);
-  LOG_IF(WARNING, kr != KERN_SUCCESS)
-     << "Failed to mach_port_deallocate mach task " << it->second
-     << ", error " << MachErrorCode(kr);
+  MACH_LOG_IF(WARNING, kr != KERN_SUCCESS, kr) << "mach_port_deallocate";
   mach_map_.erase(it);
 }
 
 // static
 std::string MachBroker::GetMachPortName() {
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
   const bool is_child = command_line->HasSwitch(switches::kProcessType);
 
   // In non-browser (child) processes, use the parent's pid.
   const pid_t pid = is_child ? getppid() : getpid();
   return base::StringPrintf("%s.rohitfork.%d", base::mac::BaseBundleID(), pid);
 }
 
 void MachBroker::RegisterNotifications() {
   registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_CLOSED,
                  NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_TERMINATED,
                  NotificationService::AllBrowserContextsAndSources());
 
   // No corresponding StopObservingBrowserChildProcesses,
   // we leak this singleton.
   BrowserChildProcessObserver::Add(this);
 }
 
 }  // namespace content