[Backport] CVE-2022-4180: Use after free in Mojo

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4027242:
Fix a data race leading to use-after-free in mojo::ChannelMac ShutDown

Bug: 1378564
Change-Id: I67041b1e2ef08dd0ee1ccbf6d534249c539b74db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4027242
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/main@{#1071700}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/447552
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

1 files changed, 21 insertions, 10 deletions

diff --git a/chromium/mojo/core/channel_mac.cc b/chromium/mojo/core/channel_mac.cc
index d212aa8ad69..56c9d87ff38 100644
--- a/chromium/mojo/core/channel_mac.cc
+++ b/chromium/mojo/core/channel_mac.cc
@@ -25,6 +25,7 @@
 #include "base/message_loop/message_pump_for_io.h"
 #include "base/strings/stringprintf.h"
 #include "base/task/current_thread.h"
+#include "base/thread_annotations.h"
 
 extern "C" {
 kern_return_t fileport_makeport(int fd, mach_port_t*);
@@ -163,7 +164,10 @@ class ChannelMac : public Channel,
         vm_allocate(mach_task_self(), &address, size,
                     VM_MAKE_TAG(VM_MEMORY_MACH_MSG) | VM_FLAGS_ANYWHERE);
     MACH_CHECK(kr == KERN_SUCCESS, kr) << "vm_allocate";
-    send_buffer_.reset(address, size);
+    {
+      base::AutoLock lock(write_lock_);
+      send_buffer_.reset(address, size);
+    }
 
     kr = vm_allocate(mach_task_self(), &address, size,
                      VM_MAKE_TAG(VM_MEMORY_MACH_MSG) | VM_FLAGS_ANYWHERE);
@@ -203,7 +207,11 @@ class ChannelMac : public Channel,
 
     watch_controller_.StopWatchingMachPort();
 
-    send_buffer_.reset();
+    {
+      base::AutoLock lock(write_lock_);
+      send_buffer_.reset();
+      reject_writes_ = true;
+    }
     receive_buffer_.reset();
     incoming_handles_.clear();
 
@@ -310,7 +318,7 @@ class ChannelMac : public Channel,
     SendPendingMessagesLocked();
   }
 
-  void SendPendingMessagesLocked() {
+  void SendPendingMessagesLocked() EXCLUSIVE_LOCKS_REQUIRED(write_lock_) {
     // If a previous send failed due to the receiver's kernel message queue
     // being full, attempt to send that failed message first.
     if (send_buffer_contains_message_ && !reject_writes_) {
@@ -337,7 +345,8 @@ class ChannelMac : public Channel,
     }
   }
 
-  bool SendMessageLocked(MessagePtr message) {
+  bool SendMessageLocked(MessagePtr message)
+      EXCLUSIVE_LOCKS_REQUIRED(write_lock_) {
     DCHECK(!send_buffer_contains_message_);
     base::BufferIterator<char> buffer(
         reinterpret_cast<char*>(send_buffer_.address()), send_buffer_.size());
@@ -432,7 +441,8 @@ class ChannelMac : public Channel,
     return MachMessageSendLocked(header);
   }
 
-  bool MachMessageSendLocked(mach_msg_header_t* header) {
+  bool MachMessageSendLocked(mach_msg_header_t* header)
+      EXCLUSIVE_LOCKS_REQUIRED(write_lock_) {
     kern_return_t kr = mach_msg(header, MACH_SEND_MSG | MACH_SEND_TIMEOUT,
                                 header->msgh_size, 0, MACH_PORT_NULL,
                                 /*timeout=*/0, MACH_PORT_NULL);
@@ -652,7 +662,8 @@ class ChannelMac : public Channel,
   }
 
   // Marks the channel as unaccepting of new messages and shuts it down.
-  void OnWriteErrorLocked(Error error) {
+  void OnWriteErrorLocked(Error error)
+      EXCLUSIVE_LOCKS_REQUIRED(write_lock_) {
     reject_writes_ = true;
     io_task_runner_->PostTask(
         FROM_HERE, base::BindOnce(&ChannelMac::OnError, this, error));
@@ -694,17 +705,17 @@ class ChannelMac : public Channel,
   // Lock that protects the following members.
   base::Lock write_lock_;
   // Whether writes should be rejected due to an internal error.
-  bool reject_writes_ = false;
+  bool reject_writes_ GUARDED_BY(write_lock_) = false;
   // IO buffer for sending Mach messages.
-  base::mac::ScopedMachVM send_buffer_;
+  base::mac::ScopedMachVM send_buffer_ GUARDED_BY(write_lock_);
   // If a message timed out during send in MachMessageSendLocked(), this will
   // be true to indicate that |send_buffer_| contains a message that must
   // be sent. If this is true, then other calls to Write() queue messages onto
   // |pending_messages_|.
-  bool send_buffer_contains_message_ = false;
+  bool send_buffer_contains_message_ GUARDED_BY(write_lock_) = false;
   // When |handshake_done_| is false or |send_buffer_contains_message_| is true,
   // calls to Write() will enqueue messages here.
-  base::circular_deque<MessagePtr> pending_messages_;
+  base::circular_deque<MessagePtr> pending_messages_ GUARDED_BY(write_lock_);
 
   DISALLOW_COPY_AND_ASSIGN(ChannelMac);
 };