[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> On 29.01.19 at 15:43, <nmanthey@xxxxxxxxx> 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, two 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. As the two problematic variables are both used in the common
> function gsi_vioapic, the mitigation is implemented there. Currently,
> the problematic callers are the functions vioapic_irq_positive_edge and
> vioapic_get_trigger_mode.

I would have wished for you to say why the other two are _not_
a problem. Afaict in both cases the functions only ever get
internal data passed.

Then again I'm not convinced it's worth taking the risk that a
problematic caller gets added down the road. How about you add
initializers everywhere, clarifying in the description that it's "just
in case" for the two currently safe ones?

> This commit is part of the SpectreV1+L1TF mitigation patch series.
> Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx>
> ---

Btw., could you please get used to the habit of adding a brief
summary of changes for at least the most recent version here,
which aids review quite a bit?

> @@ -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*/

Missing blank at the end of the comment (which, if this was the
only open point, would be easy enough to adjust while committing).


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.