win: Add Handles() to ProcessInfo

util/win/process_info.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 "util/win/process_info.h"
 
 #include <winternl.h>
 
 #include <algorithm>
 #include <limits>
 
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "base/template_util.h"
 #include "build/build_config.h"
 #include "util/numeric/safe_assignment.h"
+#include "util/win/nt_internals.h"
 #include "util/win/ntstatus_logging.h"
 #include "util/win/process_structs.h"
+#include "util/win/scoped_handle.h"
 
 namespace crashpad {
 
 namespace {
 
 NTSTATUS NtQueryInformationProcess(HANDLE process_handle,
                                    PROCESSINFOCLASS process_information_class,
                                    PVOID process_information,
                                    ULONG process_information_length,
                                    PULONG return_length) {
@@ skipping 81 matching lines @@
   mbi64.BaseAddress = reinterpret_cast<ULONGLONG>(mbi.BaseAddress);
   mbi64.AllocationBase = reinterpret_cast<ULONGLONG>(mbi.AllocationBase);
   mbi64.AllocationProtect = mbi.AllocationProtect;
   mbi64.RegionSize = mbi.RegionSize;
   mbi64.State = mbi.State;
   mbi64.Protect = mbi.Protect;
   mbi64.Type = mbi.Type;
   return mbi64;
 }
 
+// NtQueryObject with a retry for size mismatch and a hint for initial size.
+scoped_ptr<uint8_t[]> QueryObject(
+    HANDLE handle,
+    OBJECT_INFORMATION_CLASS object_information_class,
+    size_t size) {
+  ULONG return_length;
+  scoped_ptr<uint8_t[]> buffer(new uint8_t[size]);
+  NTSTATUS status = crashpad::NtQueryObject(
+      handle, object_information_class, buffer.get(), size, &return_length);
+  if (status == STATUS_INFO_LENGTH_MISMATCH) {
+    DCHECK_GE(return_length, size);
+    size = return_length;
+    buffer.reset(new uint8_t[size]);
+    status = crashpad::NtQueryObject(
+        handle, object_information_class, buffer.get(), size, &return_length);
+    if (!NT_SUCCESS(status)) {
+      NTSTATUS_LOG(ERROR, status) << "NtQueryObject";
+      return scoped_ptr<uint8_t[]>();
+    }
+  }

[Mark Mentovai, 2015/10/14 22:49:55]

  } else if (!NT_SUCCESS(status)) {
    NTSTATUS_LOG(ERROR, status) << "NtQueryObject";

[scottmg, 2015/10/15 00:17:41]

Done.

+
+  return buffer.Pass();
+}
+
 }  // namespace
 
 template <class Traits>
 bool GetProcessBasicInformation(HANDLE process,
                                 bool is_wow64,
                                 ProcessInfo* process_info,
                                 WinVMAddress* peb_address,
                                 WinVMSize* peb_size) {
   ULONG bytes_returned;
   process_types::PROCESS_BASIC_INFORMATION<Traits> process_basic_information;
@@ skipping 167 matching lines @@
 
     if (memory_basic_information.RegionSize == 0) {
       LOG(ERROR) << "RegionSize == 0";
       return false;
     }
   }
 
   return true;
 }
 
