gpu: Reduce direct use of BrowserGpuMemoryBufferManager

content/browser/gpu/browser_gpu_memory_buffer_manager.cc

 // Copyright 2014 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 "content/browser/gpu/browser_gpu_memory_buffer_manager.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/common/child_process_host_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/ipc/client/gpu_memory_buffer_impl.h"
 #include "gpu/ipc/client/gpu_memory_buffer_impl_shared_memory.h"
 #include "ui/gfx/buffer_format_util.h"
@@ skipping 39 matching lines @@
 };
 
 BrowserGpuMemoryBufferManager::BrowserGpuMemoryBufferManager(
     int gpu_client_id,
     uint64_t gpu_client_tracing_id)
     : native_configurations_(gpu::GetNativeGpuMemoryBufferConfigurations()),
       gpu_client_id_(gpu_client_id),
       gpu_client_tracing_id_(gpu_client_tracing_id) {
   DCHECK(!g_gpu_memory_buffer_manager);
   g_gpu_memory_buffer_manager = this;
+
+  // Enable the dump provider with IO thread affinity. Note that
+  // unregistration happens on the IO thread (See
+  // BrowserProcessSubThread::IOThreadPreCleanUp).

[jbauman, 2017/07/06 21:39:18]

This comment about unregistration is incorrect now. Also, shouldn't we still
unregister it? I can imagine us wanting to do a memory dump on shutdown in the
future.

[sadrul, 2017/07/06 23:38:18]

Hm, you are right. The MemoryDumpManager does not trigger the provider if the
provider is not running on the specified task-runner, and since the IO thread
has been torn down at this point, it would never run for
BrowserGpuMemoryBufferManager. However, if we attempt to do a memory dump during
tear-down, it will still try to poke the now-destroyed provider to look at its
task-runner, causing uaf.

I have restored the unregistration in BrowserProcessSubThread.

+  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+      this, "BrowserGpuMemoryBufferManager",
+      BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
 }
 
 BrowserGpuMemoryBufferManager::~BrowserGpuMemoryBufferManager() {
   g_gpu_memory_buffer_manager = nullptr;
 }
 
 // static
 BrowserGpuMemoryBufferManager* BrowserGpuMemoryBufferManager::current() {
   return g_gpu_memory_buffer_manager;
 }
@@ skipping 389 matching lines @@
       format(format),
       usage(usage),
       gpu_host_id(gpu_host_id) {}
 
 BrowserGpuMemoryBufferManager::BufferInfo::BufferInfo(const BufferInfo& other) =
     default;
 
 BrowserGpuMemoryBufferManager::BufferInfo::~BufferInfo() {}
 
 }  // namespace content