Re: speculative buffer overrun in SpiderMonkey

Luis Longeri <[email protected]> Tue, 30 Jan 2018 14:30:08 -0300
Newsgroups gmane.comp.mozilla.devel.jseng
Message-ID <CAGTxne8wJ45YpYjkxe7-BySkBySjR-k+hsBLJcbtQcvnNVg=3A@mail.gmail.com>
Yes, modulus is a terrible waste of CPU resources, if there was a machine
code instruction that could do a>b?a:0 without speculative execution would
be great.
The solution in the link is nice and very clever, but it also requires 5
opcodes.
It would be nice to have an machine opcode such as in AMD's GPU like the
bitfield insert (BFI_INT) which is a 3 operand instruction that returns dst
= (src1 & src0) | (src2 & ~src0).

On Tue, Jan 30, 2018 at 5:05 AM, Steve Fink <[email protected]> wrote:

> On 1/21/18 7:30 AM, Luis Longeri wrote:
>
>> Thanks, I suspected that MOZ_ASSERT was only for DEBUG mode, but I didn't
>> check it. As an assert, I understand it should never assert in bug free JS
>> engine, so the value of the index is already checked by the time
>> MOZ_ASSERT
>> is executed in DEBUG (or skipped in production build).
>> But the code I patched, such as the function getDenseElement, is called
>> for
>> example from HasAndGetElement or GetArrayElement (in js/src/jsarray.cpp)
>> such as:
>>
>> if (index < nobj->getDenseInitializedLength()) {
>>              vp.set(nobj->getDenseElement(size_t(index)));
>>
>> That IF statement is the branching that could trigger a speculative
>> execution of the getDenseElement function if index is greater or equal to
>> the initialized length.
>>
>
>
> Yes, that looks like an example of a spectre-vulnerable computation.
>
> I don't think modulus is a good fix, though; you have to do a division,
> which I think can take a number of cycles and tie up an ALU or division
> unit. It would be better to mask, though that means calculating or
> maintaining a mask value. (And it isn't precise; the attacker could snoop
> nearby data.)
>
> It looks like the bug for this is https://bugzilla.mozilla.org/s
> how_bug.cgi?id=1430051 which has other ideas.
>
>