+template <class Traits>
+std::vector<ProcessInfo::Handle> ProcessInfo::BuildHandleVector(
+    HANDLE process) const {
+  size_t buffer_size = 2 * 1024 * 1024;
+  scoped_ptr<uint8_t[]> buffer(new uint8_t[buffer_size]);
+
+  // Typically if the buffer were too small, STATUS_INFO_LENGTH_MISMATCH would
+  // return the correct size in the final argument, but it does not in for this
+  // SYSTEM_INFORMATION_CLASS, so we loop and attempt larger sizes.
+  NTSTATUS status;
+  for (int tries = 0; tries < 5; ++tries) {
+    status = crashpad::NtQuerySystemInformation(

[Mark Mentovai, 2015/10/14 22:49:55]

Do you want a QuerySystemInformation() wrapper for this to handle the resizing,
kind of like QueryObject()?

[scottmg, 2015/10/15 00:17:41]

It's only done once at the moment, so I think it's OK to keep it inline. I could
be convinced either way though.

+        static_cast<SYSTEM_INFORMATION_CLASS>(SystemExtendedHandleInformation),
+        buffer.get(),
+        buffer_size,
+        nullptr);
+    if (NT_SUCCESS(status)) {
+      break;
+    } else if (status == STATUS_INFO_LENGTH_MISMATCH) {
+      buffer_size *= 2;

[Mark Mentovai, 2015/10/14 22:49:55]

Why don’t you use the ReturnLength argument to tell you what to resize to, as in
your QueryObject() wrapper?

[scottmg, 2015/10/15 00:17:41]

That's the comment above the loop -- it doesn't properly return the expected
length on some OSs unfortunately, see e.g.
http://forum.sysinternals.com/topic18892.html at the end of Step 1.

+      buffer.reset(new uint8_t[buffer_size]);
+    }
+  }
+
+  if (!NT_SUCCESS(status)) {
+    NTSTATUS_LOG(ERROR, status)
+        << "NtQuerySystemInformation SystemExtendedHandleInformation";
+    return std::vector<Handle>();
+  }
+
+  process_types::SYSTEM_HANDLE_INFORMATION_EX<
+      Traits>* system_handle_information_ex =

[Mark Mentovai, 2015/10/14 22:49:55]

Weird formatting (did clang-format do this?!)

[scottmg, 2015/10/15 00:17:41]

Yup. A little better with an auto*?

+      reinterpret_cast<process_types::SYSTEM_HANDLE_INFORMATION_EX<Traits>*>(
+          buffer.get());
+
+  std::vector<Handle> handles;
+
+  for (size_t i = 0; i < system_handle_information_ex->NumberOfHandles; ++i) {
+    const auto& handle = system_handle_information_ex->Handles[i];
+    if (handle.UniqueProcessId != process_id_)
+      continue;
+
+    // TODO(scottmg): Could special case for self.
+    HANDLE dup_handle;
+    if (!DuplicateHandle(process,

[Mark Mentovai, 2015/10/14 22:49:55]

In https://code.google.com/p/crashpad/issues/detail?id=21#c2, you claimed that
DuplicateHandle() to dupe handles into our process was scary. It certainly
registers on my heebie-jeebie-meter too, but I’m not sure if that’s me being
overly paranoid or if there’s an actual risk. But maybe it’s fine to just use
with NtQueryObject() as you’re doing here. Can you clarify what you thought was
scary?

You also said that you thought you’d start with option 1, no type and no name,
but here you’re grabbing the type. Did something change in the analysis?

[scottmg, 2015/10/15 00:17:41]

DuplicateHandle()ing into ourselves scared me at first, but after talking to
Justin I was less nervous about that. For "Level #3" I'm definitely a little
more nervous about the NtQueryObject calls with obscure/undocumented values as
people have clearly noted hangs on some OSs. But I (and more importantly Justin)
think it's OK to only do the duplicate and the limited information query.

+                         reinterpret_cast<HANDLE>(handle.HandleValue),
+                         GetCurrentProcess(),
+                         &dup_handle,
+                         0,
+                         false,
+                         DUPLICATE_SAME_ACCESS)) {
+      // Some handles cannot be duplicated, for example handles of type
+      // EtwRegistration, so we simply drop these. (We also do not strictly know

[Mark Mentovai, 2015/10/14 22:49:55]

Does MiniDumpWriteDump() lose these too?

Is there an error code that we can key this off?

[scottmg, 2015/10/15 00:17:41]

Just confirmed with MiniDumpWriteDump() via
https://codereview.chromium.org/1405443006 that it doesn't have any information
for them (i.e. can't DuplicateHandle() them either, I guess), but it does at
least include them in the list of handles with no additional information, which
is better. Switched to that on (any) duplicate failure.

+      // that we're suspended here, so there's a race between here and
+      // collecting them, though in practice the target process will always be
+      // suspended.)
+      continue;
+    }
+    ScopedKernelHANDLE scoped_dup_handle(dup_handle);
+
+    scoped_ptr<uint8_t[]> object_basic_information_buffer =
+        QueryObject(dup_handle,
+                    ObjectBasicInformation,
+                    sizeof(PUBLIC_OBJECT_BASIC_INFORMATION));
+    if (!object_basic_information_buffer)
+      continue;
+
+    scoped_ptr<uint8_t[]> object_type_information_buffer =
+        QueryObject(dup_handle,
+                    ObjectTypeInformation,
+                    sizeof(PUBLIC_OBJECT_TYPE_INFORMATION));
+    if (!object_type_information_buffer)
+      continue;
+
+    PUBLIC_OBJECT_BASIC_INFORMATION* object_basic_information =
+        reinterpret_cast<PUBLIC_OBJECT_BASIC_INFORMATION*>(
+            object_basic_information_buffer.get());
+    PUBLIC_OBJECT_TYPE_INFORMATION* object_type_information =
+        reinterpret_cast<PUBLIC_OBJECT_TYPE_INFORMATION*>(
+            object_type_information_buffer.get());
+
+    Handle result_handle;
+    result_handle.type_name =
+        std::wstring(object_type_information->TypeName.Buffer,
+                     object_type_information->TypeName.Length /

[Mark Mentovai, 2015/10/14 22:49:55]

DCHECK that Length % sizeof(result_handle.type_name[0]) is 0.

[scottmg, 2015/10/15 00:17:41]

Done.

+                         sizeof(result_handle.type_name[0]));
+    result_handle.handle = static_cast<uint32_t>(handle.HandleValue);
+    // The Attributes and GrantedAccess sometimes differ slightly between the

[Mark Mentovai, 2015/10/14 22:49:55]

Blank before. Same on 439.

[scottmg, 2015/10/15 00:17:41]

Done.

+    // data retrieved in SYSTEM_HANDLE_INFORMATION_EX and
+    // PUBLIC_OBJECT_TYPE_INFORMATION. We prefer the values in
+    // SYSTEM_HANDLE_INFORMATION_EX because they were retrieved from the target
+    // process, rather than on the duplicated handle.
+    result_handle.attributes = handle.HandleAttributes;
+    result_handle.granted_access = handle.GrantedAccess;
+    result_handle.pointer_count = object_basic_information->PointerCount;
+    // Subtract one to account for our DuplicateHandle().
+    DCHECK_GT(object_basic_information->HandleCount, 0u);

[Mark Mentovai, 2015/10/14 22:49:55]

Should this be > 1? At least one original reference, and one for the duplicate
in here?

[scottmg, 2015/10/15 00:17:41]

Yeah, done.

+    result_handle.handle_count = object_basic_information->HandleCount - 1;

[Mark Mentovai, 2015/10/14 22:49:55]

Happy to see this accounted for!

+    handles.push_back(result_handle);
+  }
+  return handles;
+}
+
 ProcessInfo::Module::Module() : name(), dll_base(0), size(0), timestamp() {
 }
 
 ProcessInfo::Module::~Module() {
 }
 
+ProcessInfo::Handle::Handle()
+    : type_name(),
+      handle(),

[Mark Mentovai, 2015/10/14 22:49:55]

Zeroes in the integer fields.

[scottmg, 2015/10/15 00:17:41]

Done.

+      attributes(),
+      granted_access(),
+      pointer_count(),
+      handle_count() {
+}
+
+ProcessInfo::Handle::~Handle() {
+}
+
 ProcessInfo::ProcessInfo()
     : process_id_(),
       inherited_from_process_id_(),
       command_line_(),
       peb_address_(0),
       peb_size_(0),
       modules_(),
       memory_info_(),
       is_64_bit_(false),

[Mark Mentovai, 2015/10/14 22:49:55]

handles_()

[scottmg, 2015/10/15 00:17:41]

Done.

       is_wow64_(false),
       initialized_() {
 }
 
 ProcessInfo::~ProcessInfo() {
 }
 
 bool ProcessInfo::Initialize(HANDLE process) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
@@ skipping 38 matching lines @@
   if (!result) {
     LOG(ERROR) << "ReadProcessData failed";
     return false;
   }
 
   if (!ReadMemoryInfo(process, is_64_bit_, this)) {
     LOG(ERROR) << "ReadMemoryInfo failed";
     return false;
   }
 
+  if (is_64_bit_)
+    handles_ = BuildHandleVector<process_types::internal::Traits64>(process);
+  else
+    handles_ = BuildHandleVector<process_types::internal::Traits32>(process);
+
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
 }
 
 bool ProcessInfo::Is64Bit() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   return is_64_bit_;
 }
 
 bool ProcessInfo::IsWow64() const {
@@ skipping 95 matching lines @@
     } else {
       result.push_back(as_ranges[i]);
     }
     DCHECK(result.back().IsValid());
   }
 
   return result;
 }
 
 }  // namespace crashpad