Bug 1660826 - Work around apparent bug with sendmsg() in some 64-bit Android devices. r=nika
authorJed Davis <jld@mozilla.com>
Fri, 11 Sep 2020 04:31:50 +0000
changeset 548423 f39f39711089bc120deae7e05422c4bbea0bc6bf
parent 548422 5bca29ff76f124f4289c0507b020189eca63ea59
child 548424 455eadc62ca7571b7778f8c70a764fa40401911f
push id126171
push userjedavis@mozilla.com
push dateFri, 11 Sep 2020 23:01:24 +0000
treeherderautoland@f39f39711089 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1660826
milestone82.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1660826 - Work around apparent bug with sendmsg() in some 64-bit Android devices. r=nika Some Android ARM64 devices appear to have a bug where sendmsg sometimes returns 0xFFFFFFFF, which we're assuming is a -1 that was incorrectly truncated to 32-bit and then zero-extended. This patch detects that value (which should never legitimately be returned, because it's 16x the maximum message size) and replaces it with -1, with some additional assertions. The workaround is also enabled on x86_64 Android on debug builds only, so that the code has CI coverage. Differential Revision: https://phabricator.services.mozilla.com/D89845
ipc/chromium/src/chrome/common/ipc_channel_posix.cc
--- a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
+++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
@@ -103,16 +103,52 @@ bool SetCloseOnExec(int fd) {
   flags |= FD_CLOEXEC;
   if (fcntl(fd, F_SETFD, flags) == -1) return false;
 
   return true;
 }
 
 bool ErrorIsBrokenPipe(int err) { return err == EPIPE || err == ECONNRESET; }
 
+// Some Android ARM64 devices appear to have a bug where sendmsg
+// sometimes returns 0xFFFFFFFF, which we're assuming is a -1 that was
+// incorrectly truncated to 32-bit and then zero-extended.
+// See bug 1660826 for details.
+//
+// This is a workaround to detect that value and replace it with -1
+// (and check that there really was an error), because the largest
+// amount we'll ever write is Channel::kMaximumMessageSize (256MiB).
+//
+// The workaround is also enabled on x86_64 Android on debug builds,
+// although the bug isn't known to manifest there, so that there will
+// be some CI coverage of this code.
+
+static inline ssize_t corrected_sendmsg(int socket,
+                                        const struct msghdr* message,
+                                        int flags) {
+#if defined(ANDROID) && \
+    (defined(__aarch64__) || (defined(DEBUG) && defined(__x86_64__)))
+  static constexpr auto kBadValue = static_cast<ssize_t>(0xFFFFFFFF);
+  static_assert(kBadValue > 0);
+
+#  ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+  errno = 0;
+#  endif
+  ssize_t bytes_written = sendmsg(socket, message, flags);
+  if (bytes_written == kBadValue) {
+    MOZ_DIAGNOSTIC_ASSERT(errno != 0);
+    bytes_written = -1;
+  }
+  MOZ_DIAGNOSTIC_ASSERT(bytes_written < kBadValue);
+  return bytes_written;
+#else
+  return sendmsg(socket, message, flags);
+#endif
+}
+
 }  // namespace
 //------------------------------------------------------------------------------
 
 #if defined(MOZ_WIDGET_ANDROID)
 void Channel::SetClientChannelFd(int fd) { gClientChannelFd = fd; }
 #endif  // defined(MOZ_WIDGET_ANDROID)
 
 Channel::ChannelImpl::ChannelImpl(const ChannelId& channel_id, Mode mode,
@@ -611,17 +647,18 @@ bool Channel::ChannelImpl::ProcessOutgoi
       }
       amt_to_write += size;
       iter.Advance(msg->Buffers(), size);
     }
 
     msgh.msg_iov = iov;
     msgh.msg_iovlen = iov_count;
 
-    ssize_t bytes_written = HANDLE_EINTR(sendmsg(pipe_, &msgh, MSG_DONTWAIT));
+    ssize_t bytes_written =
+        HANDLE_EINTR(corrected_sendmsg(pipe_, &msgh, MSG_DONTWAIT));
 
 #if !defined(OS_MACOSX)
     // On OSX CommitAll gets called later, once we get the
     // RECEIVED_FDS_MESSAGE_TYPE message.
     if (bytes_written > 0) msg->file_descriptor_set()->CommitAll();
 #endif
 
     if (bytes_written < 0) {