[Backport] Security bug 1325096

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4024654:
Mojo: Add release-mode sequence checks

A fairly common class of bugs is for Mojo consumers to use thread-affine
objects like Remote<T> from the wrong thread. These bugs are dangerous
since they can lead to UAFs etc.

This CL enables release-mode sequence checks on InterfaceEndpointClient,
the main thread-affine state object underying Remote<T> etc.

Fixed: 1325096
Change-Id: I362b798b015c021316ddca14ea35cb7618942538
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4024654
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1074883}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/460503
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 17 insertions, 15 deletions

diff --git a/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h b/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h
index fa4b4704f88..bb270e0780c 100644
--- a/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h
+++ b/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h
@@ -73,27 +73,27 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient
   // Sets the error handler to receive notifications when an error is
   // encountered.
   void set_connection_error_handler(base::OnceClosure error_handler) {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    CHECK(sequence_checker_.CalledOnValidSequence());
     error_handler_ = std::move(error_handler);
     error_with_reason_handler_.Reset();
   }
 
   void set_connection_error_with_reason_handler(
       ConnectionErrorWithReasonCallback error_handler) {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    CHECK(sequence_checker_.CalledOnValidSequence());
     error_with_reason_handler_ = std::move(error_handler);
     error_handler_.Reset();
   }
 
   // Returns true if an error was encountered.
   bool encountered_error() const {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    CHECK(sequence_checker_.CalledOnValidSequence());
     return encountered_error_;
   }
 
   // Returns true if this endpoint has any pending callbacks.
   bool has_pending_responders() const {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    CHECK(sequence_checker_.CalledOnValidSequence());
     base::AutoLock lock(async_responders_lock_);
     return !async_responders_.empty() || !sync_responses_.empty();
   }
@@ -354,7 +354,9 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient
   base::Location next_call_location_;
 #endif
 
-  SEQUENCE_CHECKER(sequence_checker_);
+  // We use SequenceCheckerImpl directly, to assert some sequence checks even in
+  // release builds. See https://crbug.com/1325096.
+  base::SequenceCheckerImpl sequence_checker_;
 
   base::WeakPtrFactory<InterfaceEndpointClient> weak_ptr_factory_{this};
 };
diff --git a/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc b/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
index c1e7efe6954..c7aade50db3 100644
--- a/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
+++ b/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
@@ -455,7 +455,7 @@ InterfaceEndpointClient::InterfaceEndpointClient(
       ipc_hash_callback_(ipc_hash_callback),
       method_name_callback_(method_name_callback) {
   DCHECK(handle_.is_valid());
-  DETACH_FROM_SEQUENCE(sequence_checker_);
+  sequence_checker_.DetachFromSequence();
 
   // TODO(yzshen): the way to use validator (or message filter in general)
   // directly is a little awkward.
@@ -479,7 +479,7 @@ InterfaceEndpointClient::InterfaceEndpointClient(
 }
 
 InterfaceEndpointClient::~InterfaceEndpointClient() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
   if (controller_)
     handle_.group_controller()->DetachEndpointClient(handle_);
 }
@@ -498,7 +498,7 @@ scoped_refptr<ThreadSafeProxy> InterfaceEndpointClient::CreateThreadSafeProxy(
 }
 
 ScopedInterfaceEndpointHandle InterfaceEndpointClient::PassHandle() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
   DCHECK(!has_pending_responders());
 
   if (!handle_.is_valid())
@@ -520,7 +520,7 @@ void InterfaceEndpointClient::SetFilter(std::unique_ptr<MessageFilter> filter) {
 }
 
 void InterfaceEndpointClient::RaiseError() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
 
   if (!handle_.pending_association())
     handle_.group_controller()->RaiseError();
@@ -528,7 +528,7 @@ void InterfaceEndpointClient::RaiseError() {
 
 void InterfaceEndpointClient::CloseWithReason(uint32_t custom_reason,
                                               base::StringPiece description) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
 
   auto handle = PassHandle();
   handle.ResetWithReason(custom_reason, description);
@@ -564,7 +564,7 @@ bool InterfaceEndpointClient::AcceptWithResponder(
 
 bool InterfaceEndpointClient::SendMessage(Message* message,
                                           bool is_control_message) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
   DCHECK(!message->has_flag(Message::kFlagExpectsResponse));
   DCHECK(!handle_.pending_association());
 
@@ -600,7 +600,7 @@ bool InterfaceEndpointClient::SendMessageWithResponder(
     bool is_control_message,
     SyncSendMode sync_send_mode,
     std::unique_ptr<MessageReceiver> responder) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
   DCHECK(message->has_flag(Message::kFlagExpectsResponse));
   DCHECK(!handle_.pending_association());
 
@@ -681,7 +681,7 @@ bool InterfaceEndpointClient::SendMessageWithResponder(
 }
 
 bool InterfaceEndpointClient::HandleIncomingMessage(Message* message) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
 
   // Accept() may invalidate `this` and `message` so we need to copy the
   // members we need for logging in case of an error.
@@ -698,7 +698,7 @@ bool InterfaceEndpointClient::HandleIncomingMessage(Message* message) {
 
 void InterfaceEndpointClient::NotifyError(
     const absl::optional<DisconnectReason>& reason) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(sequence_checker_.CalledOnValidSequence());
 
   if (encountered_error_)
     return;
@@ -821,7 +821,7 @@ void InterfaceEndpointClient::MaybeSendNotifyIdle() {
 }
 
 void InterfaceEndpointClient::ResetFromAnotherSequenceUnsafe() {
-  DETACH_FROM_SEQUENCE(sequence_checker_);
+  sequence_checker_.DetachFromSequence();
 
   if (controller_) {
     controller_ = nullptr;