New issue
Advanced search Search tips

Issue 808526 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Feature

Blocking:
issue 818388



Sign in to add a comment

Investigate RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_ON

Project Member Reported by palmer@chromium.org, Feb 2 2018

Issue description

Should we use `PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_ON`? https://msdn.microsoft.com/en-us/library/windows/desktop/ms686880(v=vs.85).aspx It seems potentially handy.
 
Cc: -penny...@chromium.org
Owner: penny...@chromium.org
Status: Assigned (was: Available)
Thanks for the ping Chris.

Note: only supported on >= Win10 1709 with Jan 2018 security updates.

"This flag can be used by processes to protect against sibling hardware threads (hyperthreads) from interfering with indirect branch predictions. Processes that have sensitive information in their address space should consider enabling this flag to protect against attacks involving indirect branch prediction (such as CVE-2017-5715)."

Any assessment on whether to turn it on for all chrome processes or not?  Any rough idea on the hit to performance, and whether we care?
I'd think we should turn it on for all Chrome processes. I guess the only to test performance is to try it out and see what happens.
Status: Started (was: Assigned)
Update:
  
- CL for Windows sandbox support in review/fix stage.
https://chromium-review.googlesource.com/c/chromium/src/+/922797

- It's worth noting that along with the OS version/update requirements for this mitigation, there are of course device hardware-support requirements as well.  As of this moment, this mitigation is supported on Intel chips that have STIBP (Single Thread Indirect Branch Predictors) support.  Others would be added under the hood as appropriate.
Ref: https://newsroom.intel.com/wp-content/uploads/sites/11/2018/01/Intel-Analysis-of-Speculative-Execution-Side-Channels.pdf

- A follow-up CL to this one will enable the new mitigation for all Chromium child processes, but for the moment it cannot be enabled on browser.  This mitigation can only be enabled on creation - it cannot be enabled after.
FYI: here are some snippets from an email chain with MS on 14-15 Feb.  TL;DR: SDK(s) and public documentation should be updated in the near future!

From me:
"
...

1. Defines:
-----------

I cannot find the defines for this mitigation in any header files I can get.  Not the latest 1709 SDK, and not even the WindowsInsider SDK for build 17095.  I would expect
- at least the flag itself to be in winbase.h for use with UpdateProcThreadAttribute().
- as a bonus in winnt.h, a PROCESS_MITIGATION_POLICY enum value and a policy struct like PROCESS_MITIGATION_IMAGE_LOAD_POLICY for use with Get/SetProcessMitigationPolicy().

What version of the SDK do you expect these to be added in?

I seem to have maybe managed to implement this mitigation, after using GetProcessMitigationPolicy() with ProcessMitigationOptionsMask to dig up a mitigation bitmask which is NOT defined even in the WinInsider SDK: (0x00000001ui64 << 16) of the second mitigations DWORD64 (POLICY2).  Then I could self-define the flag and successfully pass it through to UpdateProcThreadAttribute() on patched 1709. 

Could you please confirm that  (0x00000001ui64 << 16) is the correct POLICY2 bit?


2. Implementation Details:
--------------------------

Is this mitigation meant/expected to be supported for post-startup via SetProcessMitigationPolicy(), or can it only be supported on creation?
I've seen your email thread, subject "RE: cross-process BTB attacks", so I assume the implementation detail is just that on updated Intel chips, it "will cause STIBP (Single Thread Indirect Branch Predictors) to be enabled when executing in user mode for the process."
Are you aware of equivalent updates coming for other hardware, that would silently be added under this mitigation?
If the device hardware is not Intel, or doesn't have the STIBP support, should I expect a failure and specific error code returned from UpdateProcThreadAttribute()?  Or does it succeed, and just do it's best based on HW under the hood?

3. Documentation Issues
-----------------------

Just for someone's awareness, UpdateProcThreadAttribute documentation:
(https://msdn.microsoft.com/en-us/library/windows/desktop/ms686880.aspx)

a) Aside: there are some weird copy/paste duplications happening down in the remarks section.  Big chunks repeated.

"
    PROC_THREAD_ATTRIBUTE_MITIGATION_POLICY
    - The lpValue parameter is a pointer to a DWORD or DWORD64 that specifies the exploit mitigation policy for the child process.
    - The specified policy overrides the policies set for the application and the system and cannot be changed after the child process starts running.
    - Windows Server 2008 and Windows Vista:  This value is not supported until Windows 7 and Windows Server 2008 R2.
    - The DWORD or DWORD64 pointed to by lpValue can be one or more of the values listed in the remarks.
