Re: [PATCH SpectreV1+L1TF v6 2/9] x86/vioapic: block speculative out-of-bound accesses

[Norbert Manthey <nmanthey@amazon.de>]

On 2/12/19 14:16, Jan Beulich wrote:
>>>> On 08.02.19 at 14:44, <nmanthey@amazon.de> wrote:
>> When interacting with io apic, a guest can specify values that are used
>> as index to structures, and whose values are not compared against
>> upper bounds to prevent speculative out-of-bound accesses. This change
>> prevents these speculative accesses.
>>
>> Furthermore, variables are initialized and the compiler is asked to not
>> optimized these initializations, as the uninitialized, potentially guest
>> controlled, variables might be used in a speculative out-of-bound access.
> Uninitialized variables can't be guest controlled, not even potentially.
> What we want to avoid here is speculation with uninitialized values
> (or really stale data still on the stack from use by other code),
> regardless of direct guest control.
I will drop the part "potentially guest controlled".
>
>> Out of the four initialized variables, two are potentially problematic,
>> namely ones in the functions vioapic_irq_positive_edge and
>> vioapic_get_trigger_mode.
>>
>> As the two problematic variables are both used in the common function
>> gsi_vioapic, the mitigation is implemented there. As the access pattern
>> of the currently non-guest-controlled functions might change in the
>> future as well, the other variables are initialized as well.
>>
>> This commit is part of the SpectreV1+L1TF mitigation patch series.
> Oh, I didn't pay attention in patch 1: You had meant to change this
> wording to something including "speculative hardening" (throughout
> the series).
That slipped through as I did not add that right after the discussion. I
added this to the whole series now.
>
>> @@ -212,7 +220,15 @@ static void vioapic_write_redirent(
>>      struct hvm_irq *hvm_irq = hvm_domain_irq(d);
>>      union vioapic_redir_entry *pent, ent;
>>      int unmasked = 0;
>> -    unsigned int gsi = vioapic->base_gsi + idx;
>> +    unsigned int gsi;
>> +
>> +    /* Callers of this function should make sure idx is bounded appropriately */
>> +    ASSERT(idx < vioapic->nr_pins);
>> +
>> +    /* Make sure no out-of-bound value for idx can be used */
> out-of-bounds

Will fix.

Best,
Norbert

>
> I'm fine now with all the code changes here.
>
> Jan
>
>



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

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