Add instrumentation code to investigate crbug.com/101492 to detect if it's a possible UAF.

This change also resets socket_ to nullptr when the socket is closed.

Bug: 101492
Change-Id: Ic4942fc580815eb0c062a821ac340138ac98a24a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900569Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712928}

net/quic/quic_chromium_packet_reader.cc

@@ -5,6 +5,9 @@
 #include "net/quic/quic_chromium_packet_reader.h"
 
 #include "base/bind.h"
+#ifdef TEMP_INSTRUMENTATION_1014092
+#include "base/debug/alias.h"
+#endif
 #include "base/location.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
@@ -35,7 +38,16 @@ QuicChromiumPacketReader::QuicChromiumPacketReader(
           static_cast<size_t>(quic::kMaxIncomingPacketSize))),
       net_log_(net_log) {}
 
-QuicChromiumPacketReader::~QuicChromiumPacketReader() {}
+QuicChromiumPacketReader::~QuicChromiumPacketReader() {
+#ifdef TEMP_INSTRUMENTATION_1014092
+  liveness_ = DEAD;
+  stack_trace_ = base::debug::StackTrace();
+  // Probably not necessary, but just in case compiler tries to optimize out the
+  // writes to liveness_ and stack_trace_.
+  base::debug::Alias(&liveness_);
+  base::debug::Alias(&stack_trace_);
+#endif
+}
 
 void QuicChromiumPacketReader::StartReading() {
   CHECK(!should_stop_reading_);
@@ -47,8 +59,9 @@ void QuicChromiumPacketReader::StartReading() {
     if (num_packets_read_ == 0)
       yield_after_ = clock_->Now() + yield_after_duration_;
 
-    DCHECK(socket_);
     read_pending_ = true;
+    CrashIfInvalid();
+    CHECK(socket_);
     int rv =
         socket_->Read(read_buffer_.get(), read_buffer_->size(),
                       base::BindOnce(&QuicChromiumPacketReader::OnReadComplete,
@@ -88,11 +101,14 @@ size_t QuicChromiumPacketReader::EstimateMemoryUsage() const {
 }
 
 bool QuicChromiumPacketReader::ProcessReadResult(int result) {
+  CrashIfInvalid();
+
   read_pending_ = false;
   if (result == 0)
     result = ERR_CONNECTION_CLOSED;
 
   if (result < 0) {
+    if (socket_ != nullptr)
       visitor_->OnReadError(result, socket_);
     return false;
   }
@@ -107,6 +123,8 @@ bool QuicChromiumPacketReader::ProcessReadResult(int result) {
 }
 
 void QuicChromiumPacketReader::OnReadComplete(int result) {
+  CrashIfInvalid();
+
   if (ProcessReadResult(result)) {
     if (should_stop_reading_) {
       UMA_HISTOGRAM_BOOLEAN(
@@ -118,4 +136,22 @@ void QuicChromiumPacketReader::OnReadComplete(int result) {
   }
 }
 
+void QuicChromiumPacketReader::CrashIfInvalid() const {
+#ifdef TEMP_INSTRUMENTATION_1014092
+  Liveness liveness = liveness_;
+
+  if (liveness == ALIVE)
+    return;
+
+  // Copy relevant variables onto the stack to guarantee they will be available
+  // in minidumps, and then crash.
+  base::debug::StackTrace stack_trace = stack_trace_;
+
+  base::debug::Alias(&liveness);
+  base::debug::Alias(&stack_trace);
+
+  CHECK_EQ(ALIVE, liveness);
+#endif
+}
+
 }  // namespace net

net/quic/quic_chromium_packet_reader.h

@@ -6,6 +6,13 @@
 #ifndef NET_QUIC_QUIC_CHROMIUM_PACKET_READER_H_
 #define NET_QUIC_QUIC_CHROMIUM_PACKET_READER_H_
 
+#include "build/build_config.h"
+
+// TODO(zhongyi): Temporary while investigating https://crbug.com/1014092.
+#ifndef OS_ANDROID
+#define TEMP_INSTRUMENTATION_1014092
+#include "base/debug/stack_trace.h"
+#endif
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "net/base/io_buffer.h"
@@ -58,9 +65,20 @@ class NET_EXPORT_PRIVATE QuicChromiumPacketReader {
   void SetShouldStopReading() {
     DCHECK(!should_stop_reading_);
     should_stop_reading_ = true;
+    socket_ = nullptr;
   }
 
  private:
+#ifdef TEMP_INSTRUMENTATION_1014092
+  // TODO(zhongyi): Temporary while investigating http://crbug.com/1014092.
+  enum Liveness {
+    ALIVE = 0xCA11AB13,
+    DEAD = 0xDEADBEEF,
+  };
+#endif
+  // TODO(zhongyi): Temporary while investigating http://crbug.com/1014092.
+  void CrashIfInvalid() const;
+
   // A completion callback invoked when a read completes.
   void OnReadComplete(int result);
   // Return true if reading should continue.
@@ -80,6 +98,12 @@ class NET_EXPORT_PRIVATE QuicChromiumPacketReader {
   scoped_refptr<IOBufferWithSize> read_buffer_;
   NetLogWithSource net_log_;
 
+#ifdef TEMP_INSTRUMENTATION_1014092
+  // TODO(zhongyi): Temporary while investigating http://crbug.com/1014092.
+  Liveness liveness_ = ALIVE;
+  base::debug::StackTrace stack_trace_;
+#endif
+
   base::WeakPtrFactory<QuicChromiumPacketReader> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(QuicChromiumPacketReader);