win: Add more memory regions to gathering of PEB

snapshot/win/process_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,
@@ skipping 30 matching lines @@
 bool ProcessSnapshotWin::Initialize(HANDLE process,
                                     ProcessSuspensionState suspension_state) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
   GetTimeOfDay(&snapshot_time_);
 
   if (!process_reader_.Initialize(process, suspension_state))
     return false;
 
   system_.Initialize(&process_reader_);
-  WinVMAddress peb_address;
-  WinVMSize peb_size;
-  process_reader_.GetProcessInfo().Peb(&peb_address, &peb_size);
-  peb_.Initialize(&process_reader_, peb_address, peb_size);
+
+  if (process_reader_.Is64Bit())
+    InitializePebData<process_types::internal::Traits64>();
+  else
+    InitializePebData<process_types::internal::Traits32>();
 
   InitializeThreads();
   InitializeModules();
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
 }
 
 bool ProcessSnapshotWin::InitializeException(
     WinVMAddress exception_information_address) {
@@ skipping 114 matching lines @@
   return modules;
 }
 
 const ExceptionSnapshot* ProcessSnapshotWin::Exception() const {
   return exception_.get();
 }
 
 std::vector<const MemorySnapshot*> ProcessSnapshotWin::ExtraMemory() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   std::vector<const MemorySnapshot*> extra_memory;
-  extra_memory.push_back(&peb_);
+  for (const auto& peb_memory : peb_memory_)
+    extra_memory.push_back(peb_memory);
   return extra_memory;
 }
 
 void ProcessSnapshotWin::InitializeThreads() {
   const std::vector<ProcessReaderWin::Thread>& process_reader_threads =
       process_reader_.Threads();
   for (const ProcessReaderWin::Thread& process_reader_thread :
        process_reader_threads) {
     auto thread = make_scoped_ptr(new internal::ThreadSnapshotWin());
     if (thread->Initialize(&process_reader_, process_reader_thread)) {
       threads_.push_back(thread.release());
     }
   }
 }
 
 void ProcessSnapshotWin::InitializeModules() {
   const std::vector<ProcessInfo::Module>& process_reader_modules =
       process_reader_.Modules();
   for (const ProcessInfo::Module& process_reader_module :
        process_reader_modules) {
     auto module = make_scoped_ptr(new internal::ModuleSnapshotWin());
     if (module->Initialize(&process_reader_, process_reader_module)) {
       modules_.push_back(module.release());
     }
   }
 }
 
+template <class Traits>
+void ProcessSnapshotWin::InitializePebData() {
+  WinVMAddress peb_address;
+  WinVMSize peb_size;
+  process_reader_.GetProcessInfo().Peb(&peb_address, &peb_size);
+  peb_memory_.push_back(CreateMemorySnapshot(peb_address, peb_size));
+
+  process_types::PEB<Traits> peb_data;
+  if (!process_reader_.ReadMemory(peb_address, peb_size, &peb_data)) {
+    LOG(ERROR) << "ReadMemory PEB";
+    return;
+  }
+  peb_memory_.push_back(CreateMemorySnapshot(
+      peb_data.Ldr, sizeof(process_types::PEB_LDR_DATA<Traits>)));
+
+  process_types::RTL_USER_PROCESS_PARAMETERS<Traits> process_parameters;
+  if (!process_reader_.ReadMemory(peb_data.ProcessParameters,
+                                  sizeof(process_parameters),
+                                  &process_parameters)) {
+    LOG(ERROR) << "ReadMemory RTL_USER_PROCESS_PARAMETERS";
+    return;
+  }
+
+  peb_memory_.push_back(CreateMemorySnapshotForUNICODE_STRING(
+      process_parameters.CurrentDirectory.DosPath));
+  peb_memory_.push_back(
+      CreateMemorySnapshotForUNICODE_STRING(process_parameters.DllPath));

[Mark Mentovai, 2015/09/25 21:10:16]

The “now” section of the CL description seems to indicate that this isn’t
working. Or was this field just empty in your test?

[scottmg, 2015/09/25 22:52:25]

Yes, it's often null in a live process too. It's the search path that the loader
uses to locate dlls, but if it's not overridden then (I guess) it doesn't get
filled in here.

+  peb_memory_.push_back(
+      CreateMemorySnapshotForUNICODE_STRING(process_parameters.ImagePathName));
+  peb_memory_.push_back(
+      CreateMemorySnapshotForUNICODE_STRING(process_parameters.CommandLine));
+  // http://blogs.msdn.com/b/oldnewthing/archive/2010/02/03/9957320.aspx On
+  // newer OSs there's no stated limit, but in practice grabbing 32k should be
+  // more than enough.
+  const int kMaxEnvironmentBlockSize = 32768;
+  peb_memory_.push_back(CreateMemorySnapshot(process_parameters.Environment,
+                                             kMaxEnvironmentBlockSize));

[Mark Mentovai, 2015/09/25 21:10:16]

It might be worth trying to work out the actual size of the environment block so
that we don’t over-report, but you can save that for a TODO if it’s rough to
work out.

[scottmg, 2015/09/25 22:52:25]

I don't know of any way other than probing the target process for \0, which is a
bit tedious (and I'm not sure if it's 100% correct either). Added a TODO as
*something* must know how big that block is.

[Mark Mentovai, 2015/09/25 23:11:59]

scottmg wrote:
Well, if windbg’s able to work out where to stop printing environment variables,
there’s got to be some way to do it. I think you just look for a double-NUL.
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682653(v=vs.85).aspx

[scottmg, 2015/09/26 00:07:38]

Yeah, that was my thinking too. But then I was worried that we might lose some
interesting information if the environment block was the thing that had been
futzed with. But maybe that's useful too, because you'd see what the process
would probably be able to retrieve.

... ok, switched to grabbing 32k here to find the \0\0 and then only storing up
to that point.

+}
+
+internal::MemorySnapshotWin* ProcessSnapshotWin::CreateMemorySnapshot(
+    WinVMAddress address,
+    WinVMAddress size) {

[Mark Mentovai, 2015/09/25 21:10:16]

If size is 0, it’s probably worth not creating anything. Instead, return
nullptr, and have the caller not add anything to the peb_memory_ vector.

That may mean that it’s easier for this return void and take the vector as an
argument, appending to it if size is nonzero, instead of returning something for
the caller to append (or not).

[scottmg, 2015/09/25 22:52:25]

Done.

+  internal::MemorySnapshotWin* ret = new internal::MemorySnapshotWin();
+  ret->Initialize(&process_reader_, address, size);
+  return ret;
+}
+
 }  // namespace crashpad