"

b) ^^^Does this take in a two-element array now if needed (like Get/SetProcessMitigationPolicy()), since there are more than one 64-bit values for mitigations?  I can confirm that it does, through my implementation tests, but dwLength should only include the second element if it is not 0 (at least one bit is set)... otherwise the call fails.  (Not sure if that was an intended failure or an innocent "check-not-zero" from before lpValue was extended.  It probably shouldn't fail unless both elements are 0.)

"
    Windows 10, version 1709:  The following value is available only in Windows 10, version 1709 or later and only with the January 2018 Windows security updates and any applicable firmware updates from the OEM device manufacturer. See Windows Client Guidance for IT Pros to protect against speculative execution side-channel vulnerabilities.

    PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_ON
    - This flag can be used by processes to protect against sibling hardware threads (hyperthreads) from interfering with indirect branch predictions. Processes that have sensitive information in their address space should consider enabling this flag to protect against attacks involving indirect branch prediction (such as CVE-2017-5715).
"

c) ^^^no flag value documented here (unlike all the others on this page), and as I mentioned, also not in the header define even in Windows Insider SDK (17095) winbase.h.

...
"


Quick and great response from our security counterparts at MS:
"
...

Sorry for the confusion here – adding new public API surface in servicing updates is a little bit messy.

The reason this doesn’t show up in the SDK yet is a bug. We had the flag wrapped in a preprocessor that held it back from SDK inclusion to avoid premature disclosure. We’ll fix that.

In the meantime, here is how these are defined:

//
// Define the restricted indirect branch prediction mitigation policy options.
//
#define PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_MASK        (0x00000003ui64 << 16)
#define PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_DEFER       (0x00000000ui64 << 16)
#define PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_ON   (0x00000001ui64 << 16)
#define PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_OFF  (0x00000002ui64 << 16)
#define PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_RESERVED    (0x00000003ui64 << 16)

Regarding implementation details:

No, this mitigation cannot be set via SetProcessMitigationPolicy. It has to be enabled at process creation time.

This flag can be specified on hardware that does not actually implement support for this feature. It will not result in an error if specified on hardware that does not support this feature. If/when other hardware supports this capability, the flag will automatically activate for that hardware.

Thanks for the documentation feedback. We will get that fixed.

...
"
Quick update:

-Landed: https://chromium-review.googlesource.com/c/chromium/src/+/914588/
-About to land: https://chromium-review.googlesource.com/c/chromium/src/+/922797
-MS confirmed that public documentation is fixed, and SDK (defines) will be fixed in a Redstone 4 update coming down the pike.
-An update to the chromium toolchain can happen after that SDK update.
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/60930b78d18f3330fa85b7d330ef5589e12ee28c

commit 60930b78d18f3330fa85b7d330ef5589e12ee28c
Author: Penny MacNeil <pennymac@chromium.org>
Date: Fri Mar 02 00:44:58 2018

[Windows Sandbox] Add restrict_indirect_branch_prediction support.

-Add sandbox support for new process mitigation RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_ON.
-Supported on >= Win10 RS3/1709/16299, with Jan 2018 security updates, and based on
 underlying device hardware/OEM support.  E.g. Intel STIBP
-This CL also (finally) includes an update to the ConvertProcessMitigationsToPolicy() API.
 MS ran out of bits in the DWORD64 for process mitigation flags, so related APIs can now
 take in a two-element array of DWORD64s.  The second element is for "*POLICY2*" mitigation
 flags (defined in WinBase.h).

**Any downstream users of this sandbox API will need to update their code to always
pass in a two-element array now.  |size| returned will be adjusted appropriately to be
directly passed into UpdateProcThreadAttribute(), for PROC_THREAD_ATTRIBUTE_MITIGATION_POLICY.

Bug=808526
Test=sbox_integration_tests.exe

