All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] ARM branch predictor hardening
@ 2018-01-08 18:55 Marc Zyngier
  2018-01-08 18:55 ` [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17 Marc Zyngier
                   ` (8 more replies)
  0 siblings, 9 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-08 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

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 notable exception is Cortex-A15, where
BTB invalidation behaves like a NOP, and the only way to shoot the
predictor down is to invalidate the icache *and* to have ACTLR[0] set
to 1 (which is a secure-only operation).

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 prefetch abort outside of the userspace
range. The third one nukes it on guest exit, and results in some major
surgery as we cannot take a branch from the vectors (that, and Thumb2
being a massive pain).

Patches 4 to 6 are doing a similar thing for Cortex-A15, which the
aforementioned ICIALLU.

Note 1: Contrary to the initial version, this new series relies on
the arm64/kpti branch (I reuse the per-CPU vector hook for KVM).

Note 2: 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.

* From v1:
  - Fixed broken hyp_fiq vector (noticed by Ard)
  - Fixed broken BTB invalidation in LPAE switch_mm (reported by Andre)
  - Revamped invalidation on PABT (noticed by James on arm64,
    suggested by Will)
  - Rewrote the whole HYP sequence, as Thumb2 was pretty unhappy about
    arithmetic with the stack pointer

Marc Zyngier (6):
  arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  arm: Invalidate BTB on prefetch abort outside of user mapping on
    Cortex A8, A9, A12 and A17
  arm: KVM: Invalidate BTB on guest exit
  arm: Add icache invalidation on switch_mm for Cortex-A15
  arm: Invalidate icache on prefetch abort outside of user mapping on
    Cortex-A15
  arm: KVM: Invalidate icache on guest exit for Cortex-A15

 arch/arm/include/asm/cp15.h    |  3 ++
 arch/arm/include/asm/kvm_asm.h |  2 -
 arch/arm/include/asm/kvm_mmu.h | 17 +++++++-
 arch/arm/kvm/hyp/hyp-entry.S   | 89 +++++++++++++++++++++++++++++++++++++++++-
 arch/arm/mm/fault.c            | 23 +++++++++++
 arch/arm/mm/fsr-2level.c       |  4 +-
 arch/arm/mm/fsr-3level.c       | 67 ++++++++++++++++++++++++++++++-
 arch/arm/mm/proc-v7-2level.S   | 14 ++++++-
 arch/arm/mm/proc-v7-3level.S   | 22 +++++++++++
 arch/arm/mm/proc-v7.S          | 48 +++++++++++++++--------
 10 files changed, 263 insertions(+), 26 deletions(-)

-- 
2.14.2

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
@ 2018-01-08 18:55 ` Marc Zyngier
  2018-01-09 14:14   ` Andre Przywara
  2018-01-10 17:53   ` Tony Lindgren
  2018-01-08 18:55 ` [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, " Marc Zyngier
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-08 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

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 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        | 30 +++++++++++++++---------------
 3 files changed, 23 insertions(+), 17 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..f6adfe88ead2 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	r3, #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..0a14967fd400 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/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
 
 	/*
@@ -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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, A12 and A17
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
  2018-01-08 18:55 ` [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17 Marc Zyngier
@ 2018-01-08 18:55 ` Marc Zyngier
  2018-01-09  9:56   ` Marc Zyngier
  2018-01-10 16:45   ` Russell King - ARM Linux
  2018-01-08 18:55 ` [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit Marc Zyngier
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-08 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

In order to prevent aliasing attacks on the branch predictor,
invalidate the BTB on CPUs that are known to be affected when taking
a prefetch abort on a address that is outside of a user task limit.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/cp15.h |  2 ++
 arch/arm/mm/fault.c         | 19 +++++++++++++
 arch/arm/mm/fsr-2level.c    |  4 +--
 arch/arm/mm/fsr-3level.c    | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 89 insertions(+), 3 deletions(-)

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..ff272ffcf741 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,7 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 	si.si_errno = 0;
 	si.si_code = code;
 	si.si_addr = (void __user *)addr;
+
 	force_sig_info(sig, &si, tsk);
 }
 
@@ -404,6 +406,23 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 }
 #endif					/* CONFIG_MMU */
 
+static int
+do_pabt_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
+{
+	if (addr > TASK_SIZE) {
+		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_A17:
+			write_sysreg(0, BPIALL);
+			break;
+		}
+	}
+
+	return do_page_fault(addr, fsr, regs);
+}
+
 /*
  * First Level Translation Fault Handler
  *
diff --git a/arch/arm/mm/fsr-2level.c b/arch/arm/mm/fsr-2level.c
index f2be95197265..ca07f72d8624 100644
--- a/arch/arm/mm/fsr-2level.c
+++ b/arch/arm/mm/fsr-2level.c
@@ -51,7 +51,7 @@ static struct fsr_info ifsr_info[] = {
 	{ do_bad,		SIGBUS,  0,		"unknown 4"			   },
 	{ do_translation_fault,	SIGSEGV, SEGV_MAPERR,	"section translation fault"	   },
 	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"page access flag fault"	   },
-	{ do_page_fault,	SIGSEGV, SEGV_MAPERR,	"page translation fault"	   },
+	{ do_pabt_page_fault,	SIGSEGV, SEGV_MAPERR,	"page translation fault"	   },
 	{ do_bad,		SIGBUS,	 0,		"external abort on non-linefetch"  },
 	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"section domain fault"		   },
 	{ do_bad,		SIGBUS,  0,		"unknown 10"			   },
@@ -59,7 +59,7 @@ static struct fsr_info ifsr_info[] = {
 	{ do_bad,		SIGBUS,	 0,		"external abort on translation"	   },
 	{ do_sect_fault,	SIGSEGV, SEGV_ACCERR,	"section permission fault"	   },
 	{ do_bad,		SIGBUS,	 0,		"external abort on translation"	   },
-	{ do_page_fault,	SIGSEGV, SEGV_ACCERR,	"page permission fault"		   },
+	{ do_pabt_page_fault,	SIGSEGV, SEGV_ACCERR,	"page permission fault"		   },
 	{ do_bad,		SIGBUS,  0,		"unknown 16"			   },
 	{ do_bad,		SIGBUS,  0,		"unknown 17"			   },
 	{ do_bad,		SIGBUS,  0,		"unknown 18"			   },
diff --git a/arch/arm/mm/fsr-3level.c b/arch/arm/mm/fsr-3level.c
index d0ae2963656a..88cfc7d06a30 100644
--- a/arch/arm/mm/fsr-3level.c
+++ b/arch/arm/mm/fsr-3level.c
@@ -66,4 +66,69 @@ static struct fsr_info fsr_info[] = {
 	{ do_bad,		SIGBUS,  0,		"unknown 63"			},
 };
 
-#define ifsr_info	fsr_info
+static struct fsr_info ifsr_info[] = {
+	{ do_bad,		SIGBUS,  0,		"unknown 0"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 1"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 2"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 3"			},
+	{ do_bad,		SIGBUS,  0,		"reserved translation fault"	},
+	{ do_translation_fault,	SIGSEGV, SEGV_MAPERR,	"level 1 translation fault"	},
+	{ do_translation_fault,	SIGSEGV, SEGV_MAPERR,	"level 2 translation fault"	},
+	{ do_pabt_page_fault,	SIGSEGV, SEGV_MAPERR,	"level 3 translation fault"	},
+	{ do_bad,		SIGBUS,  0,		"reserved access flag fault"	},
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"level 1 access flag fault"	},
+	{ do_pabt_page_fault,	SIGSEGV, SEGV_ACCERR,	"level 2 access flag fault"	},
+	{ do_pabt_page_fault,	SIGSEGV, SEGV_ACCERR,	"level 3 access flag fault"	},
+	{ do_bad,		SIGBUS,  0,		"reserved permission fault"	},
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"level 1 permission fault"	},
+	{ do_pabt_page_fault,	SIGSEGV, SEGV_ACCERR,	"level 2 permission fault"	},
+	{ do_pabt_page_fault,	SIGSEGV, SEGV_ACCERR,	"level 3 permission fault"	},
+	{ do_bad,		SIGBUS,  0,		"synchronous external abort"	},
+	{ do_bad,		SIGBUS,  0,		"asynchronous external abort"	},
+	{ do_bad,		SIGBUS,  0,		"unknown 18"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 19"			},
+	{ do_bad,		SIGBUS,  0,		"synchronous abort (translation table walk)" },
+	{ do_bad,		SIGBUS,  0,		"synchronous abort (translation table walk)" },
+	{ do_bad,		SIGBUS,  0,		"synchronous abort (translation table walk)" },
+	{ do_bad,		SIGBUS,  0,		"synchronous abort (translation table walk)" },
+	{ do_bad,		SIGBUS,  0,		"synchronous parity error"	},
+	{ do_bad,		SIGBUS,  0,		"asynchronous parity error"	},
+	{ do_bad,		SIGBUS,  0,		"unknown 26"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 27"			},
+	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk" },
+	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk" },
+	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk" },
+	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk" },
+	{ do_bad,		SIGBUS,  0,		"unknown 32"			},
+	{ do_bad,		SIGBUS,  BUS_ADRALN,	"alignment fault"		},
+	{ do_bad,		SIGBUS,  0,		"debug event"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 35"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 36"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 37"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 38"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 39"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 40"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 41"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 42"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 43"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 44"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 45"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 46"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 47"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 48"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 49"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 50"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 51"			},
+	{ do_bad,		SIGBUS,  0,		"implementation fault (lockdown abort)" },
+	{ do_bad,		SIGBUS,  0,		"unknown 53"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 54"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 55"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 56"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 57"			},
+	{ do_bad,		SIGBUS,  0,		"implementation fault (coprocessor abort)" },
+	{ do_bad,		SIGBUS,  0,		"unknown 59"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 60"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 61"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 62"			},
+	{ do_bad,		SIGBUS,  0,		"unknown 63"			},
+};
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
  2018-01-08 18:55 ` [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17 Marc Zyngier
  2018-01-08 18:55 ` [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, " Marc Zyngier
@ 2018-01-08 18:55 ` Marc Zyngier
  2018-01-15 12:22   ` Robin Murphy
  2018-01-23 14:22   ` Christoffer Dall
  2018-01-08 18:55 ` [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15 Marc Zyngier
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-08 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

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/include/asm/kvm_asm.h |  2 --
 arch/arm/include/asm/kvm_mmu.h | 13 ++++++++-
 arch/arm/kvm/hyp/hyp-entry.S   | 64 ++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
index 36dd2962a42d..df24ed48977d 100644
--- a/arch/arm/include/asm/kvm_asm.h
+++ b/arch/arm/include/asm/kvm_asm.h
@@ -61,8 +61,6 @@ struct kvm_vcpu;
 extern char __kvm_hyp_init[];
 extern char __kvm_hyp_init_end[];
 
-extern char __kvm_hyp_vector[];
-
 extern void __kvm_flush_vm_context(void);
 extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index eb46fc81a440..b47db5b9e407 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -37,6 +37,7 @@
 
 #include <linux/highmem.h>
 #include <asm/cacheflush.h>
+#include <asm/cputype.h>
 #include <asm/pgalloc.h>
 #include <asm/stage2_pgtable.h>
 
@@ -223,7 +224,17 @@ static inline unsigned int kvm_get_vmid_bits(void)
 
 static inline void *kvm_get_hyp_vector(void)
 {
-	return kvm_ksym_ref(__kvm_hyp_vector);
+	extern char __kvm_hyp_vector[];
+	extern char __kvm_hyp_vector_bp_inv[];
+
+	switch(read_cpuid_part()) {
+	case ARM_CPU_PART_CORTEX_A12:
+	case ARM_CPU_PART_CORTEX_A17:
+		return kvm_ksym_ref(__kvm_hyp_vector_bp_inv);
+
+	default:
+		return kvm_ksym_ref(__kvm_hyp_vector);
+	}
 }
 
 static inline int kvm_map_vectors(void)
diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
index 95a2faefc070..2e8d2179eb70 100644
--- a/arch/arm/kvm/hyp/hyp-entry.S
+++ b/arch/arm/kvm/hyp/hyp-entry.S
@@ -70,6 +70,59 @@ __kvm_hyp_vector:
 	W(b)	hyp_hvc
 	W(b)	hyp_irq
 	W(b)	hyp_fiq
+	
+	.align 5
+__kvm_hyp_vector_bp_inv:
+	.global __kvm_hyp_vector_bp_inv
+
+	/*
+	 * We encode the exception entry in the bottom 3 bits of
+	 * SP, and we have to guarantee to be 8 bytes aligned.
+	 */
+	W(add)	sp, sp, #1	/* Reset 	  7 */
+	W(add)	sp, sp, #1	/* Undef	  6 */
+	W(add)	sp, sp, #1	/* Syscall	  5 */
+	W(add)	sp, sp, #1	/* Prefetch abort 4 */
+	W(add)	sp, sp, #1	/* Data abort	  3 */
+	W(add)	sp, sp, #1	/* HVC		  2 */
+	W(add)	sp, sp, #1	/* IRQ		  1 */
+	W(add)	sp, sp, #1	/* FIQ		  0 */
+
+	sub	sp, sp, #1
+
+	mcr	p15, 0, r0, c7, c5, 6	/* BPIALL */
+	isb
+
+	/*
+	 * Yet another silly hack: Use VPIDR as a temp register.
+	 * Thumb2 is really a pain, as SP cannot be used with most
+	 * of the bitwise instructions. The vect_br macro ensures
+	 * things gets cleaned-up.
+	 */
+	mcr	p15, 4, r0, c0, c0, 0	/* VPIDR */
+	mov	r0, sp
+	and	r0, r0, #7
+	sub	sp, sp, r0
+	push	{r1, r2}
+	mov	r1, r0
+	mrc	p15, 4, r0, c0, c0, 0	/* VPIDR */
+	mrc	p15, 0, r2, c0, c0, 0	/* MIDR  */
+	mcr	p15, 4, r2, c0, c0, 0	/* VPIDR */
+
+.macro vect_br val, targ
+	cmp	r1, #\val
+	popeq	{r1, r2}
+	beq	\targ
+.endm
+
+	vect_br	0, hyp_fiq
+	vect_br	1, hyp_irq
+	vect_br	2, hyp_hvc
+	vect_br	3, hyp_dabt
+	vect_br	4, hyp_pabt
+	vect_br	5, hyp_svc
+	vect_br	6, hyp_undef
+	vect_br	7, hyp_reset
 
 .macro invalid_vector label, cause
 	.align
