Commit 1155ea4b authored by Bruce Dawson's avatar Bruce Dawson Committed by Commit Bot

Revert "Fix Windows sandbox to work with Application Verifier"

This reverts commit da69db99.

Reason for revert: Suspected of causing the crashes tracked by crbug.com/814641

Original change's description:
> Fix Windows sandbox to work with Application Verifier
> 
> Application Verifier is a useful tool for tracking down bugs on Windows.
> However Chrome's sandbox has not played well with App Verifier - it ends
> up initializing a heap before the process is ready for this. This change
> teaches the NtMapViewOfSection to skip InitHeap when handling the App
> Verifier DLLs, which don't need patching anyway.
> 
> With this change I can run Chrome with the default App Verifier settings
> enabled with the exception of Handles and TLS (those failures probably
> don't represent real bugs), without using --no-sandbox.
> 
> Bug: 752344
> Change-Id: Id1fd4be9c0f080513bbdb649ed5cf2637df8474c
> Reviewed-on: https://chromium-review.googlesource.com/925821
> Reviewed-by: James Forshaw <forshaw@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538307}

TBR=brucedawson@chromium.org,forshaw@chromium.org,wfh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 752344
Change-Id: Icd8feea1779f2891389762db0773ecf0ed9ad762
Reviewed-on: https://chromium-review.googlesource.com/934862Reviewed-by: default avatarBruce Dawson <brucedawson@chromium.org>
Reviewed-by: default avatarWill Harris <wfh@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538851}
parent e1870c41
...@@ -481,29 +481,6 @@ UNICODE_STRING* GetImageInfoFromModule(HMODULE module, uint32_t* flags) { ...@@ -481,29 +481,6 @@ UNICODE_STRING* GetImageInfoFromModule(HMODULE module, uint32_t* flags) {
#pragma warning(pop) #pragma warning(pop)
} }
const char* GetAnsiImageInfoFromModule(HMODULE module) {
// PEImage's dtor won't be run during SEH unwinding, but that's OK.
#pragma warning(push)
#pragma warning(disable : 4509)
const char* out_name = nullptr;
__try {
do {
base::win::PEImage pe(module);
if (!pe.VerifyMagic())
break;
PIMAGE_EXPORT_DIRECTORY exports = pe.GetExportDirectory();
if (exports)
out_name = static_cast<const char*>(pe.RVAToAddr(exports->Name));
} while (false);
} __except (EXCEPTION_EXECUTE_HANDLER) {
}
return out_name;
#pragma warning(pop)
}
UNICODE_STRING* GetBackingFilePath(PVOID address) { UNICODE_STRING* GetBackingFilePath(PVOID address) {
// We'll start with something close to max_path charactes for the name. // We'll start with something close to max_path charactes for the name.
SIZE_T buffer_bytes = MAX_PATH * 2; SIZE_T buffer_bytes = MAX_PATH * 2;
......
...@@ -144,12 +144,6 @@ enum MappedModuleFlags { ...@@ -144,12 +144,6 @@ enum MappedModuleFlags {
// operator delete(name, NT_ALLOC); // operator delete(name, NT_ALLOC);
UNICODE_STRING* GetImageInfoFromModule(HMODULE module, uint32_t* flags); UNICODE_STRING* GetImageInfoFromModule(HMODULE module, uint32_t* flags);
// Returns the name and characteristics for a given PE module. The return
// value is the name as defined by the export table.
//
// The returned buffer is within the PE module and must not be freed.
const char* GetAnsiImageInfoFromModule(HMODULE module);
// Returns the full path and filename for a given dll. // Returns the full path and filename for a given dll.
// May return nullptr if the provided address is not backed by a named section, // May return nullptr if the provided address is not backed by a named section,
// or if the current OS version doesn't support the call. The returned buffer // or if the current OS version doesn't support the call. The returned buffer
......
...@@ -13,14 +13,6 @@ namespace sandbox { ...@@ -13,14 +13,6 @@ namespace sandbox {
SANDBOX_INTERCEPT NtExports g_nt; SANDBOX_INTERCEPT NtExports g_nt;
const char VERIFIER_DLL_NAME[] = "verifier.dll";
const char KERNEL32_DLL_NAME[] = "kernel32.dll";
enum SectionLoadState {
kBeforeKernel32,
kAfterKernel32,
};
// Hooks NtMapViewOfSection to detect the load of DLLs. If hot patching is // Hooks NtMapViewOfSection to detect the load of DLLs. If hot patching is
// required for this dll, this functions patches it. // required for this dll, this functions patches it.
NTSTATUS WINAPI NTSTATUS WINAPI
...@@ -38,43 +30,21 @@ TargetNtMapViewOfSection(NtMapViewOfSectionFunction orig_MapViewOfSection, ...@@ -38,43 +30,21 @@ TargetNtMapViewOfSection(NtMapViewOfSectionFunction orig_MapViewOfSection,
NTSTATUS ret = orig_MapViewOfSection(section, process, base, zero_bits, NTSTATUS ret = orig_MapViewOfSection(section, process, base, zero_bits,
commit_size, offset, view_size, inherit, commit_size, offset, view_size, inherit,
allocation_type, protect); allocation_type, protect);
static SectionLoadState s_state = kBeforeKernel32;
static int s_load_count = 0;
if (1 == s_load_count) {
SandboxFactory::GetTargetServices()->GetState()->SetKernel32Loaded();
s_load_count = 2;
}
do { do {
if (!NT_SUCCESS(ret)) if (!NT_SUCCESS(ret))
break; break;
if (!IsSameProcess(process)) if (!InitHeap())
break; break;
// Only check for verifier.dll or kernel32.dll loading if we haven't moved if (!IsSameProcess(process))
// past that state yet.
if (s_state == kBeforeKernel32) {
const char* ansi_module_name =
GetAnsiImageInfoFromModule(reinterpret_cast<HMODULE>(*base));
CHECK(ansi_module_name);
// _strnicmp below may hit read access violations for some sections. We
// find what looks like a valid export directory for a PE module but the
// pointer to the module name will be pointing to invalid memory.
__try {
// Don't initialize the heap if verifier.dll is being loaded. This
// indicates Application Verifier is enabled and we should wait until
// the next module is loaded.
if (g_nt._strnicmp(ansi_module_name, VERIFIER_DLL_NAME,
sizeof(VERIFIER_DLL_NAME)) == 0)
break;
if (g_nt._strnicmp(ansi_module_name, KERNEL32_DLL_NAME,
sizeof(KERNEL32_DLL_NAME)) == 0) {
SandboxFactory::GetTargetServices()->GetState()->SetKernel32Loaded();
s_state = kAfterKernel32;
}
} __except (EXCEPTION_EXECUTE_HANDLER) {
}
}
if (!InitHeap())
break; break;
if (!IsValidImageSection(section, base, offset, view_size)) if (!IsValidImageSection(section, base, offset, view_size))
...@@ -109,6 +79,9 @@ TargetNtMapViewOfSection(NtMapViewOfSectionFunction orig_MapViewOfSection, ...@@ -109,6 +79,9 @@ TargetNtMapViewOfSection(NtMapViewOfSectionFunction orig_MapViewOfSection,
} while (false); } while (false);
if (!s_load_count)
s_load_count = 1;
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment