1431829 - [Meta] Spectre mitigations for C++ reachable from JS

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Luke Wagner [:luke]]

Spectre attacks can span JS and C++, with the attacker executing JS that passes in values to C++ which performs the speculative access, returning the speculative value to the attacker's JS.

Comment 1

[Jan de Mooij [:jandem]]

So one idea I just had is to emit lfence (assuming ARM has a similar not-super-slow instruction) after all JSNative calls that don't ignore the return value (and something similar for optimized DOM getters/methods). We have to measure perf overhead but if we can get away with it, it could fix this whole class of bugs.

Comment 2

[Luke Wagner [:luke]]

+100!  Yeah, the only thing left to audit would be the much smaller set of VMFunctions called that don't go through JSNatives.

Comment 3

[Lars T Hansen [:lth]]

You probably want "DMB ISH" though we should play around with it.  The best way to fix this might be to augment the memoryBarrier() masm code, so that we keep it all in one place.  Maybe introduce MembarSpeculation = ... in jit/AtomicOp.h, then handle that flag on each platform in the best possible way.

Comment 4

[Jan de Mooij [:jandem]]

For the micro-benchmark below I get:

before:       172 ms
with lfence:  231 ms

So this is a 35% regression for a typical JSNative call. Many native calls are slower so the (relative) overhead will be smaller, some calls will be faster. At least we don't have to do this for native calls where we don't use the result.

If I do this for all native calls in Ion and Baseline, I don't see much of a difference on SunSpider/Kraken/Octane, so in practice this is probably fine...

function f() {
    var a = [1, 2];
    var t = new Date;
    for (var i = 0; i < 10000000; i++)
        a = a.reverse();
    print(new Date - t);
    return a;
}
f();

Comment 5

[Jan de Mooij [:jandem]]

I can work on a patch that puts this behind a pref.

Adding LFENCE instructions more granularly (getDenseElement, nsTArray, etc) will likely be slower and more error-prone, and doing this for all native calls will also protect other guards in C++ code, so it's probably the right trade-off.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 24 weeks.