[PATCH 0/3] ARM branch predictor hardening

[PATCH 0/3] ARM branch predictor hardening

[Marc Zyngier]

This small series implements some basic BP hardening by invalidating
the BTB on CPUs that are known to be susceptible to aliasing attacks.

These patches are closely modelled against what we do on arm64,
although simpler as we can rely on an architected instruction to
perform the invalidation.

The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
generalises it to be used on all affected CPUs. The second perform the
same invalidation on fatal signal delivery. The last one nukes it on
guest exit, and results in some major surgery (kudos to Dimitris
Papastamos who came up with the magic vector decoding sequence).

Note that that M-class CPUs are not affected and for R-class cores,
the mitigation doesn't make much sense since we do not enforce
user/kernel isolation.

Marc Zyngier (3):
  arm: Add BTB invalidation on switch_mm for Cortex-A9, A12, A15 and A17
  arm: Invalidate BTB on fatal signal for Cortex A8, A9, A12, A15 and
    A17
  arm: KVM: Invalidate BTB on guest exit

 arch/arm/include/asm/cp15.h  |  2 ++
 arch/arm/kvm/hyp/hyp-entry.S | 74 +++++++++++++++++++++++++++++++++++++-------
 arch/arm/mm/fault.c          | 11 +++++++
 arch/arm/mm/proc-v7-2level.S |  4 +--
 arch/arm/mm/proc-v7-3level.S |  6 ++++
 arch/arm/mm/proc-v7.S        | 32 +++++++++----------
 6 files changed, 100 insertions(+), 29 deletions(-)

-- 
2.14.2

[PATCH 1/3] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12, A15 and A17

[Marc Zyngier]

In order to avoid aliasing attacks against the branch predictor,
some implementations require to invalidate the BTB when switching
from one user context to another.

For this, we reuse the existing implementation for Cortex-A8, and
apply it to A9, A12, A15, and A17.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mm/proc-v7-2level.S |  4 ++--
 arch/arm/mm/proc-v7-3level.S |  6 ++++++
 arch/arm/mm/proc-v7.S        | 32 ++++++++++++++++----------------
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index c6141a5435c3..0422e58b74e8 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -41,7 +41,7 @@
  *	even on Cortex-A8 revisions not affected by 430973.
  *	If IBE is not set, the flush BTAC/BTB won't do anything.
  */
-ENTRY(cpu_ca8_switch_mm)
+ENTRY(cpu_v7_btbinv_switch_mm)
 #ifdef CONFIG_MMU
 	mov	r2, #0
 	mcr	p15, 0, r2, c7, c5, 6		@ flush BTAC/BTB
@@ -66,7 +66,7 @@ ENTRY(cpu_v7_switch_mm)
 #endif
 	bx	lr
 ENDPROC(cpu_v7_switch_mm)
-ENDPROC(cpu_ca8_switch_mm)
+ENDPROC(cpu_v7_btbinv_switch_mm)
 
 /*
  *	cpu_v7_set_pte_ext(ptep, pte)
diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index 7d16bbc4102b..934272e1fa08 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -54,6 +54,11 @@
  * Set the translation table base pointer to be pgd_phys (physical address of
  * the new TTB).
  */
+ENTRY(cpu_v7_btbinv_switch_mm)
+#ifdef CONFIG_MMU
+	mov	r2, #0
+	mcr	p15, 0, r2, c7, c5, 6			@ flush BTAC/BTB
+#endif
 ENTRY(cpu_v7_switch_mm)
 #ifdef CONFIG_MMU
 	mmid	r2, r2
@@ -64,6 +69,7 @@ ENTRY(cpu_v7_switch_mm)
 #endif
 	ret	lr
 ENDPROC(cpu_v7_switch_mm)
+ENDPROC(cpu_v7_btbinv_switch_mm)
 
 #ifdef __ARMEB__
 #define rl r3
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 01d64c0b2563..23fa537101ab 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -159,18 +159,18 @@ ENDPROC(cpu_v7_do_resume)
 #endif
 
 /*
- * Cortex-A8
+ * Cortex-A8/A12/A15/A17 that require a BTB invalidation on switch_mm
  */
-	globl_equ	cpu_ca8_proc_init,	cpu_v7_proc_init
-	globl_equ	cpu_ca8_proc_fin,	cpu_v7_proc_fin
-	globl_equ	cpu_ca8_reset,		cpu_v7_reset
-	globl_equ	cpu_ca8_do_idle,	cpu_v7_do_idle
-	globl_equ	cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area
-	globl_equ	cpu_ca8_set_pte_ext,	cpu_v7_set_pte_ext
-	globl_equ	cpu_ca8_suspend_size,	cpu_v7_suspend_size
+	globl_equ	cpu_v7_btbinv_proc_init,	cpu_v7_proc_init
+	globl_equ	cpu_v7_btbinv_proc_fin,		cpu_v7_proc_fin
+	globl_equ	cpu_v7_btbinv_reset,		cpu_v7_reset
+	globl_equ	cpu_v7_btbinv_do_idle,		cpu_v7_do_idle
+	globl_equ	cpu_v7_btbinv_dcache_clean_area, cpu_v7_dcache_clean_area
+	globl_equ	cpu_v7_btbinv_set_pte_ext,	cpu_v7_set_pte_ext
+	globl_equ	cpu_v7_btbinv_suspend_size,	cpu_v7_suspend_size
 #ifdef CONFIG_ARM_CPU_SUSPEND
-	globl_equ	cpu_ca8_do_suspend,	cpu_v7_do_suspend
-	globl_equ	cpu_ca8_do_resume,	cpu_v7_do_resume
+	globl_equ	cpu_v7_btbinv_do_suspend,	cpu_v7_do_suspend
+	globl_equ	cpu_v7_btbinv_do_resume,	cpu_v7_do_resume
 #endif
 
 /*
@@ -181,7 +181,7 @@ ENDPROC(cpu_v7_do_resume)
 	globl_equ	cpu_ca9mp_reset,	cpu_v7_reset
 	globl_equ	cpu_ca9mp_do_idle,	cpu_v7_do_idle
 	globl_equ	cpu_ca9mp_dcache_clean_area, cpu_v7_dcache_clean_area
-	globl_equ	cpu_ca9mp_switch_mm,	cpu_v7_switch_mm
+	globl_equ	cpu_ca9mp_switch_mm,	cpu_v7_btbinv_switch_mm
 	globl_equ	cpu_ca9mp_set_pte_ext,	cpu_v7_set_pte_ext
 .globl	cpu_ca9mp_suspend_size
 .equ	cpu_ca9mp_suspend_size, cpu_v7_suspend_size + 4 * 2
@@ -548,8 +548,8 @@ __v7_setup_stack:
 
 	@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
 	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+	define_processor_functions v7_btbinv, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 #ifndef CONFIG_ARM_LPAE
-	define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 	define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 #endif
 #ifdef CONFIG_CPU_PJ4B
@@ -614,7 +614,7 @@ __v7_ca9mp_proc_info:
 __v7_ca8_proc_info:
 	.long	0x410fc080
 	.long	0xff0ffff0
-	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
+	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = v7_btbinv_processor_functions
 	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
 
 #endif	/* CONFIG_ARM_LPAE */