Change-Id: I9c5a0350d9b77f56a4a18be49d68fff039b11e54
Reviewed-on: https://chromium-review.googlesource.com/922797
Commit-Queue: Penny MacNeil <pennymac@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540380}
[modify] https://crrev.com/60930b78d18f3330fa85b7d330ef5589e12ee28c/sandbox/win/src/broker_services.cc
[modify] https://crrev.com/60930b78d18f3330fa85b7d330ef5589e12ee28c/sandbox/win/src/process_mitigations.cc
[modify] https://crrev.com/60930b78d18f3330fa85b7d330ef5589e12ee28c/sandbox/win/src/process_mitigations.h
[modify] https://crrev.com/60930b78d18f3330fa85b7d330ef5589e12ee28c/sandbox/win/src/process_mitigations_unittest.cc
[modify] https://crrev.com/60930b78d18f3330fa85b7d330ef5589e12ee28c/sandbox/win/src/security_level.h
[modify] https://crrev.com/60930b78d18f3330fa85b7d330ef5589e12ee28c/sandbox/win/tests/integration_tests/integration_tests_common.h

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc478ef126031896134faa8fc988d5f6cc5d87b7

commit dc478ef126031896134faa8fc988d5f6cc5d87b7
Author: Penny MacNeil <pennymac@chromium.org>
Date: Fri Mar 02 21:30:03 2018

[Windows Sandbox]  Enable some mitigations.

- Enable MITIGATION_RESTRICT_INDIRECT_BRANCH_PREDICTION and
MITIGATION_IMAGE_LOAD_PREFER_SYS32 on all child processes pre-startup.

- Enable MITIGATION_IMAGE_LOAD_* on browser process, post-startup.

IMPACT REMINDER:
----------------
- RESTRICT_INDIRECT_BRANCH_PREDICTION:
spectre variant 2 hyper-thread mitigation on >= Win10 1709 (with Jan2018 security patches),
given underlying hardware support (e.g.: Intel STIBP).
- PREFER_SYS32:
Forces image load preference to prioritize the Windows install System32
folder before dll load dir, application dir and any user dirs set.
(Affects IAT resolution standard search path only, NOT direct LoadLibrary or
executable search path.)
- NO_REMOTE:
Blocks mapping of images from remote devices.
- NO_LOW_LABEL:
Blocks mapping of images that have the low mandatory label.


Bug=808526
Test=sbox_integration_tests.exe, ProcessMitigationsTest.*

Change-Id: I88a75e1db1907e2c1f1b85f69b43ef73cf3ec5f5
Reviewed-on: https://chromium-review.googlesource.com/940528
Commit-Queue: Penny MacNeil <pennymac@chromium.org>
Reviewed-by: James Forshaw <forshaw@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540634}
[modify] https://crrev.com/dc478ef126031896134faa8fc988d5f6cc5d87b7/content/app/OWNERS
[modify] https://crrev.com/dc478ef126031896134faa8fc988d5f6cc5d87b7/content/app/sandbox_helper_win.cc
[modify] https://crrev.com/dc478ef126031896134faa8fc988d5f6cc5d87b7/services/service_manager/sandbox/win/sandbox_win.cc

Status: Fixed (was: Started)
Flipped the switch.  Hyper-thread mitigation should be on starting in 66.0.3360.0.

(*Note the limited scope of this mitigation in the CL description.)

Comment 9 by kbr@chromium.org, Mar 6 2018

Blocking: 818388
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c26118a632ab6a361b551a6763d94f11e120be41

commit c26118a632ab6a361b551a6763d94f11e120be41
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Wed Apr 11 19:05:06 2018

Tweak handling of branch prediction mitigations

PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_ON
first shows up after the 16299 SDK but we need to use it immediately.
Now that the 10.0.17133.0 SDK has shipped we can define it in such a
way that Chrome will cleanly build with or without it.

Once Chrome requires the 17133 SDK or later we can remove our local
define of it.

Bug: 808526
Change-Id: I964a75616191e98acf0d7d3c4f570de5c6d9f15a
Reviewed-on: https://chromium-review.googlesource.com/1006397
Reviewed-by: Penny MacNeil <pennymac@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549939}
[modify] https://crrev.com/c26118a632ab6a361b551a6763d94f11e120be41/sandbox/win/src/process_mitigations.cc

Labels: spectrevariant2 PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_O
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49fee825bb8421c3ffc89b4c926ef4b356af184f

commit 49fee825bb8421c3ffc89b4c926ef4b356af184f
Author: Penny MacNeil <pennymac@chromium.org>
Date: Fri Jun 29 22:27:15 2018

