NetworkStats: fix use after free. Don't call socket_->Read if tests are

chrome/browser/net/network_stats.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/net/network_stats.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
@@ skipping 302 matching lines @@
 }
 
 int NetworkStats::ReadData() {
   if (!socket_.get())
     return 0;
 
   if (read_state_ == READ_STATE_READ_PENDING)
     return net::ERR_IO_PENDING;
 
   int rv = 0;
-  do {
+  while (true) {
     DCHECK(!read_buffer_.get());
     read_buffer_ = new net::IOBuffer(kMaxMessageSize);
 
     rv = socket_->Read(
         read_buffer_.get(),
         kMaxMessageSize,
         base::Bind(&NetworkStats::OnReadComplete, weak_factory_.GetWeakPtr()));
-  } while (rv > 0 && !ReadComplete(rv));
+    if (rv <= 0)
+      break;
+    if (ReadComplete(rv))
+      return rv;
+  };
   if (rv == net::ERR_IO_PENDING)
     read_state_ = READ_STATE_READ_PENDING;
   return rv;
 }
 
 void NetworkStats::OnReadComplete(int result) {
   DCHECK_NE(net::ERR_IO_PENDING, result);
   DCHECK_EQ(READ_STATE_READ_PENDING, read_state_);
 
   read_state_ = READ_STATE_IDLE;
@@ skipping 45 matching lines @@
     default:
       DVLOG(1) << "Received unexpected packet type: "
                << probe_packet.header().type();
   }
 
   if (!current_test_complete) {
     // All packets have not been received for the current test.
     return false;
   }
   // All packets are received for the current test.
-  // Read completes if all tests are done.
-  bool all_tests_done = current_test_index_ >= maximum_tests_ ||
-      current_test_index_ + 1 >= test_sequence_.size();
-  TestPhaseComplete(SUCCESS, net::OK);
-  return all_tests_done;
+  // Read completes if all tests are done (if TestPhaseComplete didn't start
+  // another test).
+  return TestPhaseComplete(SUCCESS, net::OK);
 }
 
 bool NetworkStats::UpdateReception(const ProbePacket& probe_packet) {
   uint32 packet_index = probe_packet.packet_index();
   if (packet_index >= packet_rtt_.size())
     return false;
   packets_received_mask_.set(packet_index);
   TestType test_type = test_sequence_[current_test_index_];
   uint32 received_packets = packets_received_mask_.count();
 
@@ skipping 111 matching lines @@
     }
     if (received_packets >= kMinimumReceivedPacketsForNATTest) {
       test_sequence_.push_back(TOKEN_REQUEST);
       test_sequence_.push_back(NAT_BIND_TEST);
       test_sequence_.push_back(TOKEN_REQUEST);
     }
   }
   TestPhaseComplete(READ_TIMED_OUT, net::ERR_FAILED);
 }
 
-void NetworkStats::TestPhaseComplete(Status status, int result) {
+bool NetworkStats::TestPhaseComplete(Status status, int result) {
   // If there is no valid token, do nothing and delete self.
   // This includes all connection error, name resolve error, etc.
   if (write_state_ == WRITE_STATE_WRITE_PENDING) {
     UMA_HISTOGRAM_BOOLEAN("NetConnectivity5.TestFailed.WritePending", true);
-  } else if (token_.timestamp_micros() != 0 &&
-             (status == SUCCESS || status == READ_TIMED_OUT)) {
+  } else if (status == SUCCESS || status == READ_TIMED_OUT) {
     TestType current_test = test_sequence_[current_test_index_];
     DCHECK_LT(current_test, TEST_TYPE_MAX);
     if (current_test != TOKEN_REQUEST) {
       RecordHistograms(current_test);
     } else if (current_test_index_ > 0) {
       if (test_sequence_[current_test_index_ - 1] == NAT_BIND_TEST) {
         // We record the NATTestReceivedHistograms after the succeeding
         // TokenRequest.
         RecordNATTestReceivedHistograms(status);
       } else if (test_sequence_[current_test_index_ - 1] == PACKET_SIZE_TEST) {
         // We record the PacketSizeTestReceivedHistograms after the succeeding
         // TokenRequest.
         RecordPacketSizeTestReceivedHistograms(status);
       }
     }
 
     // Move to the next test.
     current_test = GetNextTest();
     if (current_test_index_ <= maximum_tests_ && current_test < TEST_TYPE_MAX) {
       DVLOG(1) << "NetworkStat: Start Probe test: " << current_test;
       base::MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(&NetworkStats::StartOneTest, weak_factory_.GetWeakPtr()));
-      return;
+      return false;
     }
   }
 
   // All tests are done.
   DoFinishCallback(result);
 
   // Close the socket so that there are no more IO operations.
   if (socket_.get())
     socket_->Close();
 
   DVLOG(1) << "NetworkStat: schedule delete self at test index "
            << current_test_index_;
   delete this;
+  return true;
 }
 
 NetworkStats::TestType NetworkStats::GetNextTest() {
   ++current_test_index_;
   if (current_test_index_ >= test_sequence_.size())
     return TEST_TYPE_MAX;
   return test_sequence_[current_test_index_];
 }
 
 void NetworkStats::DoFinishCallback(int result) {
@@ skipping 358 matching lines @@
   udp_stats_client->Start(host_resolver,
                           server_address,
                           histogram_port,
                           has_proxy_server,
                           kProbePacketBytes[probe_choice],
                           bytes_for_packet_size_test,
                           net::CompletionCallback());
 }
 
 }  // namespace chrome_browser_net