1429237 - Spectre mitigation and efficiency

Status: RESOLVED INVALID

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Douglas Crosher [:dougc]]

Although some mitigations for the Spectre vulnerabilities have been deployed, namely disabling shared array buffers and reducing timer resolution, there do not appear to be any mitigations landing to address the vulnerabilities in the actually complier generated code? For example "Bug 1423225 - Disable javascript.options.shared_memory." landed on Nov 8 2017, some time ago.

Elsewhere index masking strategies are being explored and deployed, V8, Webkit - they are at least trying!

I would like to suggest one alternative strategy that does not appear to have been explored, might need feedback from Intel etc who have not responded to me personally. The idea is to create a data flow dependency on the array length so that the protected memory access can not execute until the same time that the speculative bounds check branch can execute. Tests suggests this is faster than the index masking approaches that are added to the bounds check. Please see https://weblll.org/index.php/spectre-bounds-check-data-flow-mitigation/

Btw: Bounds check elimination might be more important now. There are many issues there too. As people should know I was pushing for many years to eliminate the bounds check more efficient with cooperation from the producer using masking, and I demonstrated very competitive performance, and obviously have many ideas for that too!

If there is work in progress that is not public then could it please be opened up now because the vulnerability is public now and the community may be able to work much faster with cooperation.

Comment 1

[Lars T Hansen [:lth]]

Hi dougc, nice to hear from you again.  We're actively exploring mitigations but as you've observed there are no public bugs at this point.  Rest assured they will appear once there is concrete implementation work to be done.  Right now we're in an exploratory phase, trying to repro the attacks in a robust manner in FF and so on.  It's not just the JITs - code in the C++ parts of the runtime is also affected, and may require mitigations.  It's likely a large job.

(I'm just being pedantic here but bug 1423255 landed last week, it's just that the patch was written in November when we became aware of the issue and knew we might have to mitigate it by disabling SAB.)

I like your suggestion for the data dependency, it will be interesting to compare this to other proposed lightweight approaches on more hardware.  Clearly lfence is a method of last resort.

Bce will be increasingly important on 32-bit platforms since anything that kills speculation past the check will make the check much more costly.  (It looks like currently 1/3 of the FF user population on desktop is on 32-bit, clearly a significant fraction.  And then there's mobile.)  The BCE we have at the moment is pretty simple, and we can almost certainly do more.  With our tiered compilation system we can also afford to do more analysis for BCE within Ion.

Comment 2

[Douglas Crosher [:dougc]]

Thank you for the feedback. Fwiw the V8 team claim it is not effective in all cases see https://bugs.chromium.org/p/chromium/issues/detail?id=800503 as do webkit see https://bugs.webkit.org/show_bug.cgi?id=181453

I presume they are right, but it would be nice to see it refuted by an example that casts this proposal as hopeless, so if you happen to have an example to demonstrate that this is hopeless or feedback from Intel etc that can be shared then it would be appreciated if that could be disclosed?

If the memory access in this proposal can proceed even after the branch inputs are available, if the end of speculation can be out of order, then that might be a very significant extra point to be wary of. Note that for this proposal to fail it also requires that the subsequent memory access that leaks the value is also speculatively executed, enough to affect the cache. I presume I am missing something, this is a new area for me - hope someone can share an example.

The alternative of masking and then bounds checking, as V8 have done so far, is going to cause a big hit, as can be seen in the performance of the current V8 code. Even the wasm masking, padding to a power of two, has a hit, and the JS access hit is extreme, and it does not appear that it could be hoisted. It might need extensive changes to the way compilers and type systems work, and perhaps even to the web platform. The proposal here might help avoid this, so wouldn't be it good to settle this asap.

Here are just some concerns if this proposal is not practical. It would appear that the type system will need to track the non-speculated and the speculated types, and simplifications such as BCE would need to use only the speculated type. So the types derived from 'beta' nodes in range analysis might not be unusable for BCE - perhaps the hoisting in there that seems to depend on those types is suspect, and loops might speculatively run more times than expected so the loop analysis might no be sound, really not sure, but perhaps something to check. The BCE in EliminateBoundsChecks might also not be valid as it is based on branching not data flow (assuming Ion even had safe bounds checks). Also it appears that the speculation does not respect memory protection for Intel CPUs, so even if there were a trailing guard zone, and loop strides were analysed and bound checks only eliminated when they hit that zone, the CPU might speculate into that and perhaps beyond that zone. I worry there might be more problems for the type system in general, e.g. where a type test based on a branch can not be depended on and some inputs can be controlled by the caller. That seems a huge problem to analyse.

Comment 3

[Douglas Crosher [:dougc]]

The V8 team have provided confirmation that this is not effective and an explanation that casts it as hopeless:

"AFAWK the order in which the pending branches are verified is not specified, which can lead to an earlier, much slower, condition allowing the CPU to continue to speculate past the branch in your example."

This sounds plausible. Sorry for the noise, but perhaps it is good to have on the record so that the other solutions are understood to be necessary.