[Windows sandbox] Remove local define no longer needed.

- Removed local define of
PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_ON
from sandbox, now that chromium is on SDK 17134.
- A few comments I've been meaning to add to chrome_elf.

Bug: 808526
Test: sbox_integration_tests.exe
Change-Id: Ic4e381241a2d40aaa8e901a74982c1843fc2bc88
Reviewed-on: https://chromium-review.googlesource.com/1120637
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Penny MacNeil <pennymac@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571680}
[modify] https://crrev.com/49fee825bb8421c3ffc89b4c926ef4b356af184f/chrome_elf/chrome_elf_test_stubs.cc
[modify] https://crrev.com/49fee825bb8421c3ffc89b4c926ef4b356af184f/chrome_elf/elf_imports_unittest.cc
[modify] https://crrev.com/49fee825bb8421c3ffc89b4c926ef4b356af184f/sandbox/win/src/process_mitigations.cc

Labels: -Pri-2 -M-65 spectrevariant4 M-73 Pri-1
Owner: wfh@chromium.org
Status: Assigned (was: Fixed)
we should measure the real-world perf impact of this, as well as PROCESS_CREATION_MITIGATION_POLICY2_SPECULATIVE_STORE_BYPASS_DISABLE_ALWAYS_ON 
(1809 only) via a finch experiment, so I'll track that work in this bug.
Labels: -PROCESS_CREATION_MITIGATION_POLICY2_RESTRICT_INDIRECT_BRANCH_PREDICTION_ALWAYS_O
Some background on why this bug is suddenly active again: we started investigating when somebody noticed that Intel's Microcode patch (KB4100347) was slowing performance on some benchmarks on some machines running Chrome. We then used ETW/PMC data to confirm that branch mispredicts were the problem. So, we now know with a high degree of certainty that branch mispredicts cause performance problems with some benchmarks on some machines in Chrome when KB4100347 is installed.

The summary of the ETW/PMC (performance counter) results are here - you can see a dramatic increase in both the absolute count and in the percentage of branch mispredicts in the KB+sandbox case. In all cases the process in question is a renderer process, running Octane2. Counter1/2 are recording branch mispredicts and branches respectively:

ETW result  w/o KB, w/ sandbox:
                              Process name: counter1/counter2, counters
                        chrome.exe (12160):  0.77%, [489829340, 63339402188L], 39983 context switches, time: 36461230

ETW result  w/ KB, w/ sandbox:
                              Process name: counter1/counter2, counters
                        chrome.exe (20188):  1.83%, [1099490503, 60132193611L], 40642 context switches, time: 40378030

ETW result w/ KB, w/o sandbox:
                              Process name: counter1/counter2, counters
                        chrome.exe (11132):  0.77%, [511046230, 66322693908L], 41675 context switches, time: 38964695

I then used an estimate of the cost for mispredicts to confirm that the extra branch mispredicts could plausibly account for all of the slowdown. The estimate was taken from the results in this blog post:
https://randomascii.wordpress.com/2016/11/27/cpu-performance-counters-on-windows/

The math done was:
- Cost of branch mispredict in test results in blog post per unspecified "time" value = (906066 - 199941) / (90733535 - 310491) = 0.007809
- Increased cost of branch mispredicts from first to second run data above is 0.007809 * (1099490503 - 489829340) = 4760844 (calculated cost/time multipled by difference in time value)
- Increased time from first to second run data above is 40378030 - 36461230 = 3916800
- So, the predicted time increase of 4760844 is 21.5% higher than the actual increase of 3916800 which is an extremely good fit given all of the uncertainties

Allowing selective disabling of indirect branch prediction would allow us to triple-check these results, and get results from the wild.

The script used to record the performance counters on all context switch events was based on the files referenced from the above blog post which can be found here:
https://github.com/google/UIforETW/tree/master/LabScripts/ETWPMCDemo

It is not known why this regression only appears on some machines. I tested on an i5-6300U CPU which has two-cores/four-threads and therefore should, I would think, be affected, but it was not. Either there is some unknown reason for this CPU to be immune or else I ran the tests incorrectly. There have been reports from others about some machines (from a set of identical configurations) being unaffected.

Cc: kerrnel@chromium.org mnissler@chromium.org
+kerrnel, mnissler: This may be relevant to your interests.

Sign in to add a comment