Commit c928d34e authored by shrikant's avatar shrikant Committed by Commit bot

Add HANDLE_FLAG_PROTECT_FROM_CLOSE flag to Tracked/ScopedHandle.

Idea here is to try and protect our scoped handles so that any third party close/accidental close could be some what restricted.

Besides adding flag following changes are in this CL.
1. While launching test process (Python executable), we pass it a pipe handle which we add it to ScopedHandle before passing, this marked handle as protected, now when we let python process inherit that handle, python process cannot close it and throws exception. We now duplicate handle without protection and then pass it onto python process and let host process continue with protected scoped handle.
2. Other change is related to disabling of active verifier dynamically. If ActiveVerifier gets invoked before disabling it (because someone used scoped handle before instruction pointer reached to UseHooks/InstallCloseHandleHooks) then it protects handle from closing. If now we disable ActiveVerifier when scoped handle goes out of scope, it tries to stop tracking handle, but in this case as active verifier got disabled later, that particular handle remains protected. Now we don't disable active verifier dynamically, but just decide on setting up of CloseHandleHook based on certain parameters.

BUG=475872
R=cpu@chromium.org,rvargas@chromium.org

Review URL: https://codereview.chromium.org/1113063004

Cr-Commit-Position: refs/heads/master@{#329516}
parent 795f2d66
...@@ -152,6 +152,12 @@ void ActiveVerifier::StartTracking(HANDLE handle, const void* owner, ...@@ -152,6 +152,12 @@ void ActiveVerifier::StartTracking(HANDLE handle, const void* owner,
if (!enabled_) if (!enabled_)
return; return;
// Idea here is to make our handles non-closable until we close it ourselves.
// Handles provided could be totally fabricated especially through our
// unittest, we are ignoring that for now by not checking return value.
::SetHandleInformation(handle, HANDLE_FLAG_PROTECT_FROM_CLOSE,
HANDLE_FLAG_PROTECT_FROM_CLOSE);
// Grab the thread id before the lock. // Grab the thread id before the lock.
DWORD thread_id = GetCurrentThreadId(); DWORD thread_id = GetCurrentThreadId();
...@@ -172,6 +178,15 @@ void ActiveVerifier::StopTracking(HANDLE handle, const void* owner, ...@@ -172,6 +178,15 @@ void ActiveVerifier::StopTracking(HANDLE handle, const void* owner,
if (!enabled_) if (!enabled_)
return; return;
// We expect handle to be protected till this point.
DWORD flags = 0;
if (::GetHandleInformation(handle, &flags)) {
CHECK_NE(0U, (flags & HANDLE_FLAG_PROTECT_FROM_CLOSE));
// Unprotect handle so that it could be closed.
::SetHandleInformation(handle, HANDLE_FLAG_PROTECT_FROM_CLOSE, 0);
}
AutoNativeLock lock(*lock_); AutoNativeLock lock(*lock_);
HandleMap::iterator i = map_.find(handle); HandleMap::iterator i = map_.find(handle);
if (i == map_.end()) if (i == map_.end())
......
...@@ -185,7 +185,8 @@ bool UseHooks() { ...@@ -185,7 +185,8 @@ bool UseHooks() {
#elif defined(NDEBUG) #elif defined(NDEBUG)
chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel(); chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
if (channel == chrome::VersionInfo::CHANNEL_CANARY || if (channel == chrome::VersionInfo::CHANNEL_CANARY ||
channel == chrome::VersionInfo::CHANNEL_DEV) { channel == chrome::VersionInfo::CHANNEL_DEV ||
channel == chrome::VersionInfo::CHANNEL_UNKNOWN) {
return true; return true;
} }
...@@ -221,8 +222,6 @@ void InstallCloseHandleHooks() { ...@@ -221,8 +222,6 @@ void InstallCloseHandleHooks() {
// threads attempting to call CloseHandle. // threads attempting to call CloseHandle.
hooks->AddEATPatch(); hooks->AddEATPatch();
PatchLoadedModules(hooks); PatchLoadedModules(hooks);
} else {
base::win::DisableHandleVerifier();
} }
} }
......
...@@ -101,8 +101,9 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) { ...@@ -101,8 +101,9 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) {
child_write_fd_.Set(child_write); child_write_fd_.Set(child_write);
// Have the child inherit the write half. // Have the child inherit the write half.
if (!SetHandleInformation(child_write, HANDLE_FLAG_INHERIT, if (!::DuplicateHandle(::GetCurrentProcess(), child_write,
HANDLE_FLAG_INHERIT)) { ::GetCurrentProcess(), &child_write, 0, TRUE,
DUPLICATE_SAME_ACCESS)) {
PLOG(ERROR) << "Failed to enable pipe inheritance"; PLOG(ERROR) << "Failed to enable pipe inheritance";
return false; return false;
} }
...@@ -125,9 +126,11 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) { ...@@ -125,9 +126,11 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) {
process_ = base::LaunchProcess(python_command, launch_options); process_ = base::LaunchProcess(python_command, launch_options);
if (!process_.IsValid()) { if (!process_.IsValid()) {
LOG(ERROR) << "Failed to launch " << python_command.GetCommandLineString(); LOG(ERROR) << "Failed to launch " << python_command.GetCommandLineString();
::CloseHandle(child_write);
return false; return false;
} }
::CloseHandle(child_write);
return true; return true;
} }
......
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