Open Bug 1431829 Opened 5 years ago Updated 4 years ago

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

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

Tracking Status
firefox60 --- affected

People

(Reporter: luke, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: meta)

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.
Depends on: 1431831
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.
+100!  Yeah, the only thing left to audit would be the much smaller set of VMFunctions called that don't go through JSNatives.
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.
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();
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.
Priority: -- → P1
Depends on: 1438886
Depends on: 1439940
Depends on: 1444473
Moving to p3 because no activity for at least 24 weeks.
Priority: P1 → P3
You need to log in before you can comment on or make changes to this bug.