[tracing] Sanitize process memory dumps for background mode

base/trace_event/process_memory_dump.cc

 // Copyright 2015 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 "base/trace_event/process_memory_dump.h"
 
 #include <errno.h>
 
 #include <vector>
 
 #include "base/memory/ptr_util.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/heap_profiler_heap_dump_writer.h"
+#include "base/trace_event/memory_dump_request_args.h"
+#include "base/trace_event/memory_infra_background_whitelist.h"
 #include "base/trace_event/process_memory_totals.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "build/build_config.h"
 
 #if defined(OS_IOS)
 #include <sys/sysctl.h>
 #endif
 
 #if defined(OS_POSIX)
 #include <sys/mman.h>
@@ skipping 116 matching lines @@
   DCHECK(!failure);
   if (failure) {
     total_resident_size = 0;
     LOG(ERROR) << "CountResidentBytes failed. The resident size is invalid";
   }
   return total_resident_size;
 }
 #endif  // defined(COUNT_RESIDENT_BYTES_SUPPORTED)
 
 ProcessMemoryDump::ProcessMemoryDump(
-    scoped_refptr<MemoryDumpSessionState> session_state)
+    scoped_refptr<MemoryDumpSessionState> session_state,
+    const MemoryDumpArgs& dump_args)
     : has_process_totals_(false),
       has_process_mmaps_(false),
