win: Save contents of TEBs allowing !teb and !gle to work in windbg

snapshot/win/thread_snapshot_win.cc

 // Copyright 2015 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 #include "snapshot/win/thread_snapshot_win.h"
 
+#include <vector>
+
 #include "base/logging.h"
 #include "snapshot/win/cpu_context_win.h"
 #include "snapshot/win/process_reader_win.h"
 
 namespace crashpad {
 namespace internal {
 
 ThreadSnapshotWin::ThreadSnapshotWin()
-    : ThreadSnapshot(), context_(), stack_(), thread_(), initialized_() {
+    : ThreadSnapshot(),
+      context_(),
+      stack_(),
+      teb_(),
+      thread_(),
+      initialized_() {
 }
 
 ThreadSnapshotWin::~ThreadSnapshotWin() {
 }
 
 bool ThreadSnapshotWin::Initialize(
     ProcessReaderWin* process_reader,
     const ProcessReaderWin::Thread& process_reader_thread) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
   thread_ = process_reader_thread;
   stack_.Initialize(
       process_reader, thread_.stack_region_address, thread_.stack_region_size);
+  teb_.Initialize(process_reader, thread_.teb_address, thread_.teb_size);
 
 #if defined(ARCH_CPU_X86_64)
   if (process_reader->Is64Bit()) {
     context_.architecture = kCPUArchitectureX86_64;
     context_.x86_64 = &context_union_.x86_64;
     InitializeX64Context(process_reader_thread.context.native, context_.x86_64);
   } else {
     context_.architecture = kCPUArchitectureX86;
     context_.x86 = &context_union_.x86;
     InitializeX86Context(process_reader_thread.context.wow64, context_.x86);
@@ skipping 28 matching lines @@
   return thread_.suspend_count;
 }
 
 int ThreadSnapshotWin::Priority() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   return thread_.priority;
 }
 
 uint64_t ThreadSnapshotWin::ThreadSpecificDataAddress() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return thread_.teb;
+  return thread_.teb_address;
+}
+
+std::vector<const MemorySnapshot*> ThreadSnapshotWin::ExtraMemory() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  std::vector<const MemorySnapshot*> extra_memory;

[Mark Mentovai, 2015/10/01 20:28:05]

(1, &teb_)

[scottmg, 2015/10/01 21:01:07]

Done.

+  extra_memory.push_back(&teb_);

[Mark Mentovai, 2015/10/01 20:28:05]

I’ll give you a pass on this for now, as long as you leave behind a link to
crashpad:59.

I guess I’m a little more fearful of this than you. Since a process’ address
space is basically within its own control, it can be busted in pretty much any
way imaginable, intentionally or not. Yeah, it’s true that only a sick thread
would have a screwed-up TEB, but we’re in the crash-reporting business, so we
can’t just throw our hands up in the air because a thread or process is sick.
“Unreadable TEB” may wind up being the reason for a crash some day.

I mention “intentionally” because we should also be aware that garbageware that
injects itself into Chrome might try to break internal structures to prevent it
from being debugged effectively, or to prevent crash dumps from being sent at
all. We’ve got a very strong interest in minimizing that attack area.

Anyway, yeah, we do need to be able to read the TEB ourselves to do our job
here, but we only consult a few of its early fields, and if they’re messed up,
we only bail on reporting the thread altogether. Maybe we should report
something for the thread in that case too, but it’s kind of important that we
don’t toss the whole dump out. With this change, a problem later in the TEB
would be fatal for the dump as a whole, but the thread itself might not appear
to have anything wrong with it until it attempted a TLS operation.

[scottmg, 2015/10/01 21:01:07]

I'm probably not fearful enough. Added TODO links. 

+  return extra_memory;
 }
 
 }  // namespace internal
 }  // namespace crashpad