Add HANDLE_FLAG_PROTECT_FROM_CLOSE flag to Tracked/ScopedHandle.

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

Committed: https://crrev.com/c928d34ee861fd4102c352d9e79e1a4959a47209
Cr-Commit-Position: refs/heads/master@{#329516}

[Shrikant Kelkar, 2015-04-30 23:24:02]

shrikant@chromium.org changed reviewers:
+ rvargas@chromium.org

[rvargas (doing something else), 2015-04-30 23:57:17]

drive-by (no comments on the actual meat of the change)

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (left):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#o...
base/win/scoped_handle.cc:138: g_active_verifier->Disable();
We should not be disabling the verifier on all platforms to deal with a
_regression_ on (the verified code on) one platform. :(

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:223: return;
hold on... we should not be modifying the behavior of the system though this
interception, even if it is for an experiment. All third_party code goes through
here.

[Will Harris, 2015-05-01 00:03:45]

wfh@chromium.org changed reviewers:
+ wfh@chromium.org

[Will Harris, 2015-05-01 00:03:46]

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:165: 
I don't think you can call SetHandleInformation on all handle types for example
you can't on a DC, it seems?

This might break ScopedCreateDC and others...?

https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935.aspx

[Shrikant Kelkar, 2015-05-01 00:06:48]

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:223: return;
On 2015/04/30 23:57:16, rvargas (out of office) wrote:
> hold on... we should not be modifying the behavior of the system though this
> interception, even if it is for an experiment. All third_party code goes
through
> here.

??
This code will trigger only if we are about crash through CHECK()

[Shrikant Kelkar, 2015-05-01 00:11:34]

On 2015/05/01 00:03:46, Will Harris wrote:
> https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
> File base/win/scoped_handle.cc (right):
> 
>
https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
> base/win/scoped_handle.cc:165: 
> I don't think you can call SetHandleInformation on all handle types for
example
> you can't on a DC, it seems?
> 
> This might break ScopedCreateDC and others...?
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935.aspx

Don't those function deal with HDC/GDI handles not HANDLE handles? Are they also
handled by ActiveVerifier?

[cpu_(ooo_6.6-7.5), 2015-05-01 00:42:52]

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:164: HANDLE_FLAG_PROTECT_FROM_CLOSE);
this can be done outside the lock.

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:165: 
On 2015/05/01 00:03:45, Will Harris wrote:
> I don't think you can call SetHandleInformation on all handle types for
example
> you can't on a DC, it seems?
> 
> This might break ScopedCreateDC and others...?
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935.aspx

Hopefully they are not using ScopedHandle because we call CloseHandle() and the
gdi/user32 handles cannot be closed this way.

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:223: return;
On 2015/04/30 23:57:16, rvargas (out of office) wrote:
> hold on... we should not be modifying the behavior of the system though this
> interception, even if it is for an experiment. All third_party code goes
through
> here.

Not sure if we want this or not, I need to think more about this, but if we do,
it needs to be outside the lock. I am not sure what Rick means by modifying the
system behavior. Isn't it the whole point of the hook to do so?


My current thinking was more along the lines of only CHECKing a fraction of the
time in 227, say 1/3rd (at random)

[Shrikant Kelkar, 2015-05-01 00:57:31]

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (left):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#o...
base/win/scoped_handle.cc:138: g_active_verifier->Disable();
On 2015/04/30 23:57:17, rvargas (out of office) wrote:
> We should not be disabling the verifier on all platforms to deal with a
> _regression_ on (the verified code on) one platform. :(

Acknowledged.

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:164: HANDLE_FLAG_PROTECT_FROM_CLOSE);
On 2015/05/01 00:42:52, cpu wrote:
> this can be done outside the lock.

Done.

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:223: return;
On 2015/05/01 00:42:51, cpu wrote:
> On 2015/04/30 23:57:16, rvargas (out of office) wrote:
> > hold on... we should not be modifying the behavior of the system though this
> > interception, even if it is for an experiment. All third_party code goes
> through
> > here.
> 
> Not sure if we want this or not, I need to think more about this, but if we
do,
> it needs to be outside the lock. I am not sure what Rick means by modifying
the
> system behavior. Isn't it the whole point of the hook to do so?
> 
> 
> My current thinking was more along the lines of only CHECKing a fraction of
the
> time in 227, say 1/3rd (at random)

Acknowledged.

[grt (UTC plus 2), 2015-05-01 13:26:14]

drive-by style comment

https://codereview.chromium.org/1113063004/diff/40001/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/40001/base/win/scoped_handle....
base/win/scoped_handle.cc:182: if (!(flags & HANDLE_FLAG_PROTECT_FROM_CLOSE))
minor style nit: check what you care about rather than testing and then
CHECK(FALSE). for example:
CHECK_NE(0U, (flags & HANDLE_FLAG_PROTECT_FROM_CLOSE));

[Shrikant Kelkar, 2015-05-01 17:48:42]

https://codereview.chromium.org/1113063004/diff/40001/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/40001/base/win/scoped_handle....
base/win/scoped_handle.cc:182: if (!(flags & HANDLE_FLAG_PROTECT_FROM_CLOSE))
On 2015/05/01 13:26:14, grt wrote:
> minor style nit: check what you care about rather than testing and then
> CHECK(FALSE). for example:
> CHECK_NE(0U, (flags & HANDLE_FLAG_PROTECT_FROM_CLOSE));

Done.

[Will Harris, 2015-05-03 21:55:19]

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:165: 
On 2015/05/01 00:42:51, cpu wrote:
> On 2015/05/01 00:03:45, Will Harris wrote:
> > I don't think you can call SetHandleInformation on all handle types for
> example
> > you can't on a DC, it seems?
> > 
> > This might break ScopedCreateDC and others...?
> > 
> > https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935.aspx
> 
> Hopefully they are not using ScopedHandle because we call CloseHandle() and
the
> gdi/user32 handles cannot be closed this way.

ScopedCreateDC is specifically using VerifierTraits which means it's opting into
handle tracking.

https://code.google.com/p/chromium/codesearch#chromium/src/base/win/scoped_hd...

[Shrikant Kelkar, 2015-05-03 23:59:56]

On 2015/05/03 21:55:19, Will Harris wrote:
> https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
> File base/win/scoped_handle.cc (right):
> 
>
https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
> base/win/scoped_handle.cc:165: 
> On 2015/05/01 00:42:51, cpu wrote:
> > On 2015/05/01 00:03:45, Will Harris wrote:
> > > I don't think you can call SetHandleInformation on all handle types for
> > example
> > > you can't on a DC, it seems?
> > > 
> > > This might break ScopedCreateDC and others...?
> > > 
> > > https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935.aspx
> > 
> > Hopefully they are not using ScopedHandle because we call CloseHandle() and
> the
> > gdi/user32 handles cannot be closed this way.
> 
> ScopedCreateDC is specifically using VerifierTraits which means it's opting
into
> handle tracking.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/win/scoped_hd...

I think you may have a point... We may be tracking unintentionally tracking HDC
as well....if true ...

[Shrikant Kelkar, 2015-05-05 22:55:40]

Removed GetHandleInfo before CHECK and separated out (already checked-in)
Verifier->Disable in different CL.

ptal.

[rvargas (doing something else), 2015-05-06 01:47:23]

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:165: 
On 2015/05/03 21:55:19, Will Harris wrote:
> On 2015/05/01 00:42:51, cpu wrote:
> > On 2015/05/01 00:03:45, Will Harris wrote:
> > > I don't think you can call SetHandleInformation on all handle types for
> > example
> > > you can't on a DC, it seems?
> > > 
> > > This might break ScopedCreateDC and others...?
> > > 
> > > https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935.aspx
> > 
> > Hopefully they are not using ScopedHandle because we call CloseHandle() and
> the
> > gdi/user32 handles cannot be closed this way.
> 
> ScopedCreateDC is specifically using VerifierTraits which means it's opting
into
> handle tracking.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/win/scoped_hd...

That's wrong, isn't it? the verifier doesn't work with anything that is not a
kernel handle (not win32k), and it may generate crashes when the actual value
overlaps.

https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
base/win/scoped_handle.cc:223: return;
On 2015/05/01 00:57:31, Shrikant Kelkar wrote:
> On 2015/05/01 00:42:51, cpu wrote:
> > On 2015/04/30 23:57:16, rvargas (out of office) wrote:
> > > hold on... we should not be modifying the behavior of the system though
this
> > > interception, even if it is for an experiment. All third_party code goes
> > through
> > > here.
> > 
> > Not sure if we want this or not, I need to think more about this, but if we
> do,
> > it needs to be outside the lock. I am not sure what Rick means by modifying
> the
> > system behavior. Isn't it the whole point of the hook to do so?
> > 
> > 
> > My current thinking was more along the lines of only CHECKing a fraction of
> the
> > time in 227, say 1/3rd (at random)
> 
> Acknowledged.

ah, right. I missed that this was after searching for the handle in the map. But
then I guess I don't get what this line tries to do: the problem right now is
that there are not enough crashes hitting line 227, and too many crashes that
apparently evade this interception and crash later on. It seems it adds more
uncertainty to the crash data.

[Will Harris, 2015-05-06 02:18:24]

On 2015/05/06 01:47:23, rvargas (out of office) wrote:
> https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc
> File base/win/scoped_handle.cc (right):
> 
>
https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
> base/win/scoped_handle.cc:165: 
> On 2015/05/03 21:55:19, Will Harris wrote:
> > On 2015/05/01 00:42:51, cpu wrote:
> > > On 2015/05/01 00:03:45, Will Harris wrote:
> > > > I don't think you can call SetHandleInformation on all handle types for
> > > example
> > > > you can't on a DC, it seems?
> > > > 
> > > > This might break ScopedCreateDC and others...?
> > > > 
> > > > https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935.aspx
> > > 
> > > Hopefully they are not using ScopedHandle because we call CloseHandle()
and
> > the
> > > gdi/user32 handles cannot be closed this way.
> > 
> > ScopedCreateDC is specifically using VerifierTraits which means it's opting
> into
> > handle tracking.
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/win/scoped_hd...
> 
> That's wrong, isn't it? the verifier doesn't work with anything that is not a
> kernel handle (not win32k), and it may generate crashes when the actual value
> overlaps.
> 

Yes! This was fixed in https://codereview.chromium.org/1121233003/ :)

>
https://codereview.chromium.org/1113063004/diff/1/base/win/scoped_handle.cc#n...
> base/win/scoped_handle.cc:223: return;
> On 2015/05/01 00:57:31, Shrikant Kelkar wrote:
> > On 2015/05/01 00:42:51, cpu wrote:
> > > On 2015/04/30 23:57:16, rvargas (out of office) wrote:
> > > > hold on... we should not be modifying the behavior of the system though
> this
> > > > interception, even if it is for an experiment. All third_party code goes
> > > through
> > > > here.
> > > 
> > > Not sure if we want this or not, I need to think more about this, but if
we
> > do,
> > > it needs to be outside the lock. I am not sure what Rick means by
modifying
> > the
> > > system behavior. Isn't it the whole point of the hook to do so?
> > > 
> > > 
> > > My current thinking was more along the lines of only CHECKing a fraction
of
> > the
> > > time in 227, say 1/3rd (at random)
> > 
> > Acknowledged.
> 
> ah, right. I missed that this was after searching for the handle in the map.
But
> then I guess I don't get what this line tries to do: the problem right now is
> that there are not enough crashes hitting line 227, and too many crashes that
> apparently evade this interception and crash later on. It seems it adds more
> uncertainty to the crash data.

[Shrikant Kelkar, 2015-05-06 18:40:42]

Added CHECK() if SetHandleInfo fails.

[cpu_(ooo_6.6-7.5), 2015-05-06 20:37:19]

lgtm

[cpu_(ooo_6.6-7.5), 2015-05-06 20:37:20]

lgtm

[cpu_(ooo_6.6-7.5), 2015-05-06 20:37:23]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2015-05-06 20:37:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1113063004/100001

[commit-bot: I haz the power, 2015-05-07 00:31:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-07 00:31:12]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[Shrikant Kelkar, 2015-05-07 21:13:46]

Reduced scope of CL to deal with only valid handles. Unit tests failed
(Especially ScopedProcessInformation) which pass on fabricated handle values.

ptal.

[cpu_(ooo_6.6-7.5), 2015-05-08 02:45:19]

https://codereview.chromium.org/1113063004/diff/120001/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/120001/base/win/scoped_handle...
base/win/scoped_handle.cc:163: if (error != ERROR_INVALID_HANDLE) {
what other error would SetHandleInformation could return?

it seems to me if we are ignoring the only probable error we might as well do
nothing

[Shrikant Kelkar, 2015-05-08 19:43:03]

On 2015/05/08 02:45:19, cpu wrote:
>
https://codereview.chromium.org/1113063004/diff/120001/base/win/scoped_handle.cc
> File base/win/scoped_handle.cc (right):
> 
>
https://codereview.chromium.org/1113063004/diff/120001/base/win/scoped_handle...
> base/win/scoped_handle.cc:163: if (error != ERROR_INVALID_HANDLE) {
> what other error would SetHandleInformation could return?
> 
> it seems to me if we are ignoring the only probable error we might as well do
> nothing

Makes sense, removed error checking.

ptal. I am looking into unit-test failures, some of them don't seem to like this
new flag :)

[cpu_(ooo_6.6-7.5), 2015-05-08 19:53:00]

ok, lgtm as is. let me know if you are changing it again.

[Shrikant Kelkar, 2015-05-08 22:07:43]

On 2015/05/08 19:53:00, cpu wrote:
> ok, lgtm as is. let me know if you are changing it again.

Found issue why unit tests were failing (local_test_server_win.cc). I have added
those changes here for the purpose of finding out all failures that happen due
to ScopedHandle changes. I will separate unrelated changes if bots succeed. 

Modified additional file: net/test/spawned_test_server/local_test_server_win.cc

ptal.

[Shrikant Kelkar, 2015-05-10 17:44:13]

Finally bots are green. Newly added both changes are related, would like to keep
them together.

ptal.

[cpu_(ooo_6.6-7.5), 2015-05-11 20:31:59]

lgtm

[cpu_(ooo_6.6-7.5), 2015-05-11 20:32:05]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2015-05-11 20:32:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1113063004/180001

[commit-bot: I haz the power, 2015-05-12 00:14:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-12 00:15:00]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Shrikant Kelkar, 2015-05-12 19:36:48]

Now both x86 and x64 bots seem to be happy. Please let me know if you want me to
separate out changes inside chrome/app/close_handle_hook_win.cc?

ptal.

[cpu_(ooo_6.6-7.5), 2015-05-12 21:23:09]

can you add a paragraph in the CL description about the change including the
last two issues?

then lgtm

[Shrikant Kelkar, 2015-05-12 21:38:59]

The CQ bit was checked by shrikant@chromium.org

[commit-bot: I haz the power, 2015-05-12 21:40:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1113063004/200001

[commit-bot: I haz the power, 2015-05-12 22:29:05]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-05-12 22:29:57]

Patchset 11 (id:??) landed as
https://crrev.com/c928d34ee861fd4102c352d9e79e1a4959a47209
Cr-Commit-Position: refs/heads/master@{#329516}

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

sebmarchand@chromium.org changed reviewers:
+ sebmarchand@chromium.org

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

One comment / potential bug (I don't know enough about Windows handles to say).

https://codereview.chromium.org/1113063004/diff/200001/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/200001/base/win/scoped_handle...
base/win/scoped_handle.cc:159: HANDLE_FLAG_PROTECT_FROM_CLOSE);
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]

https://codereview.chromium.org/1113063004/diff/200001/base/win/scoped_handle.cc
File base/win/scoped_handle.cc (right):

https://codereview.chromium.org/1113063004/diff/200001/base/win/scoped_handle...
base/win/scoped_handle.cc:159: HANDLE_FLAG_PROTECT_FROM_CLOSE);
On 2015/07/22 15:01:08, Sébastien Marchand wrote:
> 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...)
> 
> 
> 
> 

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.