MachMessageServer: handle allocations more reasonably

util/mach/mach_message_server.cc

 // Copyright 2014 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ skipping 12 matching lines @@
 namespace crashpad {
 
 namespace {
 
 const int kNanosecondsPerMillisecond = 1E6;
 
 // TimerRunning determines whether |deadline| has passed. If |deadline| is in
 // the future, |*remaining_ms| is set to the number of milliseconds remaining,
 // which will always be a positive value, and this function returns true. If
 // |deadline| is zero (indicating that no timer is in effect), |*remaining_ms|
-// is set to zero and this function returns true. Otherwise, this function
-// returns false. |deadline| is specified on the same time base as is returned
-// by ClockMonotonicNanoseconds().
+// is set to zero and this function returns true. Otherwise, this function sets
+// |*remaining_ms| to zero and returns false. |deadline| is specified on the
+// same time base as is returned by ClockMonotonicNanoseconds().
 bool TimerRunning(uint64_t deadline, mach_msg_timeout_t* remaining_ms) {
   if (!deadline) {
     *remaining_ms = MACH_MSG_TIMEOUT_NONE;
     return true;
   }
 
   uint64_t now = ClockMonotonicNanoseconds();
 
   if (now >= deadline) {
+    *remaining_ms = 0;
     return false;
   }
 
   uint64_t remaining = deadline - now;
 
   // Round to the nearest millisecond, taking care not to overflow.
   const int kHalfMillisecondInNanoseconds = kNanosecondsPerMillisecond / 2;
   mach_msg_timeout_t remaining_mach;
   if (remaining <=
       std::numeric_limits<uint64_t>::max() - kHalfMillisecondInNanoseconds) {
     remaining_mach = (remaining + kHalfMillisecondInNanoseconds) /
                      kNanosecondsPerMillisecond;
   } else {
     remaining_mach = remaining / kNanosecondsPerMillisecond;
   }
 
   if (remaining_mach == MACH_MSG_TIMEOUT_NONE) {
+    *remaining_ms = 0;
     return false;
   }
 
   *remaining_ms = remaining_mach;
   return true;
 }
 
 }  // namespace
 
 // This implementation is based on 10.9.4
@@ skipping 52 matching lines @@
                                      ? max_request_size + trailer_alloc
                                      : request_alloc;
 
   mach_msg_size_t max_reply_size = interface->MachMessageServerReplySize();
 
   // mach_msg_server() and mach_msg_server_once() would consider whether
   // |options| contains MACH_SEND_TRAILER and include MAX_TRAILER_SIZE in this
   // computation if it does, but that option is ineffective on OS X.
   mach_msg_size_t reply_alloc = round_page(max_reply_size);
 
+  base::mac::ScopedMachVM request_scoper;
+  base::mac::ScopedMachVM reply_scoper;
+  bool received_any_request = false;
+
   kern_return_t kr;
 
   do {
     mach_msg_size_t this_request_alloc = request_alloc;
     mach_msg_size_t this_request_size = request_size;
 
-    base::mac::ScopedMachVM request_scoper;
     mach_msg_header_t* request_header = nullptr;
 
-    while (!request_scoper.address()) {
-      vm_address_t request_addr;
-      kr = vm_allocate(mach_task_self(),
-                       &request_addr,
-                       this_request_alloc,
-                       VM_FLAGS_ANYWHERE | VM_MAKE_TAG(VM_MEMORY_MACH_MSG));
-      if (kr != KERN_SUCCESS) {
-        return kr;
+    do {
+      if (request_scoper.size() != this_request_alloc) {

[Robert Sesek, 2014/12/01 20:59:17]

Rather than equality, why not just if this_request_alloc > request_scoper.size()
?

[Mark Mentovai, 2014/12/01 21:04:09]

Robert Sesek wrote:
I added a comment explaining why.

+        // reset() first, so that two allocations don’t exist simultaneously.
+        request_scoper.reset();
+
+        vm_address_t request_addr;
+        kr = vm_allocate(mach_task_self(),
+                         &request_addr,
+                         this_request_alloc,
+                         VM_FLAGS_ANYWHERE | VM_MAKE_TAG(VM_MEMORY_MACH_MSG));
+        if (kr != KERN_SUCCESS) {
+          return kr;
+        }
+
+        request_scoper.reset(request_addr, this_request_alloc);
       }
-      base::mac::ScopedMachVM trial_request_scoper(request_addr,
-                                                   this_request_alloc);
-      request_header = reinterpret_cast<mach_msg_header_t*>(request_addr);
 
-      bool run_mach_msg_receive = false;
+      request_header =
+          reinterpret_cast<mach_msg_header_t*>(request_scoper.address());
+
       do {
         // If |options| contains MACH_RCV_INTERRUPT, retry mach_msg() in a loop
         // when it returns MACH_RCV_INTERRUPTED to recompute |remaining_ms|
         // rather than allowing mach_msg() to retry using the original timeout
         // value. See 10.9.4 xnu-2422.110.17/libsyscall/mach/mach_msg.c
-        // mach_msg().
+        // mach_msg(). Don’t return early here if nothing has ever been
+        // received: this method should always attempt to dequeue at least one
+        // message.
         mach_msg_timeout_t remaining_ms;
-        if (!TimerRunning(deadline, &remaining_ms)) {
+        if (!TimerRunning(deadline, &remaining_ms) && received_any_request) {
           return MACH_RCV_TIMED_OUT;
         }
 
         kr = mach_msg(request_header,
                       options | MACH_RCV_MSG,
                       0,
                       this_request_size,
                       receive_port,
                       remaining_ms,
                       MACH_PORT_NULL);
 
         if (kr == MACH_RCV_TOO_LARGE && receive_large == kReceiveLargeIgnore) {
           MACH_LOG(WARNING, kr) << "mach_msg: ignoring large message";
-          run_mach_msg_receive = true;
-        } else if (kr == MACH_RCV_INTERRUPTED) {
-          run_mach_msg_receive = true;
         }
-      } while (run_mach_msg_receive);
+      } while (
+          (kr == MACH_RCV_TOO_LARGE && receive_large == kReceiveLargeIgnore) ||
+          kr == MACH_RCV_INTERRUPTED);
 
-      if (kr == MACH_MSG_SUCCESS) {
-        request_scoper.swap(trial_request_scoper);
-      } else if (kr == MACH_RCV_TOO_LARGE &&
-                 receive_large == kReceiveLargeResize) {
+      if (kr == MACH_RCV_TOO_LARGE && receive_large == kReceiveLargeResize) {
         this_request_size =
             round_page(request_header->msgh_size + trailer_alloc);
         this_request_alloc = this_request_size;
-      } else {
+      } else if (kr != MACH_MSG_SUCCESS) {
         return kr;
       }
+    } while (kr != MACH_MSG_SUCCESS);
+
+    received_any_request = true;
+
+    if (reply_scoper.size() == 0) {
+      vm_address_t reply_addr;
+      kr = vm_allocate(mach_task_self(),
+                       &reply_addr,
+                       reply_alloc,
+                       VM_FLAGS_ANYWHERE | VM_MAKE_TAG(VM_MEMORY_MACH_MSG));
+      if (kr != KERN_SUCCESS) {
+        return kr;
+      }
+
+      reply_scoper.reset(reply_addr, reply_alloc);
     }
 
-    vm_address_t reply_addr;
-    kr = vm_allocate(mach_task_self(),
-                     &reply_addr,
-                     reply_alloc,
-                     VM_FLAGS_ANYWHERE | VM_MAKE_TAG(VM_MEMORY_MACH_MSG));
-    if (kr != KERN_SUCCESS) {
-      return kr;
-    }
-
-    base::mac::ScopedMachVM reply_scoper(reply_addr, reply_alloc);
-
     mach_msg_header_t* reply_header =
-        reinterpret_cast<mach_msg_header_t*>(reply_addr);
+        reinterpret_cast<mach_msg_header_t*>(reply_scoper.address());
     bool destroy_complex_request = false;
     interface->MachMessageServerFunction(
         request_header, reply_header, &destroy_complex_request);
 
     if (!(reply_header->msgh_bits & MACH_MSGH_BITS_COMPLEX)) {
       // This only works if the reply message is not complex, because otherwise,
       // the location of the RetCode field is not known. It should be possible
       // to locate the RetCode field by looking beyond the descriptors in a
       // complex reply message, but this is not currently done. This behavior
       // has not proven itself necessary in practice, and it’s not done by
@@ skipping 30 matching lines @@
 
       bool running;
       do {
         // If |options| contains MACH_SEND_INTERRUPT, retry mach_msg() in a loop
         // when it returns MACH_SEND_INTERRUPTED to recompute |remaining_ms|
         // rather than allowing mach_msg() to retry using the original timeout
         // value. See 10.9.4 xnu-2422.110.17/libsyscall/mach/mach_msg.c
         // mach_msg().
         mach_msg_timeout_t remaining_ms;
         running = TimerRunning(deadline, &remaining_ms);
-        if (!running) {
-          // Don’t return just yet. If the timer ran out in between the time the
-          // request was received and now, at least try to send the response.
-          remaining_ms = 0;
-        }
+
+        // Don’t return just yet even if |running| is false. If the timer ran
+        // out in between the time the request was received and now, at least
+        // try to send the response.
 
         kr = mach_msg(reply_header,
                       send_options,
                       reply_header->msgh_size,
                       0,
                       MACH_PORT_NULL,
                       remaining_ms,
                       MACH_PORT_NULL);
       } while (kr == MACH_SEND_INTERRUPTED);
 
@@ skipping 10 matching lines @@
         // persistent mode, and just return success when not in persistent mode.
         return (persistent == kPersistent) ? MACH_RCV_TIMED_OUT : kr;
       }
     }
   } while (persistent == kPersistent);
 
   return kr;
 }
 
 }  // namespace crashpad