[Backport] Security bug 1519980

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5226127:
Speculatively fix race in mojo ShutDownOnIOThread

This acquires `write_lock_` before resetting handles used by WriteNoLock
(which is called under the same lock in another thread). We also set
`reject_writes_` to prevent future write attempts after shutdown. That
seems strictly more correct.

We also acquire `fds_to_close_lock_` before clearing the FDs.

I was unable to repro locally as content_browsertests just times out
in my local setup without reporting anything interesting. This seems
strictly more correct though.

Bug: 1519980
Change-Id: I96279936ca908ecb98eddd381df20d61597cba43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5226127
Auto-Submit: Peter Boström <pbos@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Ken Rockot <rockot@google.com>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1250580}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/537376
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

1 files changed, 16 insertions, 11 deletions

diff --git a/chromium/mojo/core/channel_posix.cc b/chromium/mojo/core/channel_posix.cc
index d7d9d6cfee1..e17aa8d82a9 100644
--- a/chromium/mojo/core/channel_posix.cc
+++ b/chromium/mojo/core/channel_posix.cc
@@ -242,18 +242,23 @@ class ChannelPosix : public Channel,
   void ShutDownOnIOThread() {
     base::CurrentThread::Get()->RemoveDestructionObserver(this);
 
-    read_watcher_.reset();
-    write_watcher_.reset();
-    if (leak_handle_) {
-      ignore_result(socket_.release());
-      server_.TakePlatformHandle().release();
-    } else {
-      socket_.reset();
-      ignore_result(server_.TakePlatformHandle());
+    {
+      base::AutoLock lock(write_lock_);
+      reject_writes_ = true;
+      read_watcher_.reset();
+      write_watcher_.reset();
+      if (leak_handle_) {
+        std::ignore = socket_.release();
+        server_.TakePlatformHandle().release();
+      } else {
+        socket_.reset();
+        std::ignore = server_.TakePlatformHandle();
+      }
+ #if BUILDFLAG(IS_IOS)
+      base::AutoLock fd_lock(fds_to_close_lock_);
+      fds_to_close_.clear();
+ #endif
     }
-#if defined(OS_IOS)
-    fds_to_close_.clear();
-#endif
 
     // May destroy the |this| if it was the last reference.
     self_ = nullptr;