@@ -149,7 +202,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 +219,7 @@ hyp_hvc:
 THUMB(	orr	lr, #1)
 	blx	lr			@ Call the HYP function
 
-	pop	{lr}
+	pop	{r2, lr}
 	eret
 
 guest_trap:
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
                   ` (2 preceding siblings ...)
  2018-01-08 18:55 ` [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit Marc Zyngier
@ 2018-01-08 18:55 ` Marc Zyngier
  2018-01-10  1:28   ` Florian Fainelli
  2018-01-08 18:55 ` [PATCH v2 5/6] arm: Invalidate icache on prefetch abort outside of user mapping on Cortex-A15 Marc Zyngier
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Marc Zyngier @ 2018-01-08 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

In order to avoid aliasing attacks against the branch predictor,
Cortex-A15 require to invalidate the BTB when switching
from one user context to another. The only way to do so on this
CPU is to perform an ICIALLU, having set ACTLR[0] to 1 from secure
mode.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mm/proc-v7-2level.S | 10 ++++++++++
 arch/arm/mm/proc-v7-3level.S | 16 ++++++++++++++++
 arch/arm/mm/proc-v7.S        | 18 +++++++++++++++++-
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index 0422e58b74e8..7dc9e1c69039 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -40,7 +40,17 @@
  *	Note that we always need to flush BTAC/BTB if IBE is set
  *	even on Cortex-A8 revisions not affected by 430973.
  *	If IBE is not set, the flush BTAC/BTB won't do anything.
+ *
+ *	Cortex-A15 requires ACTLR[0] to be set from secure in order
+ *	for the icache invalidation to also invalidate the BTB.
  */
+ENTRY(cpu_ca15_switch_mm)
+#ifdef CONFIG_MMU
+	mcr	p15, 0, r0, c7, c5, 0			@ ICIALLU
+	isb
+	b	cpu_v7_switch_mm
+#endif
+ENDPROC(cpu_ca15_switch_mm)
 ENTRY(cpu_v7_btbinv_switch_mm)
 #ifdef CONFIG_MMU
 	mov	r2, #0
diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index f6adfe88ead2..0a2245b309e5 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -71,6 +71,22 @@ ENTRY(cpu_v7_switch_mm)
 ENDPROC(cpu_v7_switch_mm)
 ENDPROC(cpu_v7_btbinv_switch_mm)
 
+/*
+ *	Cortex-A15 requires ACTLR[0] to be set from secure in order
+ *	for the icache invalidation to also invalidate the BTB.
+ */
+ENTRY(cpu_ca15_switch_mm)
+#ifdef CONFIG_MMU
+	mcr	p15, 0, r0, c7, c5, 0			@ ICIALLU
+	mmid	r2, r2
+	asid	r2, r2
+	orr	rpgdh, rpgdh, r2, lsl #(48 - 32)	@ upper 32-bits of pgd
+	mcrr	p15, 0, rpgdl, rpgdh, c2		@ set TTB 0
+	isb
+#endif
+	ret	lr
+ENDPROC(cpu_ca15_switch_mm)
+
 #ifdef __ARMEB__
 #define rl r3
 #define rh r2
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 0a14967fd400..9310fd9aa1cf 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -173,6 +173,21 @@ ENDPROC(cpu_v7_do_resume)
 	globl_equ	cpu_v7_btbinv_do_resume,	cpu_v7_do_resume
 #endif
 
+/*
+ * Cortex-A15 that require an icache invalidation on switch_mm
+ */
+	globl_equ	cpu_ca15_proc_init,		cpu_v7_proc_init
+	globl_equ	cpu_ca15_proc_fin,		cpu_v7_proc_fin
+	globl_equ	cpu_ca15_reset,			cpu_v7_reset
+	globl_equ	cpu_ca15_do_idle,		cpu_v7_do_idle
+	globl_equ	cpu_ca15_dcache_clean_area, 	cpu_v7_dcache_clean_area
+	globl_equ	cpu_ca15_set_pte_ext,		cpu_v7_set_pte_ext
+	globl_equ	cpu_ca15_suspend_size,		cpu_v7_suspend_size
+#ifdef CONFIG_ARM_CPU_SUSPEND
+	globl_equ	cpu_ca15_do_suspend,		cpu_v7_do_suspend
+	globl_equ	cpu_ca15_do_resume,		cpu_v7_do_resume
+#endif
+
 /*
  * Cortex-A9 processor functions
  */
@@ -549,6 +564,7 @@ __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
+	define_processor_functions ca15, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 #ifndef CONFIG_ARM_LPAE
 	define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 #endif
@@ -668,7 +684,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 = ca15_processor_functions
 	.size	__v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
 
 	/*
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v2 5/6] arm: Invalidate icache on prefetch abort outside of user mapping on Cortex-A15
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
                   ` (3 preceding siblings ...)
  2018-01-08 18:55 ` [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15 Marc Zyngier
@ 2018-01-08 18:55 ` Marc Zyngier
  2018-01-08 18:55 ` [PATCH v2 6/6] arm: KVM: Invalidate icache on guest exit for Cortex-A15 Marc Zyngier
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-08 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

In order to prevent aliasing attacks on the branch predictor,
invalidate the icache on Cortex-A15, which has the side effect
of invalidating the BTB. This requires ACTLR[0] to be set to 1
(secure operation).

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

diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
index 9e900ae855aa..07e27f212dc7 100644
--- a/arch/arm/include/asm/cp15.h
+++ b/arch/arm/include/asm/cp15.h
@@ -66,6 +66,7 @@
 #define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
 
 #define BPIALL				__ACCESS_CP15(c7, 0, c5, 6)
+#define ICIALLU				__ACCESS_CP15(c7, 0, c5, 0)
 
 extern unsigned long cr_alignment;	/* defined in entry-armv.S */
 
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index ff272ffcf741..bda37ce63fc7 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -417,6 +417,10 @@ do_pabt_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 		case ARM_CPU_PART_CORTEX_A17:
 			write_sysreg(0, BPIALL);
 			break;
+
+		case ARM_CPU_PART_CORTEX_A15:
+			write_sysreg(0, ICIALLU);
+			break;
 		}
 	}
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v2 6/6] arm: KVM: Invalidate icache on guest exit for Cortex-A15
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
                   ` (4 preceding siblings ...)
  2018-01-08 18:55 ` [PATCH v2 5/6] arm: Invalidate icache on prefetch abort outside of user mapping on Cortex-A15 Marc Zyngier
@ 2018-01-08 18:55 ` Marc Zyngier
  2018-01-23 14:26   ` Christoffer Dall
  2018-01-09 17:13 ` [PATCH v2 0/6] ARM branch predictor hardening Florian Fainelli
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Marc Zyngier @ 2018-01-08 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

In order to avoid aliasing attacks against the branch predictor
on Cortex-A15, let's invalidate the BTB on guest exit, which can
only be done by invalidating the icache (with ACTLR[0] being set).

We use the same hack as for A12/A17 to perform the vector decoding.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_mmu.h |  4 ++++
 arch/arm/kvm/hyp/hyp-entry.S   | 27 ++++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index b47db5b9e407..72ffb4d27fde 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -226,12 +226,16 @@ static inline void *kvm_get_hyp_vector(void)
 {
 	extern char __kvm_hyp_vector[];
 	extern char __kvm_hyp_vector_bp_inv[];
+	extern char __kvm_hyp_vector_ic_inv[];
 
 	switch(read_cpuid_part()) {
 	case ARM_CPU_PART_CORTEX_A12:
 	case ARM_CPU_PART_CORTEX_A17:
 		return kvm_ksym_ref(__kvm_hyp_vector_bp_inv);
 
+	case ARM_CPU_PART_CORTEX_A15:
+		return kvm_ksym_ref(__kvm_hyp_vector_ic_inv);
+
 	default:
 		return kvm_ksym_ref(__kvm_hyp_vector);
 	}
diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
index 2e8d2179eb70..7c0059927e2e 100644
--- a/arch/arm/kvm/hyp/hyp-entry.S
+++ b/arch/arm/kvm/hyp/hyp-entry.S
@@ -70,7 +70,31 @@ __kvm_hyp_vector:
 	W(b)	hyp_hvc
 	W(b)	hyp_irq
 	W(b)	hyp_fiq
-	
+
+	.align 5
+__kvm_hyp_vector_ic_inv:
+	.global __kvm_hyp_vector_ic_inv
+
+	/*
+	 * We encode the exception entry in the bottom 3 bits of
+	 * SP, and we have to guarantee to be 8 bytes aligned.
+	 */
+	W(add)	sp, sp, #1	/* Reset 	  7 */
+	W(add)	sp, sp, #1	/* Undef	  6 */
+	W(add)	sp, sp, #1	/* Syscall	  5 */
+	W(add)	sp, sp, #1	/* Prefetch abort 4 */
+	W(add)	sp, sp, #1	/* Data abort	  3 */
+	W(add)	sp, sp, #1	/* HVC		  2 */
+	W(add)	sp, sp, #1	/* IRQ		  1 */
+	W(add)	sp, sp, #1	/* FIQ		  0 */
+
+	sub	sp, sp, #1
+
+	mcr	p15, 0, r0, c7, c5, 0	/* ICIALLU */
+	isb
+
+	b	decode_vectors
+
 	.align 5
 __kvm_hyp_vector_bp_inv:
 	.global __kvm_hyp_vector_bp_inv
@@ -93,6 +117,7 @@ __kvm_hyp_vector_bp_inv:
 	mcr	p15, 0, r0, c7, c5, 6	/* BPIALL */
 	isb
 
+decode_vectors:
 	/*
 	 * Yet another silly hack: Use VPIDR as a temp register.
 	 * Thumb2 is really a pain, as SP cannot be used with most
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, A12 and A17
  2018-01-08 18:55 ` [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, " Marc Zyngier
@ 2018-01-09  9:56   ` Marc Zyngier
  2018-01-10 16:45   ` Russell King - ARM Linux
  1 sibling, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-09  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/01/18 18:55, Marc Zyngier wrote:
> In order to prevent aliasing attacks on the branch predictor,
> invalidate the BTB on CPUs that are known to be affected when taking
> a prefetch abort on a address that is outside of a user task limit.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/cp15.h |  2 ++
>  arch/arm/mm/fault.c         | 19 +++++++++++++
>  arch/arm/mm/fsr-2level.c    |  4 +--
>  arch/arm/mm/fsr-3level.c    | 67 ++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 89 insertions(+), 3 deletions(-)
> 
> 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..ff272ffcf741 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,7 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
>  	si.si_errno = 0;
>  	si.si_code = code;
>  	si.si_addr = (void __user *)addr;
> +
>  	force_sig_info(sig, &si, tsk);
>  }
>  
> @@ -404,6 +406,23 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  }
>  #endif					/* CONFIG_MMU */
>  
> +static int
> +do_pabt_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> +{
> +	if (addr > TASK_SIZE) {
> +		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_A17:
> +			write_sysreg(0, BPIALL);
> +			break;
> +		}
> +	}
> +
> +	return do_page_fault(addr, fsr, regs);
> +}

For the record, this breaks !MMU. I've fixed it locally by moving this
function inside the CONFIG_MMU part, and provided a dummy stub for !MMU.

Thanks,

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-08 18:55 ` [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17 Marc Zyngier
@ 2018-01-09 14:14   ` Andre Przywara
  2018-01-09 14:21     ` Marc Zyngier
  2018-01-10 17:53   ` Tony Lindgren
  1 sibling, 1 reply; 32+ messages in thread
From: Andre Przywara @ 2018-01-09 14:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 08/01/18 18:55, 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 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        | 30 +++++++++++++++---------------
>  3 files changed, 23 insertions(+), 17 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..f6adfe88ead2 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	r3, #0

As Robin pointed out correctly, BPIALL ignores Rt, so you can get rid of
that line entirely (which is not matching the actual Rt below, btw).
Might be worth to add a comment about this.

Cheers,
Andre.

> +	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..0a14967fd400 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/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
>  
>  	/*
> @@ -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
>  
>  	/*
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-09 14:14   ` Andre Przywara
@ 2018-01-09 14:21     ` Marc Zyngier
  2018-01-09 14:22       ` Marc Zyngier
  0 siblings, 1 reply; 32+ messages in thread
From: Marc Zyngier @ 2018-01-09 14:21 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/01/18 14:14, Andre Przywara wrote:
> Hi,
> 
> On 08/01/18 18:55, 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 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        | 30 +++++++++++++++---------------
>>  3 files changed, 23 insertions(+), 17 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..f6adfe88ead2 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	r3, #0
> 
> As Robin pointed out correctly, BPIALL ignores Rt, so you can get rid of
> that line entirely (which is not matching the actual Rt below, btw).
> Might be worth to add a comment about this.
I know. I just kept it out of consistency with the existing Cortex-A8
workaround, which may or may not behave the same way (I don't have one
around to test...).

Thanks,

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-09 14:21     ` Marc Zyngier
@ 2018-01-09 14:22       ` Marc Zyngier
  0 siblings, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-09 14:22 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/01/18 14:21, Marc Zyngier wrote:
> On 09/01/18 14:14, Andre Przywara wrote:
>> Hi,
>>
>> On 08/01/18 18:55, 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 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        | 30 +++++++++++++++---------------
>>>  3 files changed, 23 insertions(+), 17 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..f6adfe88ead2 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	r3, #0
>>
>> As Robin pointed out correctly, BPIALL ignores Rt, so you can get rid of
>> that line entirely (which is not matching the actual Rt below, btw).
>> Might be worth to add a comment about this.
> I know. I just kept it out of consistency with the existing Cortex-A8
> workaround, which may or may not behave the same way (I don't have one
> around to test...).

[pressed send too quickly]

And yes, the r2/r3 business is yet another blunder. Duh.

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 0/6] ARM branch predictor hardening
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
                   ` (5 preceding siblings ...)
  2018-01-08 18:55 ` [PATCH v2 6/6] arm: KVM: Invalidate icache on guest exit for Cortex-A15 Marc Zyngier
@ 2018-01-09 17:13 ` Florian Fainelli
  2018-01-09 17:46   ` Russell King - ARM Linux
  2018-01-10 16:50 ` Nishanth Menon
  2018-01-10 22:59 ` Nishanth Menon
  8 siblings, 1 reply; 32+ messages in thread
From: Florian Fainelli @ 2018-01-09 17:13 UTC (permalink / raw)
  To: linux-arm-kernel

On January 8, 2018 10:55:27 AM PST, Marc Zyngier <marc.zyngier@arm.com> wrote:
>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 notable exception is Cortex-A15, where
>BTB invalidation behaves like a NOP, and the only way to shoot the
>predictor down is to invalidate the icache *and* to have ACTLR[0] set
>to 1 (which is a secure-only operation).

Is there a publicly available test case/exploit that we could use to regress test this against? I will follow up with the Brahma-B15 patches after you send your v3.

Thanks!
Hi Marc,
-- 
Florian

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 0/6] ARM branch predictor hardening
  2018-01-09 17:13 ` [PATCH v2 0/6] ARM branch predictor hardening Florian Fainelli
@ 2018-01-09 17:46   ` Russell King - ARM Linux
  0 siblings, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2018-01-09 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 09, 2018 at 09:13:37AM -0800, Florian Fainelli wrote:
> On January 8, 2018 10:55:27 AM PST, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >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 notable exception is Cortex-A15, where
> >BTB invalidation behaves like a NOP, and the only way to shoot the
> >predictor down is to invalidate the icache *and* to have ACTLR[0] set
> >to 1 (which is a secure-only operation).
> 
> Is there a publicly available test case/exploit that we could use to
> regress test this against? I will follow up with the Brahma-B15
> patches after you send your v3.

While there are some x86 programs as part of the original information
release, this is something that I've been concerned about, and over
the days since I've been working hard at researching the bugs on the
various ARM CPUs.

The first thing I need to say is that the original x86 programs have
several issues that make them unreliable even on x86 hardware - for
example, running them on a Core 2 Duo after replacing "rdtscp" with
"lfence; rdtsc" results in failures - partly because they're a slower
processor, but also because the exploits are not particularly well
written.

The whole idea is that you speculatively drag a cache line in and
identify which cache line was dragged in - particularly in the
spectre case, where we are trying to identify one of 256 cache lines.

This really gets messed up if the cache line for the zero or 255
byte value always gets loaded due to the compiler laying the data
out such that (eg) "temp" shares the same cache line as "array2"!

Hence, I'm really not impressed by these exploits - a failure with
them does not mean there isn't an issue, it just means that they
didn't identify an issue which could be due to bugs within the
exploit programs themselves!

So, I've been putting together a set of better exploit programs
which work on x86-64, x86-32, ARM64, and various ARM32 machines.

This has lead me to some interesting observations that I'm not yet
ready to share all the details of publicly, some of them lead me
to question whether flushing the BTB can be deemed to really
mitigate the problem.

This is why I've been fairly quiet on public forums about this so
far.

Please bear in mind that the release of this problem has not been
managed particularly well (the fact that there aren't fixes already
prepared and lined up to go tells you that) and there's quite a
panic to (a) understand and (b) come up with proper fixes at the
moment.

It is quite understandable that people want answers and fixes as a
top priority, but I believe that rushing ahead without a full and
proper understanding of the issues here would be very foolhardy,
and /could/ end up making exploitation easier rather than harder.

-- 
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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15
  2018-01-08 18:55 ` [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15 Marc Zyngier
@ 2018-01-10  1:28   ` Florian Fainelli
  2018-01-10  1:33     ` André Przywara
  0 siblings, 1 reply; 32+ messages in thread
From: Florian Fainelli @ 2018-01-10  1:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/08/2018 10:55 AM, Marc Zyngier wrote:
> In order to avoid aliasing attacks against the branch predictor,
> Cortex-A15 require to invalidate the BTB when switching
> from one user context to another. The only way to do so on this
> CPU is to perform an ICIALLU, having set ACTLR[0] to 1 from secure
> mode.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---

[snip]

> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> index f6adfe88ead2..0a2245b309e5 100644
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -71,6 +71,22 @@ ENTRY(cpu_v7_switch_mm)
>  ENDPROC(cpu_v7_switch_mm)
>  ENDPROC(cpu_v7_btbinv_switch_mm)
>  
> +/*
> + *	Cortex-A15 requires ACTLR[0] to be set from secure in order
> + *	for the icache invalidation to also invalidate the BTB.

Considering that writes are ignored when we don't have the correct
permission level, how about set try to set this bit from the
__v7_ca15mp_setup and __v7_b15mp_setup labels just like we are setting
the SMP_EN bit for the poor bastards out there stuck with possibly
frozen bootloaders/ATF?
-- 
Florian

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15
  2018-01-10  1:28   ` Florian Fainelli
@ 2018-01-10  1:33     ` André Przywara
  2018-01-10  1:35       ` Florian Fainelli
  0 siblings, 1 reply; 32+ messages in thread
From: André Przywara @ 2018-01-10  1:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/01/18 01:28, Florian Fainelli wrote:
> On 01/08/2018 10:55 AM, Marc Zyngier wrote:
>> In order to avoid aliasing attacks against the branch predictor,
>> Cortex-A15 require to invalidate the BTB when switching
>> from one user context to another. The only way to do so on this
>> CPU is to perform an ICIALLU, having set ACTLR[0] to 1 from secure
>> mode.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
> 
> [snip]
> 
>> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>> index f6adfe88ead2..0a2245b309e5 100644
>> --- a/arch/arm/mm/proc-v7-3level.S
>> +++ b/arch/arm/mm/proc-v7-3level.S
>> @@ -71,6 +71,22 @@ ENTRY(cpu_v7_switch_mm)
>>  ENDPROC(cpu_v7_switch_mm)
>>  ENDPROC(cpu_v7_btbinv_switch_mm)
>>  
>> +/*
>> + *	Cortex-A15 requires ACTLR[0] to be set from secure in order
>> + *	for the icache invalidation to also invalidate the BTB.
> 
> Considering that writes are ignored when we don't have the correct
> permission level, how about set try to set this bit from the
> __v7_ca15mp_setup and __v7_b15mp_setup labels just like we are setting
> the SMP_EN bit for the poor bastards out there stuck with possibly
> frozen bootloaders/ATF?

Even when writes to ACTLR are allowed by secure world, this only
actually applies to the SMP bit:
ARM DDI0438H ARM Cortex-A15 TRM, 4.3.28 Auxiliary Control Register:
"-- Read/write in Non-secure PL1 and PL2 modes if NSACR.NS_SMP is 1. In
this case, all bits are write-ignored except for the SMP bit."

So: good idea, but no luck here :-(

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15
  2018-01-10  1:33     ` André Przywara
@ 2018-01-10  1:35       ` Florian Fainelli
  2018-01-10  9:13         ` Andre Przywara
  0 siblings, 1 reply; 32+ messages in thread
From: Florian Fainelli @ 2018-01-10  1:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/09/2018 05:33 PM, Andr? Przywara wrote:
> On 10/01/18 01:28, Florian Fainelli wrote:
>> On 01/08/2018 10:55 AM, Marc Zyngier wrote:
>>> In order to avoid aliasing attacks against the branch predictor,
>>> Cortex-A15 require to invalidate the BTB when switching
>>> from one user context to another. The only way to do so on this
>>> CPU is to perform an ICIALLU, having set ACTLR[0] to 1 from secure
>>> mode.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>
>> [snip]
>>
>>> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>> index f6adfe88ead2..0a2245b309e5 100644
>>> --- a/arch/arm/mm/proc-v7-3level.S
>>> +++ b/arch/arm/mm/proc-v7-3level.S
>>> @@ -71,6 +71,22 @@ ENTRY(cpu_v7_switch_mm)
>>>  ENDPROC(cpu_v7_switch_mm)
>>>  ENDPROC(cpu_v7_btbinv_switch_mm)
>>>  
>>> +/*
>>> + *	Cortex-A15 requires ACTLR[0] to be set from secure in order
>>> + *	for the icache invalidation to also invalidate the BTB.
>>
>> Considering that writes are ignored when we don't have the correct
>> permission level, how about set try to set this bit from the
>> __v7_ca15mp_setup and __v7_b15mp_setup labels just like we are setting
>> the SMP_EN bit for the poor bastards out there stuck with possibly
>> frozen bootloaders/ATF?
> 
> Even when writes to ACTLR are allowed by secure world, this only
> actually applies to the SMP bit:
> ARM DDI0438H ARM Cortex-A15 TRM, 4.3.28 Auxiliary Control Register:
> "-- Read/write in Non-secure PL1 and PL2 modes if NSACR.NS_SMP is 1. In
> this case, all bits are write-ignored except for the SMP bit."
> 
> So: good idea, but no luck here :-(

Looks like I failed basic reading exercise here, thanks ;)
--
Florian

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15
  2018-01-10  1:35       ` Florian Fainelli
@ 2018-01-10  9:13         ` Andre Przywara
  0 siblings, 0 replies; 32+ messages in thread
From: Andre Przywara @ 2018-01-10  9:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 10/01/18 01:35, Florian Fainelli wrote:
> On 01/09/2018 05:33 PM, Andr? Przywara wrote:
>> On 10/01/18 01:28, Florian Fainelli wrote:
>>> On 01/08/2018 10:55 AM, Marc Zyngier wrote:
>>>> In order to avoid aliasing attacks against the branch predictor,
>>>> Cortex-A15 require to invalidate the BTB when switching
>>>> from one user context to another. The only way to do so on this
>>>> CPU is to perform an ICIALLU, having set ACTLR[0] to 1 from secure
>>>> mode.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>
>>> [snip]
>>>
>>>> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>>> index f6adfe88ead2..0a2245b309e5 100644
>>>> --- a/arch/arm/mm/proc-v7-3level.S
>>>> +++ b/arch/arm/mm/proc-v7-3level.S
>>>> @@ -71,6 +71,22 @@ ENTRY(cpu_v7_switch_mm)
>>>>  ENDPROC(cpu_v7_switch_mm)
>>>>  ENDPROC(cpu_v7_btbinv_switch_mm)
>>>>  
>>>> +/*
>>>> + *	Cortex-A15 requires ACTLR[0] to be set from secure in order
>>>> + *	for the icache invalidation to also invalidate the BTB.
>>>
>>> Considering that writes are ignored when we don't have the correct
>>> permission level, how about set try to set this bit from the
>>> __v7_ca15mp_setup and __v7_b15mp_setup labels just like we are setting
>>> the SMP_EN bit for the poor bastards out there stuck with possibly
>>> frozen bootloaders/ATF?
>>
>> Even when writes to ACTLR are allowed by secure world, this only
>> actually applies to the SMP bit:
>> ARM DDI0438H ARM Cortex-A15 TRM, 4.3.28 Auxiliary Control Register:
>> "-- Read/write in Non-secure PL1 and PL2 modes if NSACR.NS_SMP is 1. In
>> this case, all bits are write-ignored except for the SMP bit."
>>
>> So: good idea, but no luck here :-(
> 
> Looks like I failed basic reading exercise here, thanks ;)

No worries, this is totally non-obvious, and I wasn't aware of it as
well until Marc cursed about it yesterday ;-)

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, A12 and A17
  2018-01-08 18:55 ` [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, " Marc Zyngier
  2018-01-09  9:56   ` Marc Zyngier
@ 2018-01-10 16:45   ` Russell King - ARM Linux
  1 sibling, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2018-01-10 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 08, 2018 at 06:55:29PM +0000, Marc Zyngier wrote:
> In order to prevent aliasing attacks on the branch predictor,
> invalidate the BTB on CPUs that are known to be affected when taking
> a prefetch abort on a address that is outside of a user task limit.

Can you please describe to me what sort of exploit this is supposed
to be protecting against - if you do not wish to make the details
public, please reply in private.

As far as I can see, this has no effect on the exploits that have been
made public to date as none of them involve the prefetch abort handler,
and from what I can see in the "Cache Speculation Side-Channels"
document, no mention is made of the prefetch abort.

Indeed, I've received feedback from Florian that my set of "exploits"
based on the published information to date are unaffected by your
patch series, so I'm really interested to know exactly what this
series is trying to fix.

Thanks.

-- 
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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 0/6] ARM branch predictor hardening
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
                   ` (6 preceding siblings ...)
  2018-01-09 17:13 ` [PATCH v2 0/6] ARM branch predictor hardening Florian Fainelli
@ 2018-01-10 16:50 ` Nishanth Menon
  2018-01-10 17:16   ` Marc Zyngier
  2018-01-10 22:59 ` Nishanth Menon
  8 siblings, 1 reply; 32+ messages in thread
From: Nishanth Menon @ 2018-01-10 16:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/08/2018 12:55 PM, Marc Zyngier wrote:
> 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 notable exception is Cortex-A15, where
> BTB invalidation behaves like a NOP, and the only way to shoot the
> predictor down is to invalidate the icache *and* to have ACTLR[0] set
> to 1 (which is a secure-only operation).
> 


btw, just wanted to understand if we had any reasons as to why 
we'arent tagging these for stable? Yes, I am aware of Greg's comments 
in [1], but the v7 series impacts a heck of a lot of existing products 
and is not that extensive to cause too much of a pain is it?

OR, am I missing some thing else?

[1] http://www.kroah.com/log/blog/2018/01/06/meltdown-status/

-- 
Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 0/6] ARM branch predictor hardening
  2018-01-10 16:50 ` Nishanth Menon
@ 2018-01-10 17:16   ` Marc Zyngier
  0 siblings, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-10 17:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/01/18 16:50, Nishanth Menon wrote:
> On 01/08/2018 12:55 PM, Marc Zyngier wrote:
>> 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 notable exception is Cortex-A15, where
>> BTB invalidation behaves like a NOP, and the only way to shoot the
>> predictor down is to invalidate the icache *and* to have ACTLR[0] set
>> to 1 (which is a secure-only operation).
>>
> 
> 
> btw, just wanted to understand if we had any reasons as to why 
> we'arent tagging these for stable? Yes, I am aware of Greg's comments 
> in [1], but the v7 series impacts a heck of a lot of existing products 
> and is not that extensive to cause too much of a pain is it?
> 
> OR, am I missing some thing else?
> 
> [1] http://www.kroah.com/log/blog/2018/01/06/meltdown-status/

This is a work in progress. It is not ready for being merged yet. It can
be backported to stable after being merged into mainline.

Thanks,

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-08 18:55 ` [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17 Marc Zyngier
  2018-01-09 14:14   ` Andre Przywara
@ 2018-01-10 17:53   ` Tony Lindgren
  2018-01-10 17:57     ` Marc Zyngier
  1 sibling, 1 reply; 32+ messages in thread
From: Tony Lindgren @ 2018-01-10 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

* Marc Zyngier <marc.zyngier@arm.com> [180108 19:00]:
> 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 and A17.

I suspect we now must also make sure Cortex-A8 has the IBE bit
set unconditionally for this to work. Currently the assumption is
that IBE bit needs to be set only on the earlier CPU revisions
that suffer from ARM_ERRATA_430973.

> --- 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

So without IBE set, as the comments above say, the flush won't
do anything.

Regards,

Tony

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-10 17:53   ` Tony Lindgren
@ 2018-01-10 17:57     ` Marc Zyngier
  2018-01-10 21:52       ` Nishanth Menon
  0 siblings, 1 reply; 32+ messages in thread
From: Marc Zyngier @ 2018-01-10 17:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/01/18 17:53, Tony Lindgren wrote:
> * Marc Zyngier <marc.zyngier@arm.com> [180108 19:00]:
>> 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 and A17.
> 
> I suspect we now must also make sure Cortex-A8 has the IBE bit
> set unconditionally for this to work. Currently the assumption is
> that IBE bit needs to be set only on the earlier CPU revisions
> that suffer from ARM_ERRATA_430973.
> 
>> --- 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
> 
> So without IBE set, as the comments above say, the flush won't
> do anything.

Indeed. Firmware/bootloaders must be updated to set IBE, just like on
Cortex-A15. I'll add a note to that effect.

Thanks,

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-10 17:57     ` Marc Zyngier
@ 2018-01-10 21:52       ` Nishanth Menon
  2018-01-11  9:03         ` Marc Zyngier
  0 siblings, 1 reply; 32+ messages in thread
From: Nishanth Menon @ 2018-01-10 21:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/10/2018 11:57 AM, Marc Zyngier wrote:
> On 10/01/18 17:53, Tony Lindgren wrote:
>> * Marc Zyngier <marc.zyngier@arm.com> [180108 19:00]:
>>> 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 and A17.
>>
>> I suspect we now must also make sure Cortex-A8 has the IBE bit
>> set unconditionally for this to work. Currently the assumption is
>> that IBE bit needs to be set only on the earlier CPU revisions
>> that suffer from ARM_ERRATA_430973.
>>
>>> --- 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
>>
>> So without IBE set, as the comments above say, the flush won't
>> do anything.
> 
> Indeed. Firmware/bootloaders must be updated to set IBE, just like on
> Cortex-A15. I'll add a note to that effect.
OK. in u-boot, I had helped on the following:
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=5902f4ce0f2bd1411e40dc0ece3598a0fc19b2ae

maybe be build off that?

-- 
Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 0/6] ARM branch predictor hardening
  2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
                   ` (7 preceding siblings ...)
  2018-01-10 16:50 ` Nishanth Menon
@ 2018-01-10 22:59 ` Nishanth Menon
  8 siblings, 0 replies; 32+ messages in thread
From: Nishanth Menon @ 2018-01-10 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/08/2018 12:55 PM, Marc Zyngier wrote:
[...]
> 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 notable exception is Cortex-A15, where
> BTB invalidation behaves like a NOP, and the only way to shoot the
> predictor down is to invalidate the icache *and* to have ACTLR[0] set
> to 1 (which is a secure-only operation).

On u-boot side, we'd probably have to work off ->
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a615d0be6a73fc48a22e5662608260fe9b9149ff


[...]

-- 
Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-10 21:52       ` Nishanth Menon
@ 2018-01-11  9:03         ` Marc Zyngier
  2018-01-29 18:41           ` Fabio Estevam
  0 siblings, 1 reply; 32+ messages in thread
From: Marc Zyngier @ 2018-01-11  9:03 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/01/18 21:52, Nishanth Menon wrote:
> On 01/10/2018 11:57 AM, Marc Zyngier wrote:
>> On 10/01/18 17:53, Tony Lindgren wrote:
>>> * Marc Zyngier <marc.zyngier@arm.com> [180108 19:00]:
>>>> 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 and A17.
>>>
>>> I suspect we now must also make sure Cortex-A8 has the IBE bit
>>> set unconditionally for this to work. Currently the assumption is
>>> that IBE bit needs to be set only on the earlier CPU revisions
>>> that suffer from ARM_ERRATA_430973.
>>>
>>>> --- 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
>>>
>>> So without IBE set, as the comments above say, the flush won't
>>> do anything.
>>
>> Indeed. Firmware/bootloaders must be updated to set IBE, just like on
>> Cortex-A15. I'll add a note to that effect.
> OK. in u-boot, I had helped on the following:
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=5902f4ce0f2bd1411e40dc0ece3598a0fc19b2ae
> 
> maybe be build off that?

Turn that into something unconditional, and you'll be good.

Thanks,

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit
  2018-01-08 18:55 ` [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit Marc Zyngier
@ 2018-01-15 12:22   ` Robin Murphy
  2018-01-23 14:22   ` Christoffer Dall
  1 sibling, 0 replies; 32+ messages in thread
From: Robin Murphy @ 2018-01-15 12:22 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/01/18 18:55, Marc Zyngier 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/include/asm/kvm_asm.h |  2 --
>   arch/arm/include/asm/kvm_mmu.h | 13 ++++++++-
>   arch/arm/kvm/hyp/hyp-entry.S   | 64 ++++++++++++++++++++++++++++++++++++++++--
>   3 files changed, 74 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
> index 36dd2962a42d..df24ed48977d 100644
> --- a/arch/arm/include/asm/kvm_asm.h
> +++ b/arch/arm/include/asm/kvm_asm.h
> @@ -61,8 +61,6 @@ struct kvm_vcpu;
>   extern char __kvm_hyp_init[];
>   extern char __kvm_hyp_init_end[];
>   
> -extern char __kvm_hyp_vector[];
> -
>   extern void __kvm_flush_vm_context(void);
>   extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
>   extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index eb46fc81a440..b47db5b9e407 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -37,6 +37,7 @@
>   
>   #include <linux/highmem.h>
>   #include <asm/cacheflush.h>
> +#include <asm/cputype.h>
>   #include <asm/pgalloc.h>
>   #include <asm/stage2_pgtable.h>
>   
> @@ -223,7 +224,17 @@ static inline unsigned int kvm_get_vmid_bits(void)
>   
>   static inline void *kvm_get_hyp_vector(void)
>   {
> -	return kvm_ksym_ref(__kvm_hyp_vector);
> +	extern char __kvm_hyp_vector[];
> +	extern char __kvm_hyp_vector_bp_inv[];
> +
> +	switch(read_cpuid_part()) {
> +	case ARM_CPU_PART_CORTEX_A12:
> +	case ARM_CPU_PART_CORTEX_A17:
> +		return kvm_ksym_ref(__kvm_hyp_vector_bp_inv);
> +
> +	default:
> +		return kvm_ksym_ref(__kvm_hyp_vector);
> +	}
>   }
>   
>   static inline int kvm_map_vectors(void)
> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> index 95a2faefc070..2e8d2179eb70 100644
> --- a/arch/arm/kvm/hyp/hyp-entry.S
> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> @@ -70,6 +70,59 @@ __kvm_hyp_vector:
>   	W(b)	hyp_hvc
>   	W(b)	hyp_irq
>   	W(b)	hyp_fiq
> +	
> +	.align 5
> +__kvm_hyp_vector_bp_inv:
> +	.global __kvm_hyp_vector_bp_inv
> +
> +	/*
> +	 * We encode the exception entry in the bottom 3 bits of
> +	 * SP, and we have to guarantee to be 8 bytes aligned.
> +	 */
> +	W(add)	sp, sp, #1	/* Reset 	  7 */
> +	W(add)	sp, sp, #1	/* Undef	  6 */
> +	W(add)	sp, sp, #1	/* Syscall	  5 */
> +	W(add)	sp, sp, #1	/* Prefetch abort 4 */
> +	W(add)	sp, sp, #1	/* Data abort	  3 */
> +	W(add)	sp, sp, #1	/* HVC		  2 */
> +	W(add)	sp, sp, #1	/* IRQ		  1 */
> +	W(add)	sp, sp, #1	/* FIQ		  0 */
> +
> +	sub	sp, sp, #1

FWIW, I'd be inclined to remove the above two instructions, but leave 
the FIQ comment (and blank line before the mcr) in place for clarity.

Robin.

> +	mcr	p15, 0, r0, c7, c5, 6	/* BPIALL */
> +	isb
> +
> +	/*
> +	 * Yet another silly hack: Use VPIDR as a temp register.
> +	 * Thumb2 is really a pain, as SP cannot be used with most
> +	 * of the bitwise instructions. The vect_br macro ensures
> +	 * things gets cleaned-up.
> +	 */
> +	mcr	p15, 4, r0, c0, c0, 0	/* VPIDR */
> +	mov	r0, sp
> +	and	r0, r0, #7
> +	sub	sp, sp, r0
> +	push	{r1, r2}
> +	mov	r1, r0
> +	mrc	p15, 4, r0, c0, c0, 0	/* VPIDR */
> +	mrc	p15, 0, r2, c0, c0, 0	/* MIDR  */
> +	mcr	p15, 4, r2, c0, c0, 0	/* VPIDR */
> +
> +.macro vect_br val, targ
> +	cmp	r1, #\val
> +	popeq	{r1, r2}
> +	beq	\targ
> +.endm
> +
> +	vect_br	0, hyp_fiq
> +	vect_br	1, hyp_irq
> +	vect_br	2, hyp_hvc
> +	vect_br	3, hyp_dabt
> +	vect_br	4, hyp_pabt
> +	vect_br	5, hyp_svc
> +	vect_br	6, hyp_undef
> +	vect_br	7, hyp_reset
>   
>   .macro invalid_vector label, cause
>   	.align
> @@ -149,7 +202,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 +219,7 @@ hyp_hvc:
>   THUMB(	orr	lr, #1)
>   	blx	lr			@ Call the HYP function
>   
> -	pop	{lr}
> +	pop	{r2, lr}
>   	eret
>   
>   guest_trap:
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit
  2018-01-08 18:55 ` [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit Marc Zyngier
  2018-01-15 12:22   ` Robin Murphy
@ 2018-01-23 14:22   ` Christoffer Dall
  2018-01-23 14:38     ` Marc Zyngier
  1 sibling, 1 reply; 32+ messages in thread
From: Christoffer Dall @ 2018-01-23 14:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 08, 2018 at 06:55:30PM +0000, Marc Zyngier 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.

...because that would defeat the overall purpose of what we're trying to
do?

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

I don't understand this comment, it seems like the logic below is
limited to A12 and A17 for now?

> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_asm.h |  2 --
>  arch/arm/include/asm/kvm_mmu.h | 13 ++++++++-
>  arch/arm/kvm/hyp/hyp-entry.S   | 64 ++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 74 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
> index 36dd2962a42d..df24ed48977d 100644
> --- a/arch/arm/include/asm/kvm_asm.h
> +++ b/arch/arm/include/asm/kvm_asm.h
> @@ -61,8 +61,6 @@ struct kvm_vcpu;
>  extern char __kvm_hyp_init[];
>  extern char __kvm_hyp_init_end[];
>  
> -extern char __kvm_hyp_vector[];
> -
>  extern void __kvm_flush_vm_context(void);
>  extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
>  extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index eb46fc81a440..b47db5b9e407 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -37,6 +37,7 @@
>  
>  #include <linux/highmem.h>
>  #include <asm/cacheflush.h>
> +#include <asm/cputype.h>
>  #include <asm/pgalloc.h>
>  #include <asm/stage2_pgtable.h>
>  
> @@ -223,7 +224,17 @@ static inline unsigned int kvm_get_vmid_bits(void)
>  
>  static inline void *kvm_get_hyp_vector(void)
>  {
> -	return kvm_ksym_ref(__kvm_hyp_vector);
> +	extern char __kvm_hyp_vector[];
> +	extern char __kvm_hyp_vector_bp_inv[];
> +
> +	switch(read_cpuid_part()) {
> +	case ARM_CPU_PART_CORTEX_A12:
> +	case ARM_CPU_PART_CORTEX_A17:
> +		return kvm_ksym_ref(__kvm_hyp_vector_bp_inv);
> +
> +	default:
> +		return kvm_ksym_ref(__kvm_hyp_vector);
> +	}
>  }
>  
>  static inline int kvm_map_vectors(void)
> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> index 95a2faefc070..2e8d2179eb70 100644
> --- a/arch/arm/kvm/hyp/hyp-entry.S
> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> @@ -70,6 +70,59 @@ __kvm_hyp_vector:
>  	W(b)	hyp_hvc
>  	W(b)	hyp_irq
>  	W(b)	hyp_fiq
> +	
> +	.align 5
> +__kvm_hyp_vector_bp_inv:
> +	.global __kvm_hyp_vector_bp_inv
> +
> +	/*
> +	 * We encode the exception entry in the bottom 3 bits of
> +	 * SP, and we have to guarantee to be 8 bytes aligned.
> +	 */
> +	W(add)	sp, sp, #1	/* Reset 	  7 */
> +	W(add)	sp, sp, #1	/* Undef	  6 */
> +	W(add)	sp, sp, #1	/* Syscall	  5 */
> +	W(add)	sp, sp, #1	/* Prefetch abort 4 */
> +	W(add)	sp, sp, #1	/* Data abort	  3 */
> +	W(add)	sp, sp, #1	/* HVC		  2 */
> +	W(add)	sp, sp, #1	/* IRQ		  1 */
> +	W(add)	sp, sp, #1	/* FIQ		  0 */
> +
> +	sub	sp, sp, #1
> +
> +	mcr	p15, 0, r0, c7, c5, 6	/* BPIALL */
> +	isb
> +
> +	/*
> +	 * Yet another silly hack: Use VPIDR as a temp register.
> +	 * Thumb2 is really a pain, as SP cannot be used with most
> +	 * of the bitwise instructions. The vect_br macro ensures
> +	 * things gets cleaned-up.
> +	 */
> +	mcr	p15, 4, r0, c0, c0, 0	/* VPIDR */
> +	mov	r0, sp
> +	and	r0, r0, #7
> +	sub	sp, sp, r0
> +	push	{r1, r2}
> +	mov	r1, r0
> +	mrc	p15, 4, r0, c0, c0, 0	/* VPIDR */
> +	mrc	p15, 0, r2, c0, c0, 0	/* MIDR  */
> +	mcr	p15, 4, r2, c0, c0, 0	/* VPIDR */
> +
> +.macro vect_br val, targ
> +	cmp	r1, #\val
> +	popeq	{r1, r2}
> +	beq	\targ
> +.endm
> +
> +	vect_br	0, hyp_fiq
> +	vect_br	1, hyp_irq
> +	vect_br	2, hyp_hvc
> +	vect_br	3, hyp_dabt
> +	vect_br	4, hyp_pabt
> +	vect_br	5, hyp_svc
> +	vect_br	6, hyp_undef
> +	vect_br	7, hyp_reset
>  
>  .macro invalid_vector label, cause
>  	.align
> @@ -149,7 +202,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 +219,7 @@ hyp_hvc:
>  THUMB(	orr	lr, #1)
>  	blx	lr			@ Call the HYP function
>  
> -	pop	{lr}
> +	pop	{r2, lr}
>  	eret
>  
>  guest_trap:
> -- 
> 2.14.2
> 

Otherwise this looks 'good' to me.
-Christoffer

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 6/6] arm: KVM: Invalidate icache on guest exit for Cortex-A15
  2018-01-08 18:55 ` [PATCH v2 6/6] arm: KVM: Invalidate icache on guest exit for Cortex-A15 Marc Zyngier
@ 2018-01-23 14:26   ` Christoffer Dall
  0 siblings, 0 replies; 32+ messages in thread
From: Christoffer Dall @ 2018-01-23 14:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 08, 2018 at 06:55:33PM +0000, Marc Zyngier wrote:
> In order to avoid aliasing attacks against the branch predictor
> on Cortex-A15, let's invalidate the BTB on guest exit, which can
> only be done by invalidating the icache (with ACTLR[0] being set).
> 
> We use the same hack as for A12/A17 to perform the vector decoding.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

> ---
>  arch/arm/include/asm/kvm_mmu.h |  4 ++++
>  arch/arm/kvm/hyp/hyp-entry.S   | 27 ++++++++++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index b47db5b9e407..72ffb4d27fde 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -226,12 +226,16 @@ static inline void *kvm_get_hyp_vector(void)
>  {
>  	extern char __kvm_hyp_vector[];
>  	extern char __kvm_hyp_vector_bp_inv[];
> +	extern char __kvm_hyp_vector_ic_inv[];
>  
>  	switch(read_cpuid_part()) {
>  	case ARM_CPU_PART_CORTEX_A12:
>  	case ARM_CPU_PART_CORTEX_A17:
>  		return kvm_ksym_ref(__kvm_hyp_vector_bp_inv);
>  
> +	case ARM_CPU_PART_CORTEX_A15:
> +		return kvm_ksym_ref(__kvm_hyp_vector_ic_inv);
> +
>  	default:
>  		return kvm_ksym_ref(__kvm_hyp_vector);
>  	}
> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> index 2e8d2179eb70..7c0059927e2e 100644
> --- a/arch/arm/kvm/hyp/hyp-entry.S
> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> @@ -70,7 +70,31 @@ __kvm_hyp_vector:
>  	W(b)	hyp_hvc
>  	W(b)	hyp_irq
>  	W(b)	hyp_fiq
> -	
> +
> +	.align 5
> +__kvm_hyp_vector_ic_inv:
> +	.global __kvm_hyp_vector_ic_inv
> +
> +	/*
> +	 * We encode the exception entry in the bottom 3 bits of
> +	 * SP, and we have to guarantee to be 8 bytes aligned.
> +	 */
> +	W(add)	sp, sp, #1	/* Reset 	  7 */
> +	W(add)	sp, sp, #1	/* Undef	  6 */
> +	W(add)	sp, sp, #1	/* Syscall	  5 */
> +	W(add)	sp, sp, #1	/* Prefetch abort 4 */
> +	W(add)	sp, sp, #1	/* Data abort	  3 */
> +	W(add)	sp, sp, #1	/* HVC		  2 */
> +	W(add)	sp, sp, #1	/* IRQ		  1 */
> +	W(add)	sp, sp, #1	/* FIQ		  0 */
> +
> +	sub	sp, sp, #1
> +
> +	mcr	p15, 0, r0, c7, c5, 0	/* ICIALLU */
> +	isb
> +
> +	b	decode_vectors
> +
>  	.align 5
>  __kvm_hyp_vector_bp_inv:
>  	.global __kvm_hyp_vector_bp_inv
> @@ -93,6 +117,7 @@ __kvm_hyp_vector_bp_inv:
>  	mcr	p15, 0, r0, c7, c5, 6	/* BPIALL */
>  	isb
>  
> +decode_vectors:
>  	/*
>  	 * Yet another silly hack: Use VPIDR as a temp register.
>  	 * Thumb2 is really a pain, as SP cannot be used with most
> -- 
> 2.14.2
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit
  2018-01-23 14:22   ` Christoffer Dall
@ 2018-01-23 14:38     ` Marc Zyngier
  0 siblings, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-01-23 14:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 23/01/18 14:22, Christoffer Dall wrote:
> On Mon, Jan 08, 2018 at 06:55:30PM +0000, Marc Zyngier 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.
> 
> ...because that would defeat the overall purpose of what we're trying to
> do?

Indeed. Even direct branches could potentially be unsafe, so branching
at that stage is not a good idea.

> 
>>
>> Another thing is that we perform the invalidation on all
>> implementations, no matter if they are affected or not.
> 
> I don't understand this comment, it seems like the logic below is
> limited to A12 and A17 for now?

Ah, that's a leftover from the first version (before I discovered that
A15 needed a different method). I'll amend that when I repost the patches.

Thanks,

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-11  9:03         ` Marc Zyngier
@ 2018-01-29 18:41           ` Fabio Estevam
  2018-01-29 19:21             ` Fabio Estevam
  0 siblings, 1 reply; 32+ messages in thread
From: Fabio Estevam @ 2018-01-29 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marc,

On Thu, Jan 11, 2018 at 7:03 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:

>> OK. in u-boot, I had helped on the following:
>> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=5902f4ce0f2bd1411e40dc0ece3598a0fc19b2ae
>>
>> maybe be build off that?
>
> Turn that into something unconditional, and you'll be good.

Do you mean like this?

--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -249,7 +249,6 @@ skip_errata_801819:
        pop     {r1-r5}                 @ Restore the cpu info - fall through
 #endif

-#ifdef CONFIG_ARM_ERRATA_430973
        mrc     p15, 0, r0, c1, c0, 1   @ Read ACR

        cmp     r2, #0x21               @ Only on < r2p1
@@ -258,7 +257,6 @@ skip_errata_801819:
        push    {r1-r5}                 @ Save the cpu info registers
        bl      v7_arch_cp15_set_acr
        pop     {r1-r5}                 @ Restore the cpu info - fall through
-#endif

 #ifdef CONFIG_ARM_ERRATA_621766
        mrc     p15, 0, r0, c1, c0, 1   @ Read ACR

Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-29 18:41           ` Fabio Estevam
@ 2018-01-29 19:21             ` Fabio Estevam
  2018-01-29 19:28               ` Fabio Estevam
  0 siblings, 1 reply; 32+ messages in thread
From: Fabio Estevam @ 2018-01-29 19:21 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marc,

On Mon, Jan 29, 2018 at 4:41 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Marc,
>
> On Thu, Jan 11, 2018 at 7:03 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
>>> OK. in u-boot, I had helped on the following:
>>> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=5902f4ce0f2bd1411e40dc0ece3598a0fc19b2ae
>>>
>>> maybe be build off that?
>>
>> Turn that into something unconditional, and you'll be good.

Maybe by 'unconditional' you mean the revision check like this?

--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -249,16 +249,13 @@ skip_errata_801819:
        pop     {r1-r5}                 @ Restore the cpu info - fall through
 #endif

-#ifdef CONFIG_ARM_ERRATA_430973
        mrc     p15, 0, r0, c1, c0, 1   @ Read ACR

-       cmp     r2, #0x21               @ Only on < r2p1
-       orrlt   r0, r0, #(0x1 << 6)     @ Set IBE bit
+       orr     r0, r0, #(0x1 << 6)     @ Set IBE bit

        push    {r1-r5}                 @ Save the cpu info registers
        bl      v7_arch_cp15_set_acr
        pop     {r1-r5}                 @ Restore the cpu info - fall through
-#endif

 #ifdef CONFIG_ARM_ERRATA_621766

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17
  2018-01-29 19:21             ` Fabio Estevam
@ 2018-01-29 19:28               ` Fabio Estevam
  0 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2018-01-29 19:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 29, 2018 at 5:21 PM, Fabio Estevam <festevam@gmail.com> wrote:

> Maybe by 'unconditional' you mean the revision check like this?

Ok, just noticed that Nishanth has posted a patch for U-Boot:
https://patchwork.ozlabs.org/patch/866033/

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2018-01-29 19:28 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 18:55 [PATCH v2 0/6] ARM branch predictor hardening Marc Zyngier
2018-01-08 18:55 ` [PATCH v2 1/6] arm: Add BTB invalidation on switch_mm for Cortex-A9, A12 and A17 Marc Zyngier
2018-01-09 14:14   ` Andre Przywara
2018-01-09 14:21     ` Marc Zyngier
2018-01-09 14:22       ` Marc Zyngier
2018-01-10 17:53   ` Tony Lindgren
2018-01-10 17:57     ` Marc Zyngier
2018-01-10 21:52       ` Nishanth Menon
2018-01-11  9:03         ` Marc Zyngier
2018-01-29 18:41           ` Fabio Estevam
2018-01-29 19:21             ` Fabio Estevam
2018-01-29 19:28               ` Fabio Estevam
2018-01-08 18:55 ` [PATCH v2 2/6] arm: Invalidate BTB on prefetch abort outside of user mapping on Cortex A8, A9, " Marc Zyngier
2018-01-09  9:56   ` Marc Zyngier
2018-01-10 16:45   ` Russell King - ARM Linux
2018-01-08 18:55 ` [PATCH v2 3/6] arm: KVM: Invalidate BTB on guest exit Marc Zyngier
2018-01-15 12:22   ` Robin Murphy
2018-01-23 14:22   ` Christoffer Dall
2018-01-23 14:38     ` Marc Zyngier
2018-01-08 18:55 ` [PATCH v2 4/6] arm: Add icache invalidation on switch_mm for Cortex-A15 Marc Zyngier
2018-01-10  1:28   ` Florian Fainelli
2018-01-10  1:33     ` André Przywara
2018-01-10  1:35       ` Florian Fainelli
2018-01-10  9:13         ` Andre Przywara
2018-01-08 18:55 ` [PATCH v2 5/6] arm: Invalidate icache on prefetch abort outside of user mapping on Cortex-A15 Marc Zyngier
2018-01-08 18:55 ` [PATCH v2 6/6] arm: KVM: Invalidate icache on guest exit for Cortex-A15 Marc Zyngier
2018-01-23 14:26   ` Christoffer Dall
2018-01-09 17:13 ` [PATCH v2 0/6] ARM branch predictor hardening Florian Fainelli
2018-01-09 17:46   ` Russell King - ARM Linux
2018-01-10 16:50 ` Nishanth Menon
2018-01-10 17:16   ` Marc Zyngier
2018-01-10 22:59 ` Nishanth Menon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.