Re: [PATCH SpectreV1+L1TF v5 3/9] x86/hvm: block speculative out-of-bound accesses

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 31/01/2019 16:19, Jan Beulich wrote:
>
>> @@ -4104,6 +4108,12 @@ static int hvmop_set_param(
>>      if ( a.index >= HVM_NR_PARAMS )
>>          return -EINVAL;
>>  
>> +    /*
>> +     * Make sure the guest controlled value a.index is bounded even during
>> +     * speculative execution.
>> +     */
>> +    a.index = array_index_nospec(a.index, HVM_NR_PARAMS);
> I'd like to come back to this model of updating local variables:
> Is this really safe to do? If such a variable lives in memory
> (which here it quite likely does), does speculation always
> recognize the update to the value? Wouldn't it rather read
> what's currently in that slot, and re-do the calculation in case
> a subsequent write happens? (I know I did suggest doing so
> earlier on, so I apologize if this results in you having to go
> back to some earlier used model.)

I'm afraid that is a very complicated set of questions to answer.

The processor needs to track write=>read dependencies to avoid wasting a
large quantity of time doing erroneous speculation, therefore it does. 
Pending writes which have happened under speculation are forwarded to
dependant instructions.

This behaviour is what gives rise to Bounds Check Bypass Store - a half
spectre-v1 gadget but with a store rather than a write.  You can e.g.
speculatively modify the return address on the stack, and hijack
speculation to an attacker controlled address for a brief period of
time.  If the speculation window is long enough, the processor first
follows the RSB/RAS (correctly), then later notices that the real value
on the stack was different, discards the speculation from the RSB/RAS
and uses the attacker controlled value instead, then eventually notices
that all of this was bogus and rewinds back to the original branch.

Another corner case is Speculative Store Bypass, where memory
disambiguation speculation can miss the fact that there is a real
write=>read dependency, and cause speculation using the older stale
value for a period of time.


As to overall safety, array_index_nospec() only works as intended when
the index remains in a register between the cmp/sbb which bounds it
under speculation, and the array access.  There is no way to guarantee
this property, as the compiler can spill any value if it thinks it needs to.

The general safety of the construct relies on the fact that an
optimising compiler will do its very best to avoid spilling variable to
the stack.  As with all of these issues, you can only confirm whether
you are no longer vulnerable by inspecting the eventual compiled code.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel