macOS V2 Sandbox: Send serialized seatbelt profile in a loop.

In some cases, and on user's machines per bug reports, writev() performs
a short write. The child process receives an invalid profile and tabs
fail to launch. This switches to using a sequence of write() calls to
send the buffer length, and then send the buffer in a loop.

Bug: 801889
Change-Id: Ib1f4c57c61d3ce68b875b347de480f514100d3c2
Reviewed-on: https://chromium-review.googlesource.com/920765Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Greg Kerr <kerrnel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537360}

sandbox/mac/seatbelt_exec.cc

@@ -65,7 +65,7 @@ int SeatbeltExecClient::SendProfileAndGetFD() {
     return -1;
   }
 
-  if (!WriteString(&serialized_protobuf)) {
+  if (!WriteString(serialized_protobuf)) {
     logging::Error(
         "SeatbeltExecClient: Writing the serialized profile failed.");
     return -1;
@@ -80,25 +80,28 @@ int SeatbeltExecClient::SendProfileAndGetFD() {
   return pipe_[0];
 }
 
-bool SeatbeltExecClient::WriteString(std::string* str) {
-  struct iovec iov[1];
-  iov[0].iov_base = &(*str)[0];
-  iov[0].iov_len = str->size();
+bool SeatbeltExecClient::WriteString(const std::string& str) {
+  uint64_t str_len = static_cast<uint64_t>(str.size());
 
-  ssize_t written = HANDLE_EINTR(writev(pipe_[1], iov, arraysize(iov)));
-  if (written < 0) {
-    logging::PError("SeatbeltExecClient: writev failed");
+  if (HANDLE_EINTR(write(pipe_[1], &str_len, sizeof(str_len))) !=
+      sizeof(str_len)) {
+    logging::PError("SeatbeltExecClient: write size of buffer failed");
     return false;
   }
 
-  bool write_complete = static_cast<uint64_t>(written) == str->size();
-  if (!write_complete) {
-    logging::Error("SeatbeltExecClient: short writev(). written: %" PRIu64
-                   ", str->size(): %" PRIu64 "",
-                   static_cast<uint64_t>(written), str->size());
+  uint64_t bytes_written = 0;
+
+  while (bytes_written < str_len) {
+    ssize_t wrote_this_pass = HANDLE_EINTR(
+        write(pipe_[1], &str[bytes_written], str_len - bytes_written));
+    if (wrote_this_pass < 0) {
+      logging::PError("SeatbeltExecClient: write failed");
+      return false;
+    }
+    bytes_written += wrote_this_pass;
   }
 
-  return write_complete;
+  return true;
 }
 
 SeatbeltExecServer::SeatbeltExecServer(int fd) : fd_(fd), extra_params_() {}
@@ -147,19 +150,27 @@ bool SeatbeltExecServer::ApplySandboxProfile(const mac::SandboxPolicy& policy) {
 }
 
 bool SeatbeltExecServer::ReadString(std::string* str) {
-  // 4 pages of memory is enough to hold the sandbox profiles.
-  std::vector<char> buffer(4096 * 4, '\0');
+  uint64_t buf_len = 0;
+  if (HANDLE_EINTR(read(fd_, &buf_len, sizeof(buf_len))) != sizeof(buf_len)) {
+    logging::PError("SeatbeltExecServer: read buffer length failed");
+    return false;
+  }
 
-  struct iovec iov[1];
-  iov[0].iov_base = buffer.data();
-  iov[0].iov_len = buffer.size();
+  str->clear();
+  str->resize(buf_len);
 
-  ssize_t read_length = HANDLE_EINTR(readv(fd_, iov, arraysize(iov)));
-  if (read_length < 0) {
-    logging::PError("SeatbeltExecServer: readv failed");
-    return false;
+  uint64_t bytes_read = 0;
+
+  while (bytes_read < buf_len) {
+    ssize_t read_this_pass =
+        HANDLE_EINTR(read(fd_, &(*str)[bytes_read], buf_len - bytes_read));
+    if (read_this_pass < 0) {
+      logging::PError("SeatbeltExecServer: read failed");
+      return false;
+    }
+    bytes_read += read_this_pass;
   }
-  str->assign(buffer.data());
+
   return true;
 }
 

sandbox/mac/seatbelt_exec.h

@@ -45,7 +45,7 @@ class SEATBELT_EXPORT SeatbeltExecClient {
 
  private:
   // This writes a string (the serialized protobuf) to the |pipe_|.
-  bool WriteString(std::string* str);
+  bool WriteString(const std::string& str);
 
   // This is the protobuf which contains the sandbox profile and parameters,
   // and is serialized and sent to the other process.