1432207 - [Meta] Spectre mitigations for Value type checks

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

The plan so far:

* 64-bit platforms:

Instead of unboxing as follows: ValueBits & JSVAL_PAYLOAD_MASK, we will have a type specific mask that has the high bits set to the type tag. Unboxing a string becomes ValueBits ^ string-specific-mask. This is nice because it means when you pass in an int32, object, or any other type, you'll get a JSString* that has some high bits set, so dereferencing it is guaranteed to give you an invalid address.

I'm excited about this scheme, because it's cheap (no extra overhead!) and it has other nice security properties: doing val.toObject() on something that's not an object, will now always crash and that could help us catch other bugs.

The main difficulty here is passing through the JSValueType to various MacroAssembler methods. I hacked this up locally and it doesn't seem too difficult.

* 32-bit platforms

We're not sure yet what we will do here. Maybe XOR GC things with a random value, but there might be other ideas. For now I'm focusing on 64-bit.

Comment 1

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> doing val.toObject() on something
> that's not an object, will now always crash and that could help us catch
> other bugs.

More correct: it will crash when you dereference this value.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> The main difficulty here is passing through the JSValueType to various
> MacroAssembler methods.

There are also some places where we call masm.unboxObject on string values etc. That kind of "sloppiness" will now result in crashes and it's a Good Thing; I just have to track them down.

Comment 3

[Steve Fink [:sfink] [:s:]]

I hit this in a rebase of my nursery strings patch. Sadly, this does introduce a small amount of overhead for me, since I have a check for whether a Value is a nursery cell (so it could be either an object or a string). Oh well. I've been Spectrified or something.

The overhead is pretty close to zero for my current scheme anyway, other than a tiny bit of code size, since I'm already doing separate type tag tests for object and string (it's just that previously they both ended up in the same code.)

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Steve Fink [:sfink] [:s:] from comment #3)
> The overhead is pretty close to zero for my current scheme anyway, other
> than a tiny bit of code size, since I'm already doing separate type tag
> tests for object and string (it's just that previously they both ended up in
> the same code.)

You may be able to get away with something like this: https://searchfox.org/mozilla-central/rev/97cb0aa64ae51adcabff76fb3b5eb18368f5f8ab/js/src/jit/MacroAssembler.cpp#3161

Assuming it's Spectre-safe (it depends on your code).

Comment 5

[Jason Orendorff [:jorendorff]]

This is such a great idea.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

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