-      session_state_(std::move(session_state)) {}
+      session_state_(std::move(session_state)),
+      dump_args_(dump_args) {
+  if (dump_args_.level_of_detail == MemoryDumpLevelOfDetail::BACKGROUND) {
+    black_hole_mad_.reset(new MemoryAllocatorDump("dummy", this));

[Primiano Tucci (use gerrit), 2016/06/02 20:24:04]

I'd create this only if we need it. If we do everything right there will be no
need of the black hole, so you can avoid this malloc.
Maybe create an helper GetBlackHoleMad(). Also if you have that you can remove
all these DCHECK_NE and add a NOTREACHED in that.
Also I'd s/"dummy"/"discarded"/ just to give more hints about what this is
doing.

[ssid, 2016/06/03 01:59:46]

Added a get method. dcheck_ne was added because of tests, explained below.
Removed anyways.

+  }
+}
 
 ProcessMemoryDump::~ProcessMemoryDump() {}
 
 MemoryAllocatorDump* ProcessMemoryDump::CreateAllocatorDump(
     const std::string& absolute_name) {
-  return AddAllocatorDumpInternal(
+  auto mad = AddAllocatorDumpInternal(
       WrapUnique(new MemoryAllocatorDump(absolute_name, this)));
+  DCHECK_NE(black_hole_mad_.get(), mad);
+  return mad;

[Primiano Tucci (use gerrit), 2016/06/02 20:24:04]

you can keep this as it was if you use the helper.

[ssid, 2016/06/03 01:59:46]

This is what I tried to explain in my previous comment.
I have added this dcheck here only because it makes it possible to add unittest
for the AddInternal method.
Now that there's bool i can remove this.

 }
 
 MemoryAllocatorDump* ProcessMemoryDump::CreateAllocatorDump(
     const std::string& absolute_name,
     const MemoryAllocatorDumpGuid& guid) {
-  return AddAllocatorDumpInternal(
+  auto mad = AddAllocatorDumpInternal(
       WrapUnique(new MemoryAllocatorDump(absolute_name, this, guid)));
+  DCHECK_NE(black_hole_mad_.get(), mad);
+  return mad;
 }
 
 MemoryAllocatorDump* ProcessMemoryDump::AddAllocatorDumpInternal(
     std::unique_ptr<MemoryAllocatorDump> mad) {
+  // In background mode return dummy dump if invalid dump name is given.
+  if (dump_args_.level_of_detail == MemoryDumpLevelOfDetail::BACKGROUND &&
+      !IsMemoryAllocatorDumpNameWhitelisted(mad->absolute_name())) {
+    return black_hole_mad_.get();
+  }
+
   auto insertion_result = allocator_dumps_.insert(
       std::make_pair(mad->absolute_name(), std::move(mad)));
   MemoryAllocatorDump* inserted_mad = insertion_result.first->second.get();
   DCHECK(insertion_result.second) << "Duplicate name: "
                                   << inserted_mad->absolute_name();
   return inserted_mad;
 }
 
 MemoryAllocatorDump* ProcessMemoryDump::GetAllocatorDump(
     const std::string& absolute_name) const {
   auto it = allocator_dumps_.find(absolute_name);
-  return it == allocator_dumps_.end() ? nullptr : it->second.get();
+  if (it != allocator_dumps_.end())
+    return it->second.get();
+  return dump_args_.level_of_detail == MemoryDumpLevelOfDetail::BACKGROUND

[Primiano Tucci (use gerrit), 2016/06/02 20:24:04]

nit: At this point stay consistent and keep doing:
if (... == background)
  return blackhole;
return nullptr.

[ssid, 2016/06/03 01:59:46]

Done.

+             ? black_hole_mad_.get()
+             : nullptr;
 }
 
 MemoryAllocatorDump* ProcessMemoryDump::GetOrCreateAllocatorDump(
     const std::string& absolute_name) {
   MemoryAllocatorDump* mad = GetAllocatorDump(absolute_name);
   return mad ? mad : CreateAllocatorDump(absolute_name);
 }
 
 MemoryAllocatorDump* ProcessMemoryDump::CreateSharedGlobalAllocatorDump(
     const MemoryAllocatorDumpGuid& guid) {
+  // Global dumps are disabled in background mode.
+  if (dump_args_.level_of_detail == MemoryDumpLevelOfDetail::BACKGROUND)
+    return black_hole_mad_.get();
+
   // A shared allocator dump can be shared within a process and the guid could
   // have been created already.
   MemoryAllocatorDump* mad = GetSharedGlobalAllocatorDump(guid);
   if (mad) {
     // The weak flag is cleared because this method should create a non-weak
     // dump.
     mad->clear_flags(MemoryAllocatorDump::Flags::WEAK);
     return mad;
   }
   return CreateAllocatorDump(GetSharedGlobalAllocatorDumpName(guid), guid);
 }
 
 MemoryAllocatorDump* ProcessMemoryDump::CreateWeakSharedGlobalAllocatorDump(
     const MemoryAllocatorDumpGuid& guid) {
+  // Global dumps are disabled in background mode.
+  if (dump_args_.level_of_detail == MemoryDumpLevelOfDetail::BACKGROUND)
+    return black_hole_mad_.get();
+
   MemoryAllocatorDump* mad = GetSharedGlobalAllocatorDump(guid);
   if (mad)
     return mad;
   mad = CreateAllocatorDump(GetSharedGlobalAllocatorDumpName(guid), guid);
   mad->set_flags(MemoryAllocatorDump::Flags::WEAK);
   return mad;
 }
 
 MemoryAllocatorDump* ProcessMemoryDump::GetSharedGlobalAllocatorDump(
     const MemoryAllocatorDumpGuid& guid) const {
@@ skipping 102 matching lines @@
 }
 
 void ProcessMemoryDump::AddOwnershipEdge(
     const MemoryAllocatorDumpGuid& source,
     const MemoryAllocatorDumpGuid& target) {
   AddOwnershipEdge(source, target, 0 /* importance */);
 }
 
 void ProcessMemoryDump::AddSuballocation(const MemoryAllocatorDumpGuid& source,
                                          const std::string& target_node_name) {
+  // Do not create new dumps for suballocations in background mode.
+  if (dump_args_.level_of_detail == MemoryDumpLevelOfDetail::BACKGROUND)
+    return;

[Primiano Tucci (use gerrit), 2016/06/02 20:24:04]

+ NOTREACHED

[ssid, 2016/06/03 01:59:46]

Hmm, that needs an ugly fix of checking background mode before creating
suballocation in many dump providers. If you do not like silently ignoring this
part, I will just remove this change and create sub allocations.

[Primiano Tucci (use gerrit), 2016/06/03 16:24:36]

Ahh I thought you were not expecting suballocation to be created.
It's fine leave it as it is. If you expect some dumpers to still invoke
AddSuballocation, as long as they are not too many (so we don't waste cpu time)
it's ok to keep this silent.

+
   std::string child_mad_name = target_node_name + "/__" + source.ToString();
   MemoryAllocatorDump* target_child_mad = CreateAllocatorDump(child_mad_name);
   AddOwnershipEdge(source, target_child_mad->guid());
 }
 
 }  // namespace trace_event
 }  // namespace base