@@ -658,7 +658,7 @@ __v7_ca7mp_proc_info:
 __v7_ca12mp_proc_info:
 	.long	0x410fc0d0
 	.long	0xff0ffff0
-	__v7_proc __v7_ca12mp_proc_info, __v7_ca12mp_setup
+	__v7_proc __v7_ca12mp_proc_info, __v7_ca12mp_setup, proc_fns = v7_btbinv_processor_functions
 	.size	__v7_ca12mp_proc_info, . - __v7_ca12mp_proc_info
 
 	/*
@@ -668,7 +668,7 @@ __v7_ca12mp_proc_info:
 __v7_ca15mp_proc_info:
 	.long	0x410fc0f0
 	.long	0xff0ffff0
-	__v7_proc __v7_ca15mp_proc_info, __v7_ca15mp_setup
+	__v7_proc __v7_ca15mp_proc_info, __v7_ca15mp_setup, proc_fns = v7_btbinv_processor_functions
 	.size	__v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
 
 	/*
@@ -688,7 +688,7 @@ __v7_b15mp_proc_info:
 __v7_ca17mp_proc_info:
 	.long	0x410fc0e0
 	.long	0xff0ffff0
-	__v7_proc __v7_ca17mp_proc_info, __v7_ca17mp_setup
+	__v7_proc __v7_ca17mp_proc_info, __v7_ca17mp_setup, proc_fns = v7_btbinv_processor_functions
 	.size	__v7_ca17mp_proc_info, . - __v7_ca17mp_proc_info
 
 	/*
-- 
2.14.2

[PATCH 2/3] arm: Invalidate BTB on fatal signal for Cortex A8, A9, A12, A15 and A17

[Marc Zyngier]

In order to prevent aliasing attacks on the branch predictor,
invalidate the BTB on CPUs that are known to be affected.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/cp15.h |  2 ++
 arch/arm/mm/fault.c         | 11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
index 4c9fa72b59f5..9e900ae855aa 100644
--- a/arch/arm/include/asm/cp15.h
+++ b/arch/arm/include/asm/cp15.h
@@ -65,6 +65,8 @@
 #define __write_sysreg(v, r, w, c, t)	asm volatile(w " " c : : "r" ((t)(v)))
 #define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
 
+#define BPIALL				__ACCESS_CP15(c7, 0, c5, 6)
+
 extern unsigned long cr_alignment;	/* defined in entry-armv.S */
 
 static inline unsigned long get_cr(void)
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 42f585379e19..62da5f99eda0 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -21,6 +21,7 @@
 #include <linux/highmem.h>
 #include <linux/perf_event.h>
 
+#include <asm/cp15.h>
 #include <asm/exception.h>
 #include <asm/pgtable.h>
 #include <asm/system_misc.h>
@@ -181,6 +182,16 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 	si.si_errno = 0;
 	si.si_code = code;
 	si.si_addr = (void __user *)addr;
+
+	switch(read_cpuid_part()) {
+	case ARM_CPU_PART_CORTEX_A8:
+	case ARM_CPU_PART_CORTEX_A9:
+	case ARM_CPU_PART_CORTEX_A12:
+	case ARM_CPU_PART_CORTEX_A15:
+	case ARM_CPU_PART_CORTEX_A17:
+		write_sysreg(0, BPIALL);
+		break;
+	}
 	force_sig_info(sig, &si, tsk);
 }
 
-- 
2.14.2

[PATCH 3/3] arm: KVM: Invalidate BTB on guest exit

[Marc Zyngier]

In order to avoid aliasing attacks against the branch predictor,
let's invalidate the BTB on guest exit. This is made complicated
by the fact that we cannot take a branch before invalidating the
BTB.

Another thing is that we perform the invalidation on all
implementations, no matter if they are affected or not.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/hyp/hyp-entry.S | 74 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 63 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
index 95a2faefc070..aa8adfa64ec9 100644
--- a/arch/arm/kvm/hyp/hyp-entry.S
+++ b/arch/arm/kvm/hyp/hyp-entry.S
@@ -61,15 +61,60 @@
 __kvm_hyp_vector:
 	.global __kvm_hyp_vector
 
-	@ Hyp-mode exception vector
-	W(b)	hyp_reset
-	W(b)	hyp_undef
-	W(b)	hyp_svc
-	W(b)	hyp_pabt
-	W(b)	hyp_dabt
-	W(b)	hyp_hvc
-	W(b)	hyp_irq
-	W(b)	hyp_fiq
+	/*
+	 * We encode the exception entry in the bottom 3 bits of
+	 * SP, and we have to guarantee to be 8 bytes aligned.
+	 */
+	add	sp, sp, #1	/* Reset 	  7 */
+	add	sp, sp, #1	/* Undef	  6 */
+	add	sp, sp, #1	/* Syscall	  5 */
+	add	sp, sp, #1	/* Prefetch abort 4 */
+	add	sp, sp, #1	/* Data abort	  3 */
+	add	sp, sp, #1	/* HVC		  2 */
+	add	sp, sp, #1	/* IRQ		  1 */
+	add	sp, sp, #1	/* FIQ		  0 */
+
+	sub	sp, sp, #1
+
+	mcr	p15, 0, r0, c7, c5, 6	/* BPIALL */
+	isb
+
+	/*
+	 * As we cannot use any temporary registers and cannot
+	 * clobber SP, we can decode the exception entry using
+	 * an unrolled binary search.
+	 */
+	tst	sp, #4
+	bne	1f
+
+	tst	sp, #2
+	bne	3f
+
+	tst	sp, #1
+	bic	sp, sp, #0x7
+	bne	hyp_irq
+	b	hyp_irq
+
+1:
+	tst	sp, #2
+	bne	2f
+
+	tst	sp, #1
+	bic	sp, sp, #0x7
+	bne	hyp_svc
+	b	hyp_pabt
+
+2:
+	tst	sp, #1
+	bic	sp, sp, #0x7
+	bne	hyp_reset
+	b	hyp_undef
+
+3:
+	tst	sp, #1
+	bic	sp, sp, #0x7
+	bne	hyp_dabt
+	b	hyp_hvc
 
 .macro invalid_vector label, cause
 	.align
