3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2024-12-27 15:32:10 +08:00
Code Issues Projects Releases Wiki Activity

win: Fix ProcessInfo.OtherProcess test flake

Browse Source 
Removes the /BASE:N and /FIXED arguments to the child, which weren't
actually testing correctly (see bug), and were causing problems at least
on Win7 when something collided with that address.

Additionally, switches to storing modules in load order, rather than a
combination of memory order and initialization order, since that was a
bit confusing and there was no great rationale for it.

While reviewing, handle the case of a corrupted module name, and if it's
unreadable continue emitting "???" as a name. Adds a test for this
functionality.

Bug: chromium:792619
Change-Id: I2e95a81b02fe4d527868f6a5f980d315604255a6
Reviewed-on: https://chromium-review.googlesource.com/815875
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This commit is contained in:
Scott Graham 2017-12-08 16:01:18 -08:00 committed by Commit Bot
parent c04c05564d
commit 92e8bc4713
5 changed files with 74 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
util/BUILD.gn
View File

@ -491,14 +491,6 @@ if (is_win) {
sources = [
"win/process_info_test_child.cc",
]
# Set an unusually high load address to make sure that the main executable
# still appears as the first element in ProcessInfo::Modules().
ldflags = [
"/BASE:0x78000000",
"/DYNAMICBASE:NO",
"/FIXED",
]
}
executable("crashpad_util_test_safe_terminate_process_test_child") {

12
util/util_test.gyp
View File

@ -169,18 +169,6 @@
'sources': [
'win/process_info_test_child.cc',
],
# Set an unusually high load address to make sure that the main
# executable still appears as the first element in
# ProcessInfo::Modules().
'msvs_settings': {
'VCLinkerTool': {
'AdditionalOptions': [
'/BASE:0x78000000',
],
'RandomizedBaseAddress': '1', # /DYNAMICBASE:NO.
'FixedBaseAddress': '2', # /FIXED.
},
},
},
{
'target_name': 'crashpad_util_test_safe_terminate_process_test_child',

35
util/win/process_info.cc
View File

@ -269,36 +269,15 @@ bool ReadProcessData(HANDLE process,
return false;
process_types::LDR_DATA_TABLE_ENTRY<Traits> ldr_data_table_entry;
// Include the first module in the memory order list to get our the main
// executable's name, as it's not included in initialization order below.
if (!ReadStruct(process,
static_cast<WinVMAddress>(
peb_ldr_data.InMemoryOrderModuleList.Flink) -
offsetof(process_types::LDR_DATA_TABLE_ENTRY<Traits>,
InMemoryOrderLinks),
&ldr_data_table_entry)) {
return false;
}
ProcessInfo::Module module;
if (!ReadUnicodeString(
process, ldr_data_table_entry.FullDllName, &module.name)) {
return false;
}
module.dll_base = ldr_data_table_entry.DllBase;
module.size = ldr_data_table_entry.SizeOfImage;
module.timestamp = ldr_data_table_entry.TimeDateStamp;
process_info->modules_.push_back(module);
// Walk the PEB LDR structure (doubly-linked list) to get the list of loaded
// modules. We use this method rather than EnumProcessModules to get the
// modules in initialization order rather than memory order.
typename Traits::Pointer last =
peb_ldr_data.InInitializationOrderModuleList.Blink;
for (typename Traits::Pointer cur =
peb_ldr_data.InInitializationOrderModuleList.Flink;
;
cur = ldr_data_table_entry.InInitializationOrderLinks.Flink) {
// modules in load order rather than memory order. Notably, this includes the
// main executable as the first element.
typename Traits::Pointer last = peb_ldr_data.InLoadOrderModuleList.Blink;
for (typename Traits::Pointer cur = peb_ldr_data.InLoadOrderModuleList.Flink;;
cur = ldr_data_table_entry.InLoadOrderLinks.Flink) {
// |cur| is the pointer to the LIST_ENTRY embedded in the
// LDR_DATA_TABLE_ENTRY, in the target process's address space. So we need
// to read from the target, and also offset back to the beginning of the
@ -306,14 +285,14 @@ bool ReadProcessData(HANDLE process,
if (!ReadStruct(process,
static_cast<WinVMAddress>(cur) -
offsetof(process_types::LDR_DATA_TABLE_ENTRY<Traits>,
InInitializationOrderLinks),
InLoadOrderLinks),
&ldr_data_table_entry)) {
break;
}
// TODO(scottmg): Capture Checksum, etc. too?
if (!ReadUnicodeString(
process, ldr_data_table_entry.FullDllName, &module.name)) {
break;
module.name = L"???";
}
module.dll_base = ldr_data_table_entry.DllBase;
module.size = ldr_data_table_entry.SizeOfImage;

13
util/win/process_info_test.cc
View File

@ -180,11 +180,18 @@ void TestOtherProcess(TestPaths::Architecture architecture) {
// lz32.dll is an uncommonly-used-but-always-available module that the test
// binary manually loads.
static constexpr wchar_t kLz32dllName[] = L"\\lz32.dll";
ASSERT_GE(modules.back().name.size(), wcslen(kLz32dllName));
EXPECT_EQ(modules.back().name.substr(modules.back().name.size() -
wcslen(kLz32dllName)),
auto& lz32 = modules[modules.size() - 2];
ASSERT_GE(lz32.name.size(), wcslen(kLz32dllName));
EXPECT_EQ(lz32.name.substr(lz32.name.size() - wcslen(kLz32dllName)),
kLz32dllName);
// Note that the test code corrupts the PEB MemoryOrder list, whereas
// ProcessInfo::Modules() retrieves the module names via the PEB LoadOrder
// list. These are expected to point to the same strings, but theoretically
// could be separate.
auto& corrupted = modules.back();
EXPECT_EQ(corrupted.name, L"???");
VerifyAddressInInCodePage(process_info, code_address);
}

59
util/win/process_info_test_child.cc
View File

@ -13,10 +13,26 @@
// limitations under the License.
#include <intrin.h>
#include <stdlib.h>
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <wchar.h>
#include <windows.h>
#include <winternl.h>
namespace {
bool UnicodeStringEndsWithCaseInsensitive(const UNICODE_STRING& us,
const wchar_t* ends_with) {
const size_t len = wcslen(ends_with);
// Recall that UNICODE_STRING.Length is in bytes, not characters.
const size_t us_len_in_chars = us.Length / sizeof(wchar_t);
if (us_len_in_chars < len)
return false;
return _wcsnicmp(&us.Buffer[us_len_in_chars - len], ends_with, len) == 0;
}
} // namespace
// A simple binary to be loaded and inspected by ProcessInfo.
int wmain(int argc, wchar_t** argv) {
@ -29,10 +45,49 @@ int wmain(int argc, wchar_t** argv) {
abort();
// Load an unusual module (that we don't depend upon) so we can do an
// existence check.
// existence check. It's also important that these DLLs don't depend on
// any other DLLs, otherwise there'll be additional modules in the list, which
// the test expects not to be there.
if (!LoadLibrary(L"lz32.dll"))
abort();
// Load another unusual module so we can destroy its FullDllName field in the
// PEB to test corrupted name reads.
static constexpr wchar_t kCorruptableDll[] = L"kbdurdu.dll";
if (!LoadLibrary(kCorruptableDll))
abort();
// Find and corrupt the buffer pointer to the name in the PEB.
HINSTANCE ntdll = GetModuleHandle(L"ntdll.dll");
decltype(NtQueryInformationProcess)* nt_query_information_process =
reinterpret_cast<decltype(NtQueryInformationProcess)*>(
GetProcAddress(ntdll, "NtQueryInformationProcess"));
if (!nt_query_information_process)
abort();
PROCESS_BASIC_INFORMATION pbi;
if (nt_query_information_process(GetCurrentProcess(),
ProcessBasicInformation,
&pbi,
sizeof(pbi),
nullptr) < 0) {
abort();
}
PEB_LDR_DATA* ldr = pbi.PebBaseAddress->Ldr;
LIST_ENTRY* head = &ldr->InMemoryOrderModuleList;
LIST_ENTRY* next = head->Flink;
while (next != head) {
LDR_DATA_TABLE_ENTRY* entry =
CONTAINING_RECORD(next, LDR_DATA_TABLE_ENTRY, InMemoryOrderLinks);
if (UnicodeStringEndsWithCaseInsensitive(entry->FullDllName,
kCorruptableDll)) {
// Corrupt the pointer to the name.
entry->FullDllName.Buffer = 0;
}
next = next->Flink;
}
HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE);
if (out == INVALID_HANDLE_VALUE)
abort();