Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 04/11] x86/hvm: block speculative out-of-bound accesses

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/01/2019 11:51, Norbert Manthey wrote:
> There are multiple arrays in the HVM interface that are accessed
> with indices that are provided by the guest. To avoid speculative
> out-of-bound accesses, we use the array_index_nospec macro.
>
> When blocking speculative out-of-bound accesses, we can classify arrays
> into dynamic arrays and static arrays. Where the former are allocated
> during run time, the size of the latter is known during compile time.
> On static arrays, compiler might be able to block speculative accesses
> in the future.
>
> We introduce another macro that uses the ARRAY_SIZE macro to block
> speculative accesses. For arrays that are statically accessed, this macro
> can be used instead of the usual macro. Using this macro results in more
> readable code, and allows to modify the way this case is handled in a
> single place.
>
> This commit is part of the SpectreV1+L1TF mitigation patch series.
>
> Reported-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
> Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx>
>
> ---
>  xen/arch/x86/hvm/hvm.c   | 27 ++++++++++++++++++++++-----
>  xen/include/xen/nospec.h |  6 ++++++
>  2 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -37,6 +37,7 @@
>  #include <xen/monitor.h>
>  #include <xen/warning.h>
>  #include <xen/vpci.h>
> +#include <xen/nospec.h>
>  #include <asm/shadow.h>
>  #include <asm/hap.h>
>  #include <asm/current.h>
> @@ -2102,7 +2103,7 @@ int hvm_mov_from_cr(unsigned int cr, unsigned int gpr)
>      case 2:
>      case 3:
>      case 4:
> -        val = curr->arch.hvm.guest_cr[cr];
> +        val = array_access_nospec(curr->arch.hvm.guest_cr, cr);

This is an interesting case - we don't actually need protection here.

This path is called exclusively from intercepts, so cr is strictly one
of 0, 2, 3, 4, 8 even under adversarial speculation.  However, as
guest_cr[] is only 5 entries long, the 8 case can still result in an OoB
read.

However, given that the 8 index is in the hw_cr[] array and guaranteed
to be in the cache by this point, an attacker can't gain any additional
information by poisoning the switch logic.

(And furthermore, almost all hardware in the past decade has TPR
shadowing support, at which point we don't even hit the cr8 intercept.)

~Andrew

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