Add HANDLE_FLAG_PROTECT_FROM_CLOSE flag to Tracked/ScopedHandle.

base/win/scoped_handle.cc

Index: base/win/scoped_handle.cc
diff --git a/base/win/scoped_handle.cc b/base/win/scoped_handle.cc
index 2ebef320bb8b143d4f2944ac87f3afbbc4ef8c69..ce944e43b9fe3fb30a7b699863efeb9ab0c8ee13 100644
--- a/base/win/scoped_handle.cc
+++ b/base/win/scoped_handle.cc
@@ -152,6 +152,12 @@ void ActiveVerifier::StartTracking(HANDLE handle, const void* owner,
   if (!enabled_)
     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);

[Sébastien Marchand, 2015/07/22 15:01:08]

This doesn't work if |handle| is as been created like this:
::DuplicateHandle(::GetCurrentProcess(), STD_OUTPUT_HANDLE,
                  ::GetCurrentProcess(), &handle,
                  0, TRUE, DUPLICATE_SAME_ACCESS);

I don't know if it's a bug or not, as we don't really manipulate the std handles
in Chrome afaik (we close them at startup), but it might be an issue in some
unittests maybe ?

Here's a small repro for this:

  HANDLE original = ::GetStdHandle(STD_OUTPUT_HANDLE);
  HANDLE duplicate = nullptr;
  BOOL result = ::DuplicateHandle(::GetCurrentProcess(), original,
                                  ::GetCurrentProcess(), &duplicate,
                                  0, TRUE, DUPLICATE_SAME_ACCESS);
  assert(result);

  ::SetHandleInformation(duplicate, HANDLE_FLAG_PROTECT_FROM_CLOSE,
                         HANDLE_FLAG_PROTECT_FROM_CLOSE);
  
  DWORD flags = 0;
  result = ::GetHandleInformation(duplicate, &flags);
  assert(result);
  // This assert fail, in this case |flags| == 1 (HANDLE_FLAG_INHERIT).
  assert((flags & HANDLE_FLAG_PROTECT_FROM_CLOSE) != 0);

Is it an misuse of the Handle API or is it a real bug (I don't know so much
about handles...)

We're hitting this in the Syzygy codebase (on this line:
https://github.com/google/syzygy/blob/master/syzygy/kasko/testing/launch_pyth...)

[Sébastien Marchand, 2015/07/22 17:37:04]

Apparently this is an accepted side effect of your change (you don't support
inherited handles), we'll just avoid using the ScopedHandle class for this then.

+
   // Grab the thread id before the lock.
   DWORD thread_id = GetCurrentThreadId();
 
@@ -172,6 +178,15 @@ void ActiveVerifier::StopTracking(HANDLE handle, const void* owner,
   if (!enabled_)
     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_);
   HandleMap::iterator i = map_.find(handle);
   if (i == map_.end())