[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 4/7] xen/arm32: Add skeleton to harden branch predictor aliasing attacks
Hi Stefano,
On 25/01/18 19:37, Stefano Stabellini wrote:
On Thu, 25 Jan 2018, Julien Grall wrote:
Hi,
On 25/01/18 18:45, Stefano Stabellini wrote:
On Thu, 25 Jan 2018, Julien Grall wrote:
Hi Stefano,
On 24/01/18 23:54, Stefano Stabellini wrote:
On Fri, 19 Jan 2018, Julien Grall wrote:
Aliasing attacked against CPU branch predictors can allow an attacker
to
redirect speculative control flow on some CPUs and potentially divulge
information from one context to another.
This patch adds initiatial skeleton code behind a new Kconfig option
to enable implementation-specific mitigations against these attacks
for CPUs that are affected.
Most of mitigations will have to be applied when entering to the
hypervisor from the guest context.
Because the attack is against branch predictor, it is not possible to
safely use branch instruction before the mitigation is applied.
Therefore this has to be done in the vector entry before jump to the
helper handling a given exception.
However, on arm32, each vector contain a single instruction. This
means
that the hardened vector tables may rely on the state of registers
that
does not hold when in the hypervisor (e.g SP is 8 bytes aligned).
Therefore hypervisor code running with guest vectors table should be
minimized and always have interrupts masked to reduce the risk to use
them.
This patch provides an infrastructure to switch vector tables before
entering to the guest and when leaving it.
Note that alternative could have been used, but older Xen (4.8 or
earlier) doesn't have support. So avoid using alternative to ease
backporting.
This is part of XSA-254.
Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
I only have a couple of questions. It looks good.
---
xen/arch/arm/Kconfig | 3 +++
xen/arch/arm/arm32/entry.S | 41
++++++++++++++++++++++++++++++++++++++++-
xen/arch/arm/cpuerrata.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 06fd85cc77..2782ee6589 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -191,6 +191,9 @@ config HARDEN_BRANCH_PREDICTOR
config ARM64_HARDEN_BRANCH_PREDICTOR
def_bool y if ARM_64 && HARDEN_BRANCH_PREDICTOR
+config ARM32_HARDEN_BRANCH_PREDICTOR
+ def_bool y if ARM_32 && HARDEN_BRANCH_PREDICTOR
+
source "common/Kconfig"
source "drivers/Kconfig"
diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index c2fad5fe9b..54a1733f87 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -34,6 +34,20 @@
blne save_guest_regs
save_guest_regs:
+#ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
+ /*
+ * Restore vectors table to the default as it may have been
+ * changed when returning to the guest (see
+ * return_to_hypervisor). We need to do that early (e.g
before
+ * any interrupts are unmasked) because hardened vectors
requires
+ * SP to be 8 bytes aligned. This does not hold when running
in
+ * the hypervisor.
+ */
+ ldr r1, =hyp_traps_vector
+ mcr p15, 4, r1, c12, c0, 0
+ isb
+#endif
+
ldr r11, =0xffffffff /* Clobber SP which is only valid for
hypervisor frames. */
str r11, [sp, #UREGS_sp]
SAVE_ONE_BANKED(SP_usr)
@@ -179,12 +193,37 @@ return_to_guest:
RESTORE_ONE_BANKED(R11_fiq); RESTORE_ONE_BANKED(R12_fiq);
/* Fall thru */
return_to_hypervisor:
- cpsid i
+ cpsid ai
Why?
Asynchronous abort is a kind of interrupt, as we are going to switch to
the
hardened vector tables you don't want an interrupt to come up.
This is because the hardened vector tables requires SP to be 8 bytes
aligned.
When in the hypervisor, and particularly when restoring the register (as
below), this assumption does not hold.
So masking all interrupts (as mentioned a few time within the patch and
the
commit message) will reduce the risk to use the hardened vectors. I say
reduce
because you may have receive data abort (imagine an unlikely error in the
few
instructions to restore state).
It is also why switching vector tables are done very early in entry path
and
very late in the exit path.
All right, thanks for the explanation. Please add "and mask asynchronous
aborts" in the commit message.
I am not surely what you exactly suggest here. The commit message currently
contains:
"Therefore hypervisor code running with guest vectors table should be
minimized and always have interrupts masked to reduce the risk to use
them."
What are you suggesting?
"Therefore hypervisor code running with guest vectors table should be
minimized and always have interrupts and async aborts masked to reduce
the risk to use them."
Do you think that it is clearer?
Well, that was covered by "interrupts". If you look at the Arm Arm, A,
I, F are considered all interrupts.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