@@ -149,7 +194,14 @@ hyp_hvc:
 	bx	ip
 
 1:
-	push	{lr}
+	/*
+	 * Pushing r2 here is just a way of keeping the stack aligned to
+	 * 8 bytes on any path that can trigger a HYP exception. Here,
+	 * we may well be about to jump into the guest, and the guest
+	 * exit would otherwise be badly decoded by our fancy
+	 * "decode-exception-without-a-branch" code...
+	 */
+	push	{r2, lr}
 
 	mov	lr, r0
 	mov	r0, r1
@@ -159,7 +211,7 @@ hyp_hvc:
 THUMB(	orr	lr, #1)
 	blx	lr			@ Call the HYP function
 
-	pop	{lr}
+	pop	{r2, lr}
 	eret
 
 guest_trap:
-- 
2.14.2

[PATCH 3/3] arm: KVM: Invalidate BTB on guest exit

[Ard Biesheuvel]

On 6 January 2018 at 12:09, Marc Zyngier <marc.zyngier@arm.com> wrote:
> In order to avoid aliasing attacks against the branch predictor,
> let's invalidate the BTB on guest exit. This is made complicated
> by the fact that we cannot take a branch before invalidating the
> BTB.
>
> Another thing is that we perform the invalidation on all
> implementations, no matter if they are affected or not.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kvm/hyp/hyp-entry.S | 74 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 63 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> index 95a2faefc070..aa8adfa64ec9 100644
> --- a/arch/arm/kvm/hyp/hyp-entry.S
> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> @@ -61,15 +61,60 @@
>  __kvm_hyp_vector:
>         .global __kvm_hyp_vector
>
> -       @ Hyp-mode exception vector
> -       W(b)    hyp_reset
> -       W(b)    hyp_undef
> -       W(b)    hyp_svc
> -       W(b)    hyp_pabt
> -       W(b)    hyp_dabt
> -       W(b)    hyp_hvc
> -       W(b)    hyp_irq
> -       W(b)    hyp_fiq
> +       /*
> +        * We encode the exception entry in the bottom 3 bits of
> +        * SP, and we have to guarantee to be 8 bytes aligned.
> +        */
> +       add     sp, sp, #1      /* Reset          7 */
> +       add     sp, sp, #1      /* Undef          6 */
> +       add     sp, sp, #1      /* Syscall        5 */
> +       add     sp, sp, #1      /* Prefetch abort 4 */
> +       add     sp, sp, #1      /* Data abort     3 */
> +       add     sp, sp, #1      /* HVC            2 */
> +       add     sp, sp, #1      /* IRQ            1 */
> +       add     sp, sp, #1      /* FIQ            0 */
> +
> +       sub     sp, sp, #1
> +
> +       mcr     p15, 0, r0, c7, c5, 6   /* BPIALL */
> +       isb
> +
> +       /*
> +        * As we cannot use any temporary registers and cannot
> +        * clobber SP, we can decode the exception entry using
> +        * an unrolled binary search.
> +        */
> +       tst     sp, #4
> +       bne     1f
> +
> +       tst     sp, #2
> +       bne     3f
> +
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_irq
> +       b       hyp_irq

hyp_fiq ?

> +
> +1:
> +       tst     sp, #2
> +       bne     2f
> +
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_svc
> +       b       hyp_pabt
> +
> +2:
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_reset
> +       b       hyp_undef
> +
> +3:
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_dabt
> +       b       hyp_hvc
>
>  .macro invalid_vector label, cause
>         .align
> @@ -149,7 +194,14 @@ hyp_hvc:
>         bx      ip
>
>  1:
> -       push    {lr}
> +       /*
> +        * Pushing r2 here is just a way of keeping the stack aligned to
> +        * 8 bytes on any path that can trigger a HYP exception. Here,
> +        * we may well be about to jump into the guest, and the guest
> +        * exit would otherwise be badly decoded by our fancy
> +        * "decode-exception-without-a-branch" code...
> +        */
> +       push    {r2, lr}
>
>         mov     lr, r0
>         mov     r0, r1
> @@ -159,7 +211,7 @@ hyp_hvc:
>  THUMB( orr     lr, #1)
>         blx     lr                      @ Call the HYP function
>
> -       pop     {lr}
> +       pop     {r2, lr}
>         eret
>
>  guest_trap:
> --
> 2.14.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm: KVM: Invalidate BTB on guest exit

[Ard Biesheuvel]

On 6 January 2018 at 12:09, Marc Zyngier <marc.zyngier@arm.com> wrote:
> In order to avoid aliasing attacks against the branch predictor,
> let's invalidate the BTB on guest exit. This is made complicated
> by the fact that we cannot take a branch before invalidating the
> BTB.
>

You can't even take an unconditional branch?

> Another thing is that we perform the invalidation on all
> implementations, no matter if they are affected or not.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kvm/hyp/hyp-entry.S | 74 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 63 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> index 95a2faefc070..aa8adfa64ec9 100644
> --- a/arch/arm/kvm/hyp/hyp-entry.S
> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> @@ -61,15 +61,60 @@
>  __kvm_hyp_vector:
>         .global __kvm_hyp_vector
>
> -       @ Hyp-mode exception vector
> -       W(b)    hyp_reset
> -       W(b)    hyp_undef
> -       W(b)    hyp_svc
> -       W(b)    hyp_pabt
> -       W(b)    hyp_dabt
> -       W(b)    hyp_hvc
> -       W(b)    hyp_irq
> -       W(b)    hyp_fiq
> +       /*
> +        * We encode the exception entry in the bottom 3 bits of
> +        * SP, and we have to guarantee to be 8 bytes aligned.
> +        */
> +       add     sp, sp, #1      /* Reset          7 */
> +       add     sp, sp, #1      /* Undef          6 */
> +       add     sp, sp, #1      /* Syscall        5 */
> +       add     sp, sp, #1      /* Prefetch abort 4 */
> +       add     sp, sp, #1      /* Data abort     3 */
> +       add     sp, sp, #1      /* HVC            2 */
> +       add     sp, sp, #1      /* IRQ            1 */
> +       add     sp, sp, #1      /* FIQ            0 */
> +
> +       sub     sp, sp, #1
> +
> +       mcr     p15, 0, r0, c7, c5, 6   /* BPIALL */
> +       isb
> +
> +       /*
> +        * As we cannot use any temporary registers and cannot
> +        * clobber SP, we can decode the exception entry using
> +        * an unrolled binary search.
> +        */
> +       tst     sp, #4
> +       bne     1f
> +
> +       tst     sp, #2
> +       bne     3f
> +
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_irq
> +       b       hyp_irq
> +
> +1:
> +       tst     sp, #2
> +       bne     2f
> +
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_svc
> +       b       hyp_pabt
> +
> +2:
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_reset
> +       b       hyp_undef
> +
> +3:
> +       tst     sp, #1
> +       bic     sp, sp, #0x7
> +       bne     hyp_dabt
> +       b       hyp_hvc
>
>  .macro invalid_vector label, cause
>         .align
> @@ -149,7 +194,14 @@ hyp_hvc:
>         bx      ip
>
>  1:
> -       push    {lr}
> +       /*
> +        * Pushing r2 here is just a way of keeping the stack aligned to
> +        * 8 bytes on any path that can trigger a HYP exception. Here,
> +        * we may well be about to jump into the guest, and the guest
> +        * exit would otherwise be badly decoded by our fancy
> +        * "decode-exception-without-a-branch" code...
> +        */
> +       push    {r2, lr}
>
>         mov     lr, r0
>         mov     r0, r1
> @@ -159,7 +211,7 @@ hyp_hvc:
>  THUMB( orr     lr, #1)
>         blx     lr                      @ Call the HYP function
>
> -       pop     {lr}
> +       pop     {r2, lr}
>         eret
>
>  guest_trap:
> --
> 2.14.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm: KVM: Invalidate BTB on guest exit

[Marc Zyngier]

On Sat, 6 Jan 2018 13:27:13 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 6 January 2018 at 12:09, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > In order to avoid aliasing attacks against the branch predictor,
> > let's invalidate the BTB on guest exit. This is made complicated
> > by the fact that we cannot take a branch before invalidating the
> > BTB.
> >
> > Another thing is that we perform the invalidation on all
> > implementations, no matter if they are affected or not.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/kvm/hyp/hyp-entry.S | 74 +++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 63 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> > index 95a2faefc070..aa8adfa64ec9 100644
> > --- a/arch/arm/kvm/hyp/hyp-entry.S
> > +++ b/arch/arm/kvm/hyp/hyp-entry.S
> > @@ -61,15 +61,60 @@
> >  __kvm_hyp_vector:
> >         .global __kvm_hyp_vector
> >
> > -       @ Hyp-mode exception vector
> > -       W(b)    hyp_reset
> > -       W(b)    hyp_undef
> > -       W(b)    hyp_svc
> > -       W(b)    hyp_pabt
> > -       W(b)    hyp_dabt
> > -       W(b)    hyp_hvc
> > -       W(b)    hyp_irq
> > -       W(b)    hyp_fiq
> > +       /*
> > +        * We encode the exception entry in the bottom 3 bits of
> > +        * SP, and we have to guarantee to be 8 bytes aligned.
> > +        */
> > +       add     sp, sp, #1      /* Reset          7 */
> > +       add     sp, sp, #1      /* Undef          6 */
> > +       add     sp, sp, #1      /* Syscall        5 */
> > +       add     sp, sp, #1      /* Prefetch abort 4 */
> > +       add     sp, sp, #1      /* Data abort     3 */
> > +       add     sp, sp, #1      /* HVC            2 */
> > +       add     sp, sp, #1      /* IRQ            1 */
> > +       add     sp, sp, #1      /* FIQ            0 */
> > +
> > +       sub     sp, sp, #1
> > +
> > +       mcr     p15, 0, r0, c7, c5, 6   /* BPIALL */
> > +       isb
> > +
> > +       /*
> > +        * As we cannot use any temporary registers and cannot
> > +        * clobber SP, we can decode the exception entry using
> > +        * an unrolled binary search.
> > +        */
> > +       tst     sp, #4
> > +       bne     1f
> > +
> > +       tst     sp, #2
> > +       bne     3f
> > +
> > +       tst     sp, #1
> > +       bic     sp, sp, #0x7
> > +       bne     hyp_irq
> > +       b       hyp_irq  
> 
> hyp_fiq ?

Indeed. Thanks for spotting that one!

	M.
-- 
Without deviation from the norm, progress is not possible.

[PATCH 3/3] arm: KVM: Invalidate BTB on guest exit

[Marc Zyngier]

On Sat, 6 Jan 2018 13:35:46 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 6 January 2018 at 12:09, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > In order to avoid aliasing attacks against the branch predictor,
> > let's invalidate the BTB on guest exit. This is made complicated
> > by the fact that we cannot take a branch before invalidating the
> > BTB.
> >  
> 
> You can't even take an unconditional branch?

Yup.  We certainly have that restriction on arm64, and I assume it is
the same on 32bit.

	M.
-- 
Without deviation from the norm, progress is not possible.

[PATCH 1/2] ARM: Add BTB invalidation on switch_mm for Brahma-B15

[Florian Fainelli]

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/include/asm/cputype.h | 4 ++++
 arch/arm/mm/proc-v7.S          | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index cb546425da8a..48bf5639214a 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -62,6 +62,7 @@
 	((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
 
 #define ARM_CPU_IMP_ARM			0x41
+#define ARM_CPU_IMP_BRCM		0x42
 #define ARM_CPU_IMP_DEC			0x44
 #define ARM_CPU_IMP_INTEL		0x69
 
@@ -82,6 +83,9 @@
 /* DEC implemented cores */
 #define ARM_CPU_PART_SA1100		0x4400a110
 
+/* Broadcom implemented cores */
+#define ARM_CPU_PART_BRAHMA_B15		0x420000f0
+
 /* Intel implemented cores */
 #define ARM_CPU_PART_SA1110		0x6900b110
 #define ARM_CPU_REV_SA1110_A0		0
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 47c1ae4e3041..0cf3e497f988 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -159,7 +159,7 @@ ENDPROC(cpu_v7_do_resume)
 #endif
 
 /*
- * Cortex-A8/A12/A15/A17 that require a BTB invalidation on switch_mm
+ * Cortex-A8/A12/A15/A17, Brahma-B15 that require a BTB invalidation on switch_mm
  */
 	globl_equ	cpu_v7_btbinv_proc_init,	cpu_v7_proc_init
 	globl_equ	cpu_v7_btbinv_proc_fin,		cpu_v7_proc_fin
@@ -678,7 +678,7 @@ __v7_ca15mp_proc_info:
 __v7_b15mp_proc_info:
 	.long	0x420f00f0
 	.long	0xff0ffff0
-	__v7_proc __v7_b15mp_proc_info, __v7_b15mp_setup, cache_fns = b15_cache_fns
+	__v7_proc __v7_b15mp_proc_info, __v7_b15mp_setup, cache_fns = b15_cache_fns, proc_fns = v7_btbinv_processor_functions
 	.size	__v7_b15mp_proc_info, . - __v7_b15mp_proc_info
 
 	/*
-- 
2.14.1

[PATCH 2/2] ARM: Invalidate BTB on fatal signal for Brahma-B15

[Florian Fainelli]

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/mm/fault.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 1a1c2638f528..fd840a7483aa 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -188,6 +188,7 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 	case ARM_CPU_PART_CORTEX_A12:
 	case ARM_CPU_PART_CORTEX_A15:
 	case ARM_CPU_PART_CORTEX_A17:
+	case ARM_CPU_PART_BRAHMA_B15:
 		write_sysreg(0, BPIALL);
 		break;
 	}
-- 
2.14.1

[PATCH 0/3] ARM branch predictor hardening

[Florian Fainelli]

Hi Marc,

Le 01/06/18 ? 04:09, Marc Zyngier a ?crit?:
> This small series implements some basic BP hardening by invalidating
> the BTB on CPUs that are known to be susceptible to aliasing attacks.
> 
> These patches are closely modelled against what we do on arm64,
> although simpler as we can rely on an architected instruction to
> perform the invalidation.
> 
> The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
> generalises it to be used on all affected CPUs. The second perform the
> same invalidation on fatal signal delivery. The last one nukes it on
> guest exit, and results in some major surgery (kudos to Dimitris
> Papastamos who came up with the magic vector decoding sequence).
> 
> Note that that M-class CPUs are not affected and for R-class cores,
> the mitigation doesn't make much sense since we do not enforce
> user/kernel isolation.

Broadcom's Brahma-B15 CPUs are also affected, I can either send an
incremental patch on top of this series once it lands in, or since it
looks like you are going to respin a v2, feel free to incorporate the
changes I sent as replies to patch 1 and 2.

What about P4JB and Krait, should they also be covered?

Even though I am assuming -stable maintainers will quickly pick those
changes, should there be an explicit mention of CVE-2017-5715?


Thanks!

> 
> Marc Zyngier (3):
>   arm: Add BTB invalidation on switch_mm for Cortex-A9, A12, A15 and A17
>   arm: Invalidate BTB on fatal signal for Cortex A8, A9, A12, A15 and
>     A17
>   arm: KVM: Invalidate BTB on guest exit
> 
>  arch/arm/include/asm/cp15.h  |  2 ++
>  arch/arm/kvm/hyp/hyp-entry.S | 74 +++++++++++++++++++++++++++++++++++++-------
>  arch/arm/mm/fault.c          | 11 +++++++
>  arch/arm/mm/proc-v7-2level.S |  4 +--
>  arch/arm/mm/proc-v7-3level.S |  6 ++++
>  arch/arm/mm/proc-v7.S        | 32 +++++++++----------
>  6 files changed, 100 insertions(+), 29 deletions(-)
> 


-- 
Florian

[PATCH 1/3] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12, A15 and A17

[Andre Przywara]

Hi,

On 06/01/18 12:09, Marc Zyngier wrote:
> In order to avoid aliasing attacks against the branch predictor,
> some implementations require to invalidate the BTB when switching
> from one user context to another.
> 
> For this, we reuse the existing implementation for Cortex-A8, and
> apply it to A9, A12, A15, and A17.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/mm/proc-v7-2level.S |  4 ++--
>  arch/arm/mm/proc-v7-3level.S |  6 ++++++
>  arch/arm/mm/proc-v7.S        | 32 ++++++++++++++++----------------
>  3 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
> index c6141a5435c3..0422e58b74e8 100644
> --- a/arch/arm/mm/proc-v7-2level.S
> +++ b/arch/arm/mm/proc-v7-2level.S
> @@ -41,7 +41,7 @@
>   *	even on Cortex-A8 revisions not affected by 430973.
>   *	If IBE is not set, the flush BTAC/BTB won't do anything.
>   */
> -ENTRY(cpu_ca8_switch_mm)
> +ENTRY(cpu_v7_btbinv_switch_mm)
>  #ifdef CONFIG_MMU
>  	mov	r2, #0
>  	mcr	p15, 0, r2, c7, c5, 6		@ flush BTAC/BTB
> @@ -66,7 +66,7 @@ ENTRY(cpu_v7_switch_mm)
>  #endif
>  	bx	lr
>  ENDPROC(cpu_v7_switch_mm)
> -ENDPROC(cpu_ca8_switch_mm)
> +ENDPROC(cpu_v7_btbinv_switch_mm)
>  
>  /*
>   *	cpu_v7_set_pte_ext(ptep, pte)
> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> index 7d16bbc4102b..934272e1fa08 100644
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -54,6 +54,11 @@
>   * Set the translation table base pointer to be pgd_phys (physical address of
>   * the new TTB).
>   */
> +ENTRY(cpu_v7_btbinv_switch_mm)
> +#ifdef CONFIG_MMU
> +	mov	r2, #0
> +	mcr	p15, 0, r2, c7, c5, 6			@ flush BTAC/BTB

r2 is already used below: this is the second argument to the function.
The first argument is a phys_addr_t, so it takes both r0 and r1 already.
That's why we need to use r3 above.
This fixes a crash on the Midway for me.

Cheers,
Andre.

> +#endif
>  ENTRY(cpu_v7_switch_mm)
>  #ifdef CONFIG_MMU
>  	mmid	r2, r2
> @@ -64,6 +69,7 @@ ENTRY(cpu_v7_switch_mm)
>  #endif
>  	ret	lr
>  ENDPROC(cpu_v7_switch_mm)
> +ENDPROC(cpu_v7_btbinv_switch_mm)
>  
>  #ifdef __ARMEB__
>  #define rl r3
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 01d64c0b2563..23fa537101ab 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -159,18 +159,18 @@ ENDPROC(cpu_v7_do_resume)
>  #endif
>  
>  /*
> - * Cortex-A8
> + * Cortex-A8/A12/A15/A17 that require a BTB invalidation on switch_mm
>   */
> -	globl_equ	cpu_ca8_proc_init,	cpu_v7_proc_init
> -	globl_equ	cpu_ca8_proc_fin,	cpu_v7_proc_fin
> -	globl_equ	cpu_ca8_reset,		cpu_v7_reset
> -	globl_equ	cpu_ca8_do_idle,	cpu_v7_do_idle
> -	globl_equ	cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area
> -	globl_equ	cpu_ca8_set_pte_ext,	cpu_v7_set_pte_ext
> -	globl_equ	cpu_ca8_suspend_size,	cpu_v7_suspend_size
> +	globl_equ	cpu_v7_btbinv_proc_init,	cpu_v7_proc_init
> +	globl_equ	cpu_v7_btbinv_proc_fin,		cpu_v7_proc_fin
> +	globl_equ	cpu_v7_btbinv_reset,		cpu_v7_reset
> +	globl_equ	cpu_v7_btbinv_do_idle,		cpu_v7_do_idle
> +	globl_equ	cpu_v7_btbinv_dcache_clean_area, cpu_v7_dcache_clean_area
> +	globl_equ	cpu_v7_btbinv_set_pte_ext,	cpu_v7_set_pte_ext
> +	globl_equ	cpu_v7_btbinv_suspend_size,	cpu_v7_suspend_size
>  #ifdef CONFIG_ARM_CPU_SUSPEND
> -	globl_equ	cpu_ca8_do_suspend,	cpu_v7_do_suspend
> -	globl_equ	cpu_ca8_do_resume,	cpu_v7_do_resume
> +	globl_equ	cpu_v7_btbinv_do_suspend,	cpu_v7_do_suspend
> +	globl_equ	cpu_v7_btbinv_do_resume,	cpu_v7_do_resume
>  #endif
>  
>  /*
> @@ -181,7 +181,7 @@ ENDPROC(cpu_v7_do_resume)
>  	globl_equ	cpu_ca9mp_reset,	cpu_v7_reset
>  	globl_equ	cpu_ca9mp_do_idle,	cpu_v7_do_idle
>  	globl_equ	cpu_ca9mp_dcache_clean_area, cpu_v7_dcache_clean_area
> -	globl_equ	cpu_ca9mp_switch_mm,	cpu_v7_switch_mm
> +	globl_equ	cpu_ca9mp_switch_mm,	cpu_v7_btbinv_switch_mm
>  	globl_equ	cpu_ca9mp_set_pte_ext,	cpu_v7_set_pte_ext
>  .globl	cpu_ca9mp_suspend_size
>  .equ	cpu_ca9mp_suspend_size, cpu_v7_suspend_size + 4 * 2
> @@ -548,8 +548,8 @@ __v7_setup_stack:
>  
>  	@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
>  	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
> +	define_processor_functions v7_btbinv, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
>  #ifndef CONFIG_ARM_LPAE
> -	define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
>  	define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
>  #endif
>  #ifdef CONFIG_CPU_PJ4B
> @@ -614,7 +614,7 @@ __v7_ca9mp_proc_info:
>  __v7_ca8_proc_info:
>  	.long	0x410fc080
>  	.long	0xff0ffff0
> -	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = v7_btbinv_processor_functions
>  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
>  
>  #endif	/* CONFIG_ARM_LPAE */
> @@ -658,7 +658,7 @@ __v7_ca7mp_proc_info:
>  __v7_ca12mp_proc_info:
>  	.long	0x410fc0d0
>  	.long	0xff0ffff0
> -	__v7_proc __v7_ca12mp_proc_info, __v7_ca12mp_setup
> +	__v7_proc __v7_ca12mp_proc_info, __v7_ca12mp_setup, proc_fns = v7_btbinv_processor_functions
>  	.size	__v7_ca12mp_proc_info, . - __v7_ca12mp_proc_info
>  
>  	/*
> @@ -668,7 +668,7 @@ __v7_ca12mp_proc_info:
>  __v7_ca15mp_proc_info:
>  	.long	0x410fc0f0
>  	.long	0xff0ffff0
> -	__v7_proc __v7_ca15mp_proc_info, __v7_ca15mp_setup
> +	__v7_proc __v7_ca15mp_proc_info, __v7_ca15mp_setup, proc_fns = v7_btbinv_processor_functions
>  	.size	__v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
>  
>  	/*
> @@ -688,7 +688,7 @@ __v7_b15mp_proc_info:
>  __v7_ca17mp_proc_info:
>  	.long	0x410fc0e0
>  	.long	0xff0ffff0
> -	__v7_proc __v7_ca17mp_proc_info, __v7_ca17mp_setup
> +	__v7_proc __v7_ca17mp_proc_info, __v7_ca17mp_setup, proc_fns = v7_btbinv_processor_functions
>  	.size	__v7_ca17mp_proc_info, . - __v7_ca17mp_proc_info
>  
>  	/*
> 

[PATCH 1/3] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12, A15 and A17

[Marc Zyngier]

On 08/01/18 14:29, Andre Przywara wrote:
> Hi,
> 
> On 06/01/18 12:09, Marc Zyngier wrote:
>> In order to avoid aliasing attacks against the branch predictor,
>> some implementations require to invalidate the BTB when switching
>> from one user context to another.
>>
>> For this, we reuse the existing implementation for Cortex-A8, and
>> apply it to A9, A12, A15, and A17.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/mm/proc-v7-2level.S |  4 ++--
>>  arch/arm/mm/proc-v7-3level.S |  6 ++++++
>>  arch/arm/mm/proc-v7.S        | 32 ++++++++++++++++----------------
>>  3 files changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
>> index c6141a5435c3..0422e58b74e8 100644
>> --- a/arch/arm/mm/proc-v7-2level.S
>> +++ b/arch/arm/mm/proc-v7-2level.S
>> @@ -41,7 +41,7 @@
>>   *	even on Cortex-A8 revisions not affected by 430973.
>>   *	If IBE is not set, the flush BTAC/BTB won't do anything.
>>   */
>> -ENTRY(cpu_ca8_switch_mm)
>> +ENTRY(cpu_v7_btbinv_switch_mm)
>>  #ifdef CONFIG_MMU
>>  	mov	r2, #0
>>  	mcr	p15, 0, r2, c7, c5, 6		@ flush BTAC/BTB
>> @@ -66,7 +66,7 @@ ENTRY(cpu_v7_switch_mm)
>>  #endif
>>  	bx	lr
>>  ENDPROC(cpu_v7_switch_mm)
>> -ENDPROC(cpu_ca8_switch_mm)
>> +ENDPROC(cpu_v7_btbinv_switch_mm)
>>  
>>  /*
>>   *	cpu_v7_set_pte_ext(ptep, pte)
>> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>> index 7d16bbc4102b..934272e1fa08 100644
>> --- a/arch/arm/mm/proc-v7-3level.S
>> +++ b/arch/arm/mm/proc-v7-3level.S
>> @@ -54,6 +54,11 @@
>>   * Set the translation table base pointer to be pgd_phys (physical address of
>>   * the new TTB).
>>   */
>> +ENTRY(cpu_v7_btbinv_switch_mm)
>> +#ifdef CONFIG_MMU
>> +	mov	r2, #0
>> +	mcr	p15, 0, r2, c7, c5, 6			@ flush BTAC/BTB
> 
> r2 is already used below: this is the second argument to the function.
> The first argument is a phys_addr_t, so it takes both r0 and r1 already.
> That's why we need to use r3 above.
> This fixes a crash on the Midway for me.

I suppose you have DRAM above 4GB. Lucky you! I'll fix that in v2.

Thanks for the heads up.

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 0/3] ARM branch predictor hardening

[Tony Lindgren]

* Marc Zyngier <marc.zyngier@arm.com> [180106 04:14]:
> This small series implements some basic BP hardening by invalidating
> the BTB on CPUs that are known to be susceptible to aliasing attacks.
> 
> These patches are closely modelled against what we do on arm64,
> although simpler as we can rely on an architected instruction to
> perform the invalidation.
> 
> The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
> generalises it to be used on all affected CPUs. The second perform the
> same invalidation on fatal signal delivery. The last one nukes it on
> guest exit, and results in some major surgery (kudos to Dimitris
> Papastamos who came up with the magic vector decoding sequence).

So if a Cortex-A8 has bootloder set the IBE bit, and kernel has
ARM_ERRATA_430973 enabled, is Cortex-A8 already hardened then?

Regards,

Tony

[PATCH 0/3] ARM branch predictor hardening

[Marc Zyngier]

Hi Tony,

On 08/01/18 16:54, Tony Lindgren wrote:
> * Marc Zyngier <marc.zyngier@arm.com> [180106 04:14]:
>> This small series implements some basic BP hardening by invalidating
>> the BTB on CPUs that are known to be susceptible to aliasing attacks.
>>
>> These patches are closely modelled against what we do on arm64,
>> although simpler as we can rely on an architected instruction to
>> perform the invalidation.
>>
>> The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
>> generalises it to be used on all affected CPUs. The second perform the
>> same invalidation on fatal signal delivery. The last one nukes it on
>> guest exit, and results in some major surgery (kudos to Dimitris
>> Papastamos who came up with the magic vector decoding sequence).
> 
> So if a Cortex-A8 has bootloder set the IBE bit, and kernel has
> ARM_ERRATA_430973 enabled, is Cortex-A8 already hardened then?

Almost. See the extra BTB invalidation in fault.c.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 0/3] ARM branch predictor hardening

[Tony Lindgren]

* Marc Zyngier <marc.zyngier@arm.com> [180108 17:05]:
> Hi Tony,
> 
> On 08/01/18 16:54, Tony Lindgren wrote:
> > * Marc Zyngier <marc.zyngier@arm.com> [180106 04:14]:
> >> This small series implements some basic BP hardening by invalidating
> >> the BTB on CPUs that are known to be susceptible to aliasing attacks.
> >>
> >> These patches are closely modelled against what we do on arm64,
> >> although simpler as we can rely on an architected instruction to
> >> perform the invalidation.
> >>
> >> The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
> >> generalises it to be used on all affected CPUs. The second perform the
> >> same invalidation on fatal signal delivery. The last one nukes it on
> >> guest exit, and results in some major surgery (kudos to Dimitris
> >> Papastamos who came up with the magic vector decoding sequence).
> > 
> > So if a Cortex-A8 has bootloder set the IBE bit, and kernel has
> > ARM_ERRATA_430973 enabled, is Cortex-A8 already hardened then?
> 
> Almost. See the extra BTB invalidation in fault.c.

OK thanks. So for Cortex-A8, only patch "[PATCH 2/3] arm: Invalidate
BTB on fatal signal for Cortex A8, A9, A12, A15 and A17" is needed
if I read that right.

Regards,

Tony

[PATCH 0/3] ARM branch predictor hardening

[Marc Zyngier]

On 08/01/18 17:24, Tony Lindgren wrote:
> * Marc Zyngier <marc.zyngier@arm.com> [180108 17:05]:
>> Hi Tony,
>>
>> On 08/01/18 16:54, Tony Lindgren wrote:
>>> * Marc Zyngier <marc.zyngier@arm.com> [180106 04:14]:
>>>> This small series implements some basic BP hardening by invalidating
>>>> the BTB on CPUs that are known to be susceptible to aliasing attacks.
>>>>
>>>> These patches are closely modelled against what we do on arm64,
>>>> although simpler as we can rely on an architected instruction to
>>>> perform the invalidation.
>>>>
>>>> The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
>>>> generalises it to be used on all affected CPUs. The second perform the
>>>> same invalidation on fatal signal delivery. The last one nukes it on
>>>> guest exit, and results in some major surgery (kudos to Dimitris
>>>> Papastamos who came up with the magic vector decoding sequence).
>>>
>>> So if a Cortex-A8 has bootloder set the IBE bit, and kernel has
>>> ARM_ERRATA_430973 enabled, is Cortex-A8 already hardened then?
>>
>> Almost. See the extra BTB invalidation in fault.c.
> 
> OK thanks. So for Cortex-A8, only patch "[PATCH 2/3] arm: Invalidate
> BTB on fatal signal for Cortex A8, A9, A12, A15 and A17" is needed
> if I read that right.

That's the idea. Although I'm about to change that patch as it's been
pointed out to me that I'm doing that in a preemptible context, which
makes it a bit dodgy.

I should have something to post later today, once I'm done with fixing
the Thumb2 build (which completely breaks on patch #3)...

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 1/3] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12, A15 and A17

[Robin Murphy]

Hi Marc,

On 08/01/18 14:46, Marc Zyngier wrote:
> On 08/01/18 14:29, Andre Przywara wrote:
>> Hi,
>>
>> On 06/01/18 12:09, Marc Zyngier wrote:
>>> In order to avoid aliasing attacks against the branch predictor,
>>> some implementations require to invalidate the BTB when switching
>>> from one user context to another.
>>>
>>> For this, we reuse the existing implementation for Cortex-A8, and
>>> apply it to A9, A12, A15, and A17.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>   arch/arm/mm/proc-v7-2level.S |  4 ++--
>>>   arch/arm/mm/proc-v7-3level.S |  6 ++++++
>>>   arch/arm/mm/proc-v7.S        | 32 ++++++++++++++++----------------
>>>   3 files changed, 24 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
>>> index c6141a5435c3..0422e58b74e8 100644
>>> --- a/arch/arm/mm/proc-v7-2level.S
>>> +++ b/arch/arm/mm/proc-v7-2level.S
>>> @@ -41,7 +41,7 @@
>>>    *	even on Cortex-A8 revisions not affected by 430973.
>>>    *	If IBE is not set, the flush BTAC/BTB won't do anything.
>>>    */
>>> -ENTRY(cpu_ca8_switch_mm)
>>> +ENTRY(cpu_v7_btbinv_switch_mm)
>>>   #ifdef CONFIG_MMU
>>>   	mov	r2, #0
>>>   	mcr	p15, 0, r2, c7, c5, 6		@ flush BTAC/BTB
>>> @@ -66,7 +66,7 @@ ENTRY(cpu_v7_switch_mm)
>>>   #endif
>>>   	bx	lr
>>>   ENDPROC(cpu_v7_switch_mm)
>>> -ENDPROC(cpu_ca8_switch_mm)
>>> +ENDPROC(cpu_v7_btbinv_switch_mm)
>>>   
>>>   /*
>>>    *	cpu_v7_set_pte_ext(ptep, pte)
>>> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>> index 7d16bbc4102b..934272e1fa08 100644
>>> --- a/arch/arm/mm/proc-v7-3level.S
>>> +++ b/arch/arm/mm/proc-v7-3level.S
>>> @@ -54,6 +54,11 @@
>>>    * Set the translation table base pointer to be pgd_phys (physical address of
>>>    * the new TTB).
>>>    */
>>> +ENTRY(cpu_v7_btbinv_switch_mm)
>>> +#ifdef CONFIG_MMU
>>> +	mov	r2, #0
>>> +	mcr	p15, 0, r2, c7, c5, 6			@ flush BTAC/BTB
>>
>> r2 is already used below: this is the second argument to the function.
>> The first argument is a phys_addr_t, so it takes both r0 and r1 already.
>> That's why we need to use r3 above.
>> This fixes a crash on the Midway for me.
> 
> I suppose you have DRAM above 4GB. Lucky you! I'll fix that in v2.

Why even zero a register at all? Since BPIALL ignores the value of Rt 
(DDI0406C.c, p1744) it shouldn't matter which register you use nor what 
it contains.

Robin.

[PATCH 0/3] ARM branch predictor hardening

[Marc Zyngier]

On Sat, 06 Jan 2018 18:50:41 +0000,
Florian Fainelli wrote:

Hi Florian,

> Le 01/06/18 ? 04:09, Marc Zyngier a ?crit?:
> > This small series implements some basic BP hardening by invalidating
> > the BTB on CPUs that are known to be susceptible to aliasing attacks.
> > 
> > These patches are closely modelled against what we do on arm64,
> > although simpler as we can rely on an architected instruction to
> > perform the invalidation.
> > 
> > The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
> > generalises it to be used on all affected CPUs. The second perform the
> > same invalidation on fatal signal delivery. The last one nukes it on
> > guest exit, and results in some major surgery (kudos to Dimitris
> > Papastamos who came up with the magic vector decoding sequence).
> > 
> > Note that that M-class CPUs are not affected and for R-class cores,
> > the mitigation doesn't make much sense since we do not enforce
> > user/kernel isolation.
> 
> Broadcom's Brahma-B15 CPUs are also affected, I can either send an
> incremental patch on top of this series once it lands in, or since it
> looks like you are going to respin a v2, feel free to incorporate the
> changes I sent as replies to patch 1 and 2.

I've re-spun the series, as there was quite a few issues with the
first one. Could you please try and respin your B15 patches on top?

> What about P4JB and Krait, should they also be covered?

I have no idea. I only know of the ARM cores. Other implementation
will have to check whether they are affected or not.

> Even though I am assuming -stable maintainers will quickly pick
> those changes, should there be an explicit mention of CVE-2017-5715?

I have no plans for these patches to be merged immediately. We're
targeting the arm64 patches at v4.16, and I don't expect the 32bit
patches to be any different.

As for the CVE mention, I'm not really bothered (yet another number
soup). Everybody knows what we're talking about, these days...

Thanks,

	M.

[PATCH 0/3] ARM branch predictor hardening

[Russell King - ARM Linux]

On Mon, Jan 08, 2018 at 09:28:28PM +0000, Marc Zyngier wrote:
> On Sat, 06 Jan 2018 18:50:41 +0000,
> Florian Fainelli wrote:
> 
> Hi Florian,
> 
> > Le 01/06/18 ? 04:09, Marc Zyngier a ?crit?:
> > > This small series implements some basic BP hardening by invalidating
> > > the BTB on CPUs that are known to be susceptible to aliasing attacks.
> > > 
> > > These patches are closely modelled against what we do on arm64,
> > > although simpler as we can rely on an architected instruction to
> > > perform the invalidation.
> > > 
> > > The first patch reuses the Cortex-A8 BTB invalidation in switch_mm and
> > > generalises it to be used on all affected CPUs. The second perform the
> > > same invalidation on fatal signal delivery. The last one nukes it on
> > > guest exit, and results in some major surgery (kudos to Dimitris
> > > Papastamos who came up with the magic vector decoding sequence).
> > > 
> > > Note that that M-class CPUs are not affected and for R-class cores,
> > > the mitigation doesn't make much sense since we do not enforce
> > > user/kernel isolation.
> > 
> > Broadcom's Brahma-B15 CPUs are also affected, I can either send an
> > incremental patch on top of this series once it lands in, or since it
> > looks like you are going to respin a v2, feel free to incorporate the
> > changes I sent as replies to patch 1 and 2.
> 
> I've re-spun the series, as there was quite a few issues with the
> first one. Could you please try and respin your B15 patches on top?
> 
> > What about P4JB and Krait, should they also be covered?
> 
> I have no idea. I only know of the ARM cores. Other implementation
> will have to check whether they are affected or not.
> 
> > Even though I am assuming -stable maintainers will quickly pick
> > those changes, should there be an explicit mention of CVE-2017-5715?
> 
> I have no plans for these patches to be merged immediately. We're
> targeting the arm64 patches at v4.16, and I don't expect the 32bit
> patches to be any different.
> 
> As for the CVE mention, I'm not really bothered (yet another number
> soup). Everybody knows what we're talking about, these days...

That is untrue.  I've been trying to talk to you guys about it, and
I'm getting very little back - you haven't replied to the email I sent
privately last week for example, and so far I've not seen any response
to my email yesterday.

So for the past week, I've been researching the information and trying
stuff out.  I've my own conclusions, reached entirely independently,
and so far I see nothing to suggest that this patch series does anything
to solve any of the published issues.

Given that, I'm not currently considering merging these patches until
it becomes clearer what they are trying to address.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up