* [PATCH 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
@ 2016-08-12 15:27 ` Catalin Marinas
2016-08-12 15:27 ` [PATCH 2/7] arm64: Factor out TTBR0_EL1 setting into a specific asm macro Catalin Marinas
` (8 subsequent siblings)
9 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
This patch moves the directly coded alternatives for turning PAN on/off
into separate uaccess_{enable,disable} macros or functions. The asm
macros take a few arguments which will be used in subsequent patches.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 20 ++++++++++++++++++++
arch/arm64/include/asm/futex.h | 14 +++++++-------
arch/arm64/include/asm/uaccess.h | 27 +++++++++++++++++++--------
arch/arm64/kernel/armv8_deprecated.c | 10 +++++-----
arch/arm64/lib/clear_user.S | 7 ++-----
arch/arm64/lib/copy_from_user.S | 7 ++-----
arch/arm64/lib/copy_in_user.S | 7 ++-----
arch/arm64/lib/copy_to_user.S | 7 ++-----
8 files changed, 59 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index d5025c69ca81..bbed373f4ab7 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -23,6 +23,7 @@
#ifndef __ASM_ASSEMBLER_H
#define __ASM_ASSEMBLER_H
+#include <asm/alternative.h>
#include <asm/asm-offsets.h>
#include <asm/cpufeature.h>
#include <asm/page.h>
@@ -350,4 +351,23 @@ alternative_endif
movk \reg, :abs_g0_nc:\val
.endm
+/*
+ * User access enabling/disabling macros.
+ */
+ .macro uaccess_disable, tmp1
+alternative_if_not ARM64_ALT_PAN_NOT_UAO
+ nop
+alternative_else
+ SET_PSTATE_PAN(1)
+alternative_endif
+ .endm
+
+ .macro uaccess_enable, tmp1, tmp2, flags, errata = 0
+alternative_if_not ARM64_ALT_PAN_NOT_UAO
+ nop
+alternative_else
+ SET_PSTATE_PAN(0)
+alternative_endif
+ .endm
+
#endif /* __ASM_ASSEMBLER_H */
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index f2585cdd32c2..7e5f236093be 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -27,9 +27,9 @@
#include <asm/sysreg.h>
#define __futex_atomic_op(insn, ret, oldval, uaddr, tmp, oparg) \
+do { \
+ uaccess_enable(ARM64_HAS_PAN); \
asm volatile( \
- ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
" prfm pstl1strm, %2\n" \
"1: ldxr %w1, %2\n" \
insn "\n" \
@@ -44,11 +44,11 @@
" .popsection\n" \
_ASM_EXTABLE(1b, 4b) \
_ASM_EXTABLE(2b, 4b) \
- ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
: "=&r" (ret), "=&r" (oldval), "+Q" (*uaddr), "=&r" (tmp) \
: "r" (oparg), "Ir" (-EFAULT) \
- : "memory")
+ : "memory"); \
+ uaccess_disable(ARM64_HAS_PAN); \
+} while (0)
static inline int
futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
@@ -118,8 +118,8 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
return -EFAULT;
+ uaccess_enable(ARM64_HAS_PAN);
asm volatile("// futex_atomic_cmpxchg_inatomic\n"
-ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
" prfm pstl1strm, %2\n"
"1: ldxr %w1, %2\n"
" sub %w3, %w1, %w4\n"
@@ -134,10 +134,10 @@ ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
" .popsection\n"
_ASM_EXTABLE(1b, 4b)
_ASM_EXTABLE(2b, 4b)
-ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
: "+r" (ret), "=&r" (val), "+Q" (*uaddr), "=&r" (tmp)
: "r" (oldval), "r" (newval), "Ir" (-EFAULT)
: "memory");
+ uaccess_disable(ARM64_HAS_PAN);
*uval = val;
return ret;
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 5e834d10b291..f04869630207 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -112,6 +112,21 @@ static inline void set_fs(mm_segment_t fs)
" .popsection\n"
/*
+ * User access enabling/disabling.
+ */
+#define uaccess_disable(alt) \
+do { \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
+ CONFIG_ARM64_PAN)); \
+} while (0)
+
+#define uaccess_enable(alt) \
+do { \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
+ CONFIG_ARM64_PAN)); \
+} while (0)
+
+/*
* The "__xxx" versions of the user access functions do not verify the address
* space - it must have been done previously with a separate "access_ok()"
* call.
@@ -138,8 +153,7 @@ static inline void set_fs(mm_segment_t fs)
do { \
unsigned long __gu_val; \
__chk_user_ptr(ptr); \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_enable(ARM64_ALT_PAN_NOT_UAO); \
switch (sizeof(*(ptr))) { \
case 1: \
__get_user_asm("ldrb", "ldtrb", "%w", __gu_val, (ptr), \
@@ -160,9 +174,8 @@ do { \
default: \
BUILD_BUG(); \
} \
+ uaccess_disable(ARM64_ALT_PAN_NOT_UAO); \
(x) = (__force __typeof__(*(ptr)))__gu_val; \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
} while (0)
#define __get_user(x, ptr) \
@@ -207,8 +220,7 @@ do { \
do { \
__typeof__(*(ptr)) __pu_val = (x); \
__chk_user_ptr(ptr); \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_enable(ARM64_ALT_PAN_NOT_UAO); \
switch (sizeof(*(ptr))) { \
case 1: \
__put_user_asm("strb", "sttrb", "%w", __pu_val, (ptr), \
@@ -229,8 +241,7 @@ do { \
default: \
BUILD_BUG(); \
} \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_disable(ARM64_ALT_PAN_NOT_UAO); \
} while (0)
#define __put_user(x, ptr) \
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 42ffdb54e162..3aaf2fafbc8a 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -281,9 +281,9 @@ static void __init register_insn_emulation_sysctl(struct ctl_table *table)
* Error-checking SWP macros implemented using ldxr{b}/stxr{b}
*/
#define __user_swpX_asm(data, addr, res, temp, B) \
+do { \
+ uaccess_enable(ARM64_HAS_PAN); \
__asm__ __volatile__( \
- ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
"0: ldxr"B" %w2, [%3]\n" \
"1: stxr"B" %w0, %w1, [%3]\n" \
" cbz %w0, 2f\n" \
@@ -299,11 +299,11 @@ static void __init register_insn_emulation_sysctl(struct ctl_table *table)
" .popsection" \
_ASM_EXTABLE(0b, 4b) \
_ASM_EXTABLE(1b, 4b) \
- ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
: "=&r" (res), "+r" (data), "=&r" (temp) \
: "r" (addr), "i" (-EAGAIN), "i" (-EFAULT) \
- : "memory")
+ : "memory"); \
+ uaccess_disable(ARM64_HAS_PAN); \
+} while (0)
#define __user_swp_asm(data, addr, res, temp) \
__user_swpX_asm(data, addr, res, temp, "")
diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index 5d1cad3ce6d6..10d7501a5ce1 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -17,7 +17,6 @@
*/
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
@@ -33,8 +32,7 @@
* Alignment fixed up by hardware.
*/
ENTRY(__clear_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable x2, x3, x4
mov x2, x1 // save the size for fixup return
subs x1, x1, #8
b.mi 2f
@@ -54,8 +52,7 @@ uao_user_alternative 9f, strh, sttrh, wzr, x0, 2
b.mi 5f
uao_user_alternative 9f, strb, sttrb, wzr, x0, 0
5: mov x0, #0
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable x2
ret
ENDPROC(__clear_user)
diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
index 0b90497d4424..0ba4147fdff8 100644
--- a/arch/arm64/lib/copy_from_user.S
+++ b/arch/arm64/lib/copy_from_user.S
@@ -16,7 +16,6 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
@@ -67,12 +66,10 @@
end .req x5
ENTRY(__arch_copy_from_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable x3, x4, x6
add end, x0, x2
#include "copy_template.S"
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable x3
mov x0, #0 // Nothing to copy
ret
ENDPROC(__arch_copy_from_user)
diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S
index f7292dd08c84..ecbdaf6aacb8 100644
--- a/arch/arm64/lib/copy_in_user.S
+++ b/arch/arm64/lib/copy_in_user.S
@@ -18,7 +18,6 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
@@ -68,12 +67,10 @@
end .req x5
ENTRY(__copy_in_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable x3, x4, x6
add end, x0, x2
#include "copy_template.S"
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable x3
mov x0, #0
ret
ENDPROC(__copy_in_user)
diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
index 7a7efe255034..5ae01cbbf333 100644
--- a/arch/arm64/lib/copy_to_user.S
+++ b/arch/arm64/lib/copy_to_user.S
@@ -16,7 +16,6 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
@@ -66,12 +65,10 @@
end .req x5
ENTRY(__arch_copy_to_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable x3, x4, x6
add end, x0, x2
#include "copy_template.S"
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable x3
mov x0, #0
ret
ENDPROC(__arch_copy_to_user)
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 2/7] arm64: Factor out TTBR0_EL1 setting into a specific asm macro
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
2016-08-12 15:27 ` [PATCH 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros Catalin Marinas
@ 2016-08-12 15:27 ` Catalin Marinas
2016-08-12 15:27 ` [PATCH 3/7] arm64: Introduce uaccess_{disable, enable} functionality based on TTBR0_EL1 Catalin Marinas
` (7 subsequent siblings)
9 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
This patch takes the TTBR0_EL1 setting code out of cpu_do_switch_mm into
a dedicated cpu_set_ttbr0 macro which will be reused in a subsequent
patch.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 25 +++++++++++++++++++++++++
arch/arm64/mm/proc.S | 16 +---------------
2 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index bbed373f4ab7..039db634a693 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -352,6 +352,31 @@ alternative_endif
.endm
/*
+ * TTBR0_EL1 update macro.
+ */
+ .macro cpu_set_ttbr0, ttbr0, errata = 0, ret = 0
+ msr ttbr0_el1, \ttbr0
+ isb
+ .if \errata
+alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
+ .if \ret
+ ret
+ .endif
+ nop
+ nop
+ nop
+alternative_else
+ ic iallu
+ dsb nsh
+ isb
+ .if \ret
+ ret
+ .endif
+alternative_endif
+ .endif
+ .endm
+
+/*
* User access enabling/disabling macros.
*/
.macro uaccess_disable, tmp1
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5bb61de23201..442ade0f44eb 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -25,8 +25,6 @@
#include <asm/hwcap.h>
#include <asm/pgtable.h>
#include <asm/pgtable-hwdef.h>
-#include <asm/cpufeature.h>
-#include <asm/alternative.h>
#ifdef CONFIG_ARM64_64K_PAGES
#define TCR_TG_FLAGS TCR_TG0_64K | TCR_TG1_64K
@@ -123,19 +121,7 @@ ENDPROC(cpu_do_resume)
ENTRY(cpu_do_switch_mm)
mmid x1, x1 // get mm->context.id
bfi x0, x1, #48, #16 // set the ASID
- msr ttbr0_el1, x0 // set TTBR0
- isb
-alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
- ret
- nop
- nop
- nop
-alternative_else
- ic iallu
- dsb nsh
- isb
- ret
-alternative_endif
+ cpu_set_ttbr0 x0, errata = 1, ret = 1
ENDPROC(cpu_do_switch_mm)
.pushsection ".idmap.text", "ax"
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 3/7] arm64: Introduce uaccess_{disable, enable} functionality based on TTBR0_EL1
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
2016-08-12 15:27 ` [PATCH 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros Catalin Marinas
2016-08-12 15:27 ` [PATCH 2/7] arm64: Factor out TTBR0_EL1 setting into a specific asm macro Catalin Marinas
@ 2016-08-12 15:27 ` Catalin Marinas
2016-08-12 15:27 ` [PATCH 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution Catalin Marinas
` (6 subsequent siblings)
9 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds the uaccess macros/functions to disable access to user
space by setting TTBR0_EL1 to a reserved zeroed page. Since the value
written to TTBR0_EL1 must be a physical address, for simplicity this
path introduces a reserved_ttbr0 page at a constant offset from
swapper_pg_dir. The uaccess_disable code uses the ttbr1_el1 value
adjusted by the reserved_ttbr0 offset.
Enabling access to user is done by restoring TTBR0_EL1 with the value
from saved_ttbr0_el1 per-CPU variable. Interrupts are disabled during
the uaccess_enable code to ensure the atomicity of the saved_ttbr0_el1
read and TTBR0_EL1 write.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 63 +++++++++++++++++++++++++++++++--
arch/arm64/include/asm/cpufeature.h | 6 ++++
arch/arm64/include/asm/kernel-pgtable.h | 7 ++++
arch/arm64/include/asm/uaccess.h | 38 +++++++++++++++++---
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/head.S | 6 ++--
arch/arm64/kernel/vmlinux.lds.S | 5 +++
arch/arm64/mm/context.c | 13 +++++++
8 files changed, 130 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 039db634a693..45545393f605 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -26,6 +26,7 @@
#include <asm/alternative.h>
#include <asm/asm-offsets.h>
#include <asm/cpufeature.h>
+#include <asm/kernel-pgtable.h>
#include <asm/page.h>
#include <asm/pgtable-hwdef.h>
#include <asm/ptrace.h>
@@ -42,6 +43,15 @@
msr daifclr, #2
.endm
+ .macro save_and_disable_irq, flags
+ mrs \flags, daif
+ msr daifset, #2
+ .endm
+
+ .macro restore_irq, flags
+ msr daif, \flags
+ .endm
+
/*
* Enable and disable debug exceptions.
*/
@@ -195,7 +205,7 @@ lr .req x30 // link register
/*
* @sym: The name of the per-cpu variable
- * @reg: Result of per_cpu(sym, smp_processor_id())
+ * @reg: Result of this_cpu_ptr(sym)
* @tmp: scratch register
*/
.macro this_cpu_ptr, sym, reg, tmp
@@ -204,6 +214,17 @@ lr .req x30 // link register
add \reg, \reg, \tmp
.endm
+ /*
+ * @sym: The name of the per-cpu variable
+ * @reg: Result of this_cpu_read(sym)
+ * @tmp: scratch register
+ */
+ .macro this_cpu_read, sym, reg, tmp
+ adr_l \reg, \sym
+ mrs \tmp, tpidr_el1
+ ldr \reg, [\reg, \tmp]
+ .endm
+
/*
* vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
*/
@@ -379,7 +400,28 @@ alternative_endif
/*
* User access enabling/disabling macros.
*/
+ .macro uaccess_ttbr0_disable, tmp1
+ mrs \tmp1, ttbr1_el1 // swapper_pg_dir
+ add \tmp1, \tmp1, #SWAPPER_DIR_SIZE // reserved_ttbr0 at the end of swapper_pg_dir
+ cpu_set_ttbr0 \tmp1 // set reserved TTBR0_EL1
+ .endm
+
+ .macro uaccess_ttbr0_enable, tmp1, tmp2, errata = 0
+ this_cpu_read saved_ttbr0_el1 \tmp1, \tmp2
+ cpu_set_ttbr0 \tmp1, errata = \errata
+ .endm
+
.macro uaccess_disable, tmp1
+#ifdef CONFIG_ARM64_TTBR0_PAN
+alternative_if_not ARM64_HAS_PAN
+ uaccess_ttbr0_disable \tmp1
+alternative_else
+ nop
+ nop
+ nop
+ nop
+alternative_endif
+#endif
alternative_if_not ARM64_ALT_PAN_NOT_UAO
nop
alternative_else
@@ -387,7 +429,24 @@ alternative_else
alternative_endif
.endm
- .macro uaccess_enable, tmp1, tmp2, flags, errata = 0
+ .macro uaccess_enable, tmp1, tmp2, tmp3
+#ifdef CONFIG_ARM64_TTBR0_PAN
+alternative_if_not ARM64_HAS_PAN
+ save_and_disable_irq \tmp3 // avoid preemption
+ uaccess_ttbr0_enable \tmp1, \tmp2
+ restore_irq \tmp3
+alternative_else
+ nop
+ nop
+ nop
+ nop
+ nop
+ nop
+ nop
+ nop
+ nop
+alternative_endif
+#endif
alternative_if_not ARM64_ALT_PAN_NOT_UAO
nop
alternative_else
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 7099f26e3702..023066d9bf7f 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -216,6 +216,12 @@ static inline bool system_supports_mixed_endian_el0(void)
return id_aa64mmfr0_mixed_endian_el0(read_system_reg(SYS_ID_AA64MMFR0_EL1));
}
+static inline bool system_supports_ttbr0_pan(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_TTBR0_PAN) &&
+ !cpus_have_cap(ARM64_HAS_PAN);
+}
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
index 7e51d1b57c0c..f825ffda9c52 100644
--- a/arch/arm64/include/asm/kernel-pgtable.h
+++ b/arch/arm64/include/asm/kernel-pgtable.h
@@ -19,6 +19,7 @@
#ifndef __ASM_KERNEL_PGTABLE_H
#define __ASM_KERNEL_PGTABLE_H
+#include <asm/pgtable.h>
#include <asm/sparsemem.h>
/*
@@ -54,6 +55,12 @@
#define SWAPPER_DIR_SIZE (SWAPPER_PGTABLE_LEVELS * PAGE_SIZE)
#define IDMAP_DIR_SIZE (IDMAP_PGTABLE_LEVELS * PAGE_SIZE)
+#ifdef CONFIG_ARM64_TTBR0_PAN
+#define RESERVED_TTBR0_SIZE (PAGE_SIZE)
+#else
+#define RESERVED_TTBR0_SIZE (0)
+#endif
+
/* Initial memory map size */
#if ARM64_SWAPPER_USES_SECTION_MAPS
#define SWAPPER_BLOCK_SHIFT SECTION_SHIFT
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index f04869630207..e0eccdfd2427 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -22,11 +22,13 @@
* User space memory access functions
*/
#include <linux/kasan-checks.h>
+#include <linux/percpu.h>
#include <linux/string.h>
#include <linux/thread_info.h>
#include <asm/alternative.h>
#include <asm/cpufeature.h>
+#include <asm/kernel-pgtable.h>
#include <asm/ptrace.h>
#include <asm/sysreg.h>
#include <asm/errno.h>
@@ -114,16 +116,44 @@ static inline void set_fs(mm_segment_t fs)
/*
* User access enabling/disabling.
*/
+DECLARE_PER_CPU(u64, saved_ttbr0_el1);
+
+static inline void uaccess_ttbr0_disable(void)
+{
+ unsigned long ttbr;
+
+ ttbr = read_sysreg(ttbr1_el1) + SWAPPER_DIR_SIZE;
+ write_sysreg(ttbr, ttbr0_el1);
+ isb();
+}
+
+static inline void uaccess_ttbr0_enable(void)
+{
+ unsigned long ttbr, flags;
+
+ local_irq_save(flags);
+ ttbr = per_cpu(saved_ttbr0_el1, smp_processor_id());
+ write_sysreg(ttbr, ttbr0_el1);
+ isb();
+ local_irq_restore(flags);
+}
+
#define uaccess_disable(alt) \
do { \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
- CONFIG_ARM64_PAN)); \
+ if (system_supports_ttbr0_pan()) \
+ uaccess_ttbr0_disable(); \
+ else \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
+ CONFIG_ARM64_PAN)); \
} while (0)
#define uaccess_enable(alt) \
do { \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
- CONFIG_ARM64_PAN)); \
+ if (system_supports_ttbr0_pan()) \
+ uaccess_ttbr0_enable(); \
+ else \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
+ CONFIG_ARM64_PAN)); \
} while (0)
/*
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 62272eac1352..fd0971afd142 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -45,6 +45,7 @@ unsigned int compat_elf_hwcap2 __read_mostly;
#endif
DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
+EXPORT_SYMBOL(cpu_hwcaps);
#define __ARM64_FTR_BITS(SIGNED, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
{ \
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index b77f58355da1..57ae28e4d8de 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -320,14 +320,14 @@ __create_page_tables:
* dirty cache lines being evicted.
*/
mov x0, x25
- add x1, x26, #SWAPPER_DIR_SIZE
+ add x1, x26, #SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE
bl __inval_cache_range
/*
* Clear the idmap and swapper page tables.
*/
mov x0, x25
- add x6, x26, #SWAPPER_DIR_SIZE
+ add x6, x26, #SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE
1: stp xzr, xzr, [x0], #16
stp xzr, xzr, [x0], #16
stp xzr, xzr, [x0], #16
@@ -406,7 +406,7 @@ __create_page_tables:
* tables again to remove any speculatively loaded cache lines.
*/
mov x0, x25
- add x1, x26, #SWAPPER_DIR_SIZE
+ add x1, x26, #SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE
dmb sy
bl __inval_cache_range
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 659963d40bb4..fe393ccf9352 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -196,6 +196,11 @@ SECTIONS
swapper_pg_dir = .;
. += SWAPPER_DIR_SIZE;
+#ifdef CONFIG_ARM64_TTBR0_PAN
+ reserved_ttbr0 = .;
+ . += PAGE_SIZE;
+#endif
+
_end = .;
STABS_DEBUG
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index efcf1f7ef1e4..f4bdee285774 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -37,6 +37,11 @@ static DEFINE_PER_CPU(atomic64_t, active_asids);
static DEFINE_PER_CPU(u64, reserved_asids);
static cpumask_t tlb_flush_pending;
+#ifdef CONFIG_ARM64_TTBR0_PAN
+DEFINE_PER_CPU(u64, saved_ttbr0_el1);
+EXPORT_PER_CPU_SYMBOL(saved_ttbr0_el1);
+#endif
+
#define ASID_MASK (~GENMASK(asid_bits - 1, 0))
#define ASID_FIRST_VERSION (1UL << asid_bits)
#define NUM_USER_ASIDS ASID_FIRST_VERSION
@@ -226,6 +231,8 @@ switch_mm_fastpath:
static int asids_init(void)
{
+ unsigned int cpu __maybe_unused;
+
asid_bits = get_cpu_asid_bits();
/*
* Expect allocation after rollover to fail if we don't have@least
@@ -239,6 +246,12 @@ static int asids_init(void)
panic("Failed to allocate bitmap for %lu ASIDs\n",
NUM_USER_ASIDS);
+#ifdef CONFIG_ARM64_TTBR0_PAN
+ /* Initialise saved_ttbr0_el1 to the reserved TTBR0 and ASID */
+ for_each_possible_cpu(cpu)
+ per_cpu(saved_ttbr0_el1, cpu) = virt_to_phys(empty_zero_page);
+#endif
+
pr_info("ASID allocator initialised with %lu entries\n", NUM_USER_ASIDS);
return 0;
}
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
` (2 preceding siblings ...)
2016-08-12 15:27 ` [PATCH 3/7] arm64: Introduce uaccess_{disable, enable} functionality based on TTBR0_EL1 Catalin Marinas
@ 2016-08-12 15:27 ` Catalin Marinas
2016-08-15 11:18 ` Mark Rutland
2016-08-12 15:27 ` [PATCH 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled Catalin Marinas
` (5 subsequent siblings)
9 siblings, 1 reply; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
When the TTBR0 PAN feature is enabled, the kernel entry points need to
disable access to TTBR0_EL1. The PAN status of the interrupted context
is stored as part of the saved pstate, reusing the PSR_PAN_BIT (22).
Restoring access to TTBR0_PAN is done on exception return if returning
to user or returning to a context where PAN was disabled.
Context switching via switch_mm() must defer the update of TTBR0_EL1
until a return to user or an explicit uaccess_enable() call.
Special care needs to be taken for two cases where TTBR0_EL1 is set
outside the normal kernel context switch operation: EFI run-time
services (via efi_set_pgd) and CPU suspend (via cpu_(un)install_idmap).
Code has been added to avoid deferred TTBR0_EL1 switching as in
switch_mm() and restore the reserved TTBR0_EL1 when uninstalling the
special TTBR0_EL1.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/efi.h | 14 ++++++++
arch/arm64/include/asm/mmu_context.h | 3 +-
arch/arm64/include/uapi/asm/ptrace.h | 2 ++
arch/arm64/kernel/entry.S | 62 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/suspend.c | 12 +++----
arch/arm64/mm/context.c | 12 ++++++-
6 files changed, 97 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a9e54aad15ef..1d7810b88255 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -1,6 +1,7 @@
#ifndef _ASM_EFI_H
#define _ASM_EFI_H
+#include <asm/cpufeature.h>
#include <asm/io.h>
#include <asm/mmu_context.h>
#include <asm/neon.h>
@@ -76,6 +77,19 @@ static inline void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
static inline void efi_set_pgd(struct mm_struct *mm)
{
switch_mm(NULL, mm, NULL);
+
+ /*
+ * Force TTBR0_EL1 setting. If restoring the active_mm pgd, defer the
+ * switching after uaccess_enable(). This code is calling
+ * cpu_switch_mm() directly (instead of uaccess_enable()) to force
+ * potential errata workarounds.
+ */
+ if (system_supports_ttbr0_pan()) {
+ if (mm != current->active_mm)
+ cpu_switch_mm(mm->pgd, mm);
+ else
+ cpu_set_reserved_ttbr0();
+ }
}
void efi_virtmap_load(void);
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index b1892a0dbcb0..7762125657bf 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -23,6 +23,7 @@
#include <linux/sched.h>
#include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
#include <asm/proc-fns.h>
#include <asm-generic/mm_hooks.h>
#include <asm/cputype.h>
@@ -113,7 +114,7 @@ static inline void cpu_uninstall_idmap(void)
local_flush_tlb_all();
cpu_set_default_tcr_t0sz();
- if (mm != &init_mm)
+ if (mm != &init_mm && !system_supports_ttbr0_pan())
cpu_switch_mm(mm->pgd, mm);
}
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index b5c3933ed441..9283e6b247f9 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -52,6 +52,8 @@
#define PSR_Z_BIT 0x40000000
#define PSR_N_BIT 0x80000000
+#define _PSR_PAN_BIT 22
+
/*
* Groups of PSR bits
*/
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 96e4a2b64cc1..b77034f0ffab 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -29,6 +29,7 @@
#include <asm/esr.h>
#include <asm/irq.h>
#include <asm/memory.h>
+#include <asm/ptrace.h>
#include <asm/thread_info.h>
#include <asm/unistd.h>
@@ -109,6 +110,37 @@
mrs x22, elr_el1
mrs x23, spsr_el1
stp lr, x21, [sp, #S_LR]
+
+#ifdef CONFIG_ARM64_TTBR0_PAN
+ /*
+ * Set the TTBR0 PAN in SPSR. When the exception is taken from EL0,
+ * there is no need to check the state of TTBR0_EL1 since accesses are
+ * always enabled.
+ * Note that the meaning of this bit differs from the ARMv8.1 PAN
+ * feature as all TTBR0_EL1 accesses are disabled, not just those to
+ * user mappings.
+ */
+alternative_if_not ARM64_HAS_PAN
+ nop
+alternative_else
+ b 1f // skip TTBR0 PAN
+alternative_endif
+
+ .if \el != 0
+ mrs lr, ttbr0_el1
+ tst lr, #0xffff << 48 // Check for the reserved ASID
+ orr x23, x23, #PSR_PAN_BIT
+ b.eq 1f // TTBR0 access already disabled
+ .endif
+
+ uaccess_ttbr0_disable x21
+
+ .if \el != 0
+ and x23, x23, #~PSR_PAN_BIT // TTBR0 access previously enabled
+ .endif
+1:
+#endif
+
stp x22, x23, [sp, #S_PC]
/*
@@ -168,6 +200,36 @@ alternative_else
alternative_endif
#endif
.endif
+
+#ifdef CONFIG_ARM64_TTBR0_PAN
+ /*
+ * Restore access to TTBR0_EL1. If returning to EL0, no need for SPSR
+ * PAN bit checking.
+ */
+alternative_if_not ARM64_HAS_PAN
+ nop
+alternative_else
+ b 2f // skip TTBR0 PAN
+alternative_endif
+
+ .if \el != 0
+ tbnz x22, #_PSR_PAN_BIT, 1f // Only re-enable TTBR0 access if SPSR.PAN == 0
+ .endif
+
+ /*
+ * Enable errata workarounds only if returning to user. The only
+ * workaround currently required for TTBR0_EL1 changes are for the
+ * Cavium erratum 27456 (broadcast TLBI instructions may cause I-cache
+ * corruption).
+ */
+ uaccess_ttbr0_enable x0, x1, errata = \el == 0
+
+ .if \el != 0
+1: and x22, x22, #~PSR_PAN_BIT // ARMv8.0 CPUs do not understand this bit
+ .endif
+2:
+#endif
+
msr elr_el1, x21 // set up the return data
msr spsr_el1, x22
ldp x0, x1, [sp, #16 * 0]
diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
index b616e365cee3..e10993bcaf13 100644
--- a/arch/arm64/kernel/suspend.c
+++ b/arch/arm64/kernel/suspend.c
@@ -35,6 +35,12 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
void notrace __cpu_suspend_exit(void)
{
/*
+ * Restore per-cpu offset before any kernel
+ * subsystem relying on it has a chance to run.
+ */
+ set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
+
+ /*
* We are resuming from reset with the idmap active in TTBR0_EL1.
* We must uninstall the idmap and restore the expected MMU
* state before we can possibly return to userspace.
@@ -42,12 +48,6 @@ void notrace __cpu_suspend_exit(void)
cpu_uninstall_idmap();
/*
- * Restore per-cpu offset before any kernel
- * subsystem relying on it has a chance to run.
- */
- set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
-
- /*
* Restore HW breakpoint registers to sane values
* before debug exceptions are possibly reenabled
* through local_dbg_restore.
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index f4bdee285774..f7406bd5eb7c 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -226,7 +226,17 @@ void check_and_switch_context(struct mm_struct *mm, unsigned int cpu)
raw_spin_unlock_irqrestore(&cpu_asid_lock, flags);
switch_mm_fastpath:
- cpu_switch_mm(mm->pgd, mm);
+#ifdef CONFIG_ARM64_TTBR0_PAN
+ /*
+ * Defer TTBR0_EL1 setting for user tasks to uaccess_enable() when
+ * emulating PAN.
+ */
+ if (system_supports_ttbr0_pan())
+ __this_cpu_write(saved_ttbr0_el1,
+ virt_to_phys(mm->pgd) | asid << 48);
+ else
+#endif
+ cpu_switch_mm(mm->pgd, mm);
}
static int asids_init(void)
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution
2016-08-12 15:27 ` [PATCH 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution Catalin Marinas
@ 2016-08-15 11:18 ` Mark Rutland
2016-08-15 16:39 ` Catalin Marinas
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2016-08-15 11:18 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Aug 12, 2016 at 04:27:43PM +0100, Catalin Marinas wrote:
> diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> index b5c3933ed441..9283e6b247f9 100644
> --- a/arch/arm64/include/uapi/asm/ptrace.h
> +++ b/arch/arm64/include/uapi/asm/ptrace.h
> @@ -52,6 +52,8 @@
> #define PSR_Z_BIT 0x40000000
> #define PSR_N_BIT 0x80000000
>
> +#define _PSR_PAN_BIT 22
Given this is under uapi/, shouldn't we lose the leading underscore to align
with other PSR_* definitions?
Or should we not have this under uapi/?
[...]
> + mrs lr, ttbr0_el1
> + tst lr, #0xffff << 48 // Check for the reserved ASID
Did we not have a regular register spare here? Not a problem, but using the lr
here stands out as unusual.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution
2016-08-15 11:18 ` Mark Rutland
@ 2016-08-15 16:39 ` Catalin Marinas
0 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-15 16:39 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:18:58PM +0100, Mark Rutland wrote:
> On Fri, Aug 12, 2016 at 04:27:43PM +0100, Catalin Marinas wrote:
> > diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> > index b5c3933ed441..9283e6b247f9 100644
> > --- a/arch/arm64/include/uapi/asm/ptrace.h
> > +++ b/arch/arm64/include/uapi/asm/ptrace.h
> > @@ -52,6 +52,8 @@
> > #define PSR_Z_BIT 0x40000000
> > #define PSR_N_BIT 0x80000000
> >
> > +#define _PSR_PAN_BIT 22
>
> Given this is under uapi/, shouldn't we lose the leading underscore to align
> with other PSR_* definitions?
>
> Or should we not have this under uapi/?
I moved it to the non-uapi ptrace.h.
> [...]
>
> > + mrs lr, ttbr0_el1
> > + tst lr, #0xffff << 48 // Check for the reserved ASID
>
> Did we not have a regular register spare here? Not a problem, but using the lr
> here stands out as unusual.
LR is a general purpose register, we just have an alias for it. I've
replaced it with x21 so that it doesn't stand out.
--
Catalin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
` (3 preceding siblings ...)
2016-08-12 15:27 ` [PATCH 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution Catalin Marinas
@ 2016-08-12 15:27 ` Catalin Marinas
2016-08-12 15:27 ` [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call Catalin Marinas
` (4 subsequent siblings)
9 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
When TTBR0_EL1 is set to the reserved page, an erroneous kernel access
to user space would generate a translation fault. This patch adds the
checks for the software-set PSR_PAN_BIT to emulate a permission fault
and report it accordingly.
This patch also updates the description of the synchronous external
aborts on translation table walks.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/mm/fault.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c8beaa0da7df..a3b2f9b63c55 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -262,12 +262,19 @@ out:
return fault;
}
-static inline bool is_permission_fault(unsigned int esr)
+static inline bool is_permission_fault(unsigned int esr, struct pt_regs *regs)
{
unsigned int ec = ESR_ELx_EC(esr);
unsigned int fsc_type = esr & ESR_ELx_FSC_TYPE;
- return (ec == ESR_ELx_EC_DABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
+ if (ec != ESR_ELx_EC_DABT_CUR)
+ return false;
+
+ if (system_supports_ttbr0_pan())
+ return fsc_type == ESR_ELx_FSC_FAULT &&
+ (regs->pstate & PSR_PAN_BIT);
+ else
+ return fsc_type == ESR_ELx_FSC_PERM;
}
static bool is_el0_instruction_abort(unsigned int esr)
@@ -307,7 +314,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
mm_flags |= FAULT_FLAG_WRITE;
}
- if (is_permission_fault(esr) && (addr < USER_DS)) {
+ if (addr < USER_DS && is_permission_fault(esr, regs)) {
/* regs->orig_addr_limit may be 0 if we entered from EL0 */
if (regs->orig_addr_limit == KERNEL_DS)
die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
@@ -496,10 +503,10 @@ static const struct fault_info {
{ do_bad, SIGBUS, 0, "unknown 17" },
{ 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 external abort (translation table walk)" },
+ { do_bad, SIGBUS, 0, "synchronous external abort (translation table walk)" },
+ { do_bad, SIGBUS, 0, "synchronous external abort (translation table walk)" },
+ { do_bad, SIGBUS, 0, "synchronous external abort (translation table walk)" },
{ do_bad, SIGBUS, 0, "synchronous parity error" },
{ do_bad, SIGBUS, 0, "unknown 25" },
{ do_bad, SIGBUS, 0, "unknown 26" },
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
` (4 preceding siblings ...)
2016-08-12 15:27 ` [PATCH 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled Catalin Marinas
@ 2016-08-12 15:27 ` Catalin Marinas
2016-08-15 9:58 ` Julien Grall
2016-08-12 15:27 ` [PATCH 7/7] arm64: Enable CONFIG_ARM64_TTBR0_PAN Catalin Marinas
` (3 subsequent siblings)
9 siblings, 1 reply; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
Privcmd calls are issued by the userspace. The kernel needs to enable
access to TTBR0_EL1 as the hypervisor would issue stage 1 translations
to user memory via AT instructions. Since AT instructions are not
affected by the PAN bit (ARMv8.1), we only need the explicit
uaccess_enable/disable if the TTBR0 PAN option is enabled.
Cc: Julien Grall <julien.grall@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/xen/hypercall.S | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 329c8027b0a9..4c509f4f4dcc 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -91,6 +91,24 @@ ENTRY(privcmd_call)
mov x2, x3
mov x3, x4
mov x4, x5
+#ifdef CONFIG_ARM64_TTBR0_PAN
+ /*
+ * Privcmd calls are issued by the userspace. The kernel needs to
+ * enable access to TTBR0_EL1 as the hypervisor would issue stage 1
+ * translations to user memory via AT instructions. Since AT
+ * instructions are not affected by the PAN bit (ARMv8.1), we only
+ * need the explicit uaccess_enable/disable if the TTBR0 PAN option is
+ * enabled.
+ */
+ uaccess_enable x6, x7, x8
+#endif
hvc XEN_IMM
+
+#ifdef CONFIG_ARM64_TTBR0_PAN
+ /*
+ * Disable userspace access from kernel once the hyp call completed.
+ */
+ uaccess_disable x6
+#endif
ret
ENDPROC(privcmd_call);
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call
2016-08-12 15:27 ` [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call Catalin Marinas
@ 2016-08-15 9:58 ` Julien Grall
2016-08-15 18:00 ` Stefano Stabellini
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2016-08-15 9:58 UTC (permalink / raw)
To: linux-arm-kernel
Hi Catalin,
I have CCed Stefano who is maintaining the Xen ARM code in Linux.
On 12/08/2016 17:27, Catalin Marinas wrote:
> Privcmd calls are issued by the userspace. The kernel needs to enable
> access to TTBR0_EL1 as the hypervisor would issue stage 1 translations
> to user memory via AT instructions. Since AT instructions are not
> affected by the PAN bit (ARMv8.1), we only need the explicit
> uaccess_enable/disable if the TTBR0 PAN option is enabled.
>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Regards,
> ---
> arch/arm64/xen/hypercall.S | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
> index 329c8027b0a9..4c509f4f4dcc 100644
> --- a/arch/arm64/xen/hypercall.S
> +++ b/arch/arm64/xen/hypercall.S
> @@ -91,6 +91,24 @@ ENTRY(privcmd_call)
> mov x2, x3
> mov x3, x4
> mov x4, x5
> +#ifdef CONFIG_ARM64_TTBR0_PAN
> + /*
> + * Privcmd calls are issued by the userspace. The kernel needs to
> + * enable access to TTBR0_EL1 as the hypervisor would issue stage 1
> + * translations to user memory via AT instructions. Since AT
> + * instructions are not affected by the PAN bit (ARMv8.1), we only
> + * need the explicit uaccess_enable/disable if the TTBR0 PAN option is
> + * enabled.
> + */
> + uaccess_enable x6, x7, x8
> +#endif
> hvc XEN_IMM
> +
> +#ifdef CONFIG_ARM64_TTBR0_PAN
> + /*
> + * Disable userspace access from kernel once the hyp call completed.
> + */
> + uaccess_disable x6
> +#endif
> ret
> ENDPROC(privcmd_call);
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
Julien Grall
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call
2016-08-15 9:58 ` Julien Grall
@ 2016-08-15 18:00 ` Stefano Stabellini
0 siblings, 0 replies; 37+ messages in thread
From: Stefano Stabellini @ 2016-08-15 18:00 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 15 Aug 2016, Julien Grall wrote:
> Hi Catalin,
>
> I have CCed Stefano who is maintaining the Xen ARM code in Linux.
>
> On 12/08/2016 17:27, Catalin Marinas wrote:
> > Privcmd calls are issued by the userspace. The kernel needs to enable
> > access to TTBR0_EL1 as the hypervisor would issue stage 1 translations
> > to user memory via AT instructions. Since AT instructions are not
> > affected by the PAN bit (ARMv8.1), we only need the explicit
> > uaccess_enable/disable if the TTBR0 PAN option is enabled.
> >
> > Cc: Julien Grall <julien.grall@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Reviewed-by: Julien Grall <julien.grall@arm.com>
>
> Regards,
>
> > ---
> > arch/arm64/xen/hypercall.S | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
> > index 329c8027b0a9..4c509f4f4dcc 100644
> > --- a/arch/arm64/xen/hypercall.S
> > +++ b/arch/arm64/xen/hypercall.S
> > @@ -91,6 +91,24 @@ ENTRY(privcmd_call)
> > mov x2, x3
> > mov x3, x4
> > mov x4, x5
> > +#ifdef CONFIG_ARM64_TTBR0_PAN
> > + /*
> > + * Privcmd calls are issued by the userspace. The kernel needs to
> > + * enable access to TTBR0_EL1 as the hypervisor would issue stage 1
> > + * translations to user memory via AT instructions. Since AT
> > + * instructions are not affected by the PAN bit (ARMv8.1), we only
> > + * need the explicit uaccess_enable/disable if the TTBR0 PAN option is
> > + * enabled.
That's a pity but it is how it is.
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Given that the patch is part of your PAN series, I expect that it is
going to go via your tree.
> > + uaccess_enable x6, x7, x8
> > +#endif
> > hvc XEN_IMM
> > +
> > +#ifdef CONFIG_ARM64_TTBR0_PAN
> > + /*
> > + * Disable userspace access from kernel once the hyp call completed.
> > + */
> > + uaccess_disable x6
> > +#endif
> > ret
> > ENDPROC(privcmd_call);
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
> --
> Julien Grall
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 7/7] arm64: Enable CONFIG_ARM64_TTBR0_PAN
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
` (5 preceding siblings ...)
2016-08-12 15:27 ` [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call Catalin Marinas
@ 2016-08-12 15:27 ` Catalin Marinas
2016-08-12 18:04 ` [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Kees Cook
` (2 subsequent siblings)
9 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds the Kconfig option to enable support for TTBR0 PAN. The
option is default off because of a slight performance hit when enabled,
caused by the additional TTBR0_EL1 switching during user access
operations or exception entry/exit code.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 69c8787bec7d..e7d5d0a7f6b4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -784,6 +784,14 @@ config SETEND_EMULATION
If unsure, say Y
endif
+config ARM64_TTBR0_PAN
+ bool "Priviledged Access Never using TTBR0_EL1 switching"
+ help
+ Enabling this option prevents the kernel from accessing
+ user-space memory directly by pointing TTBR0_EL1 to a reserved
+ zeroed area and reserved ASID. The user access routines
+ restore the valid TTBR0_EL1 temporarily.
+
menu "ARMv8.1 architectural features"
config ARM64_HW_AFDBM
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
` (6 preceding siblings ...)
2016-08-12 15:27 ` [PATCH 7/7] arm64: Enable CONFIG_ARM64_TTBR0_PAN Catalin Marinas
@ 2016-08-12 18:04 ` Kees Cook
2016-08-12 18:22 ` Catalin Marinas
2016-08-13 9:13 ` [kernel-hardening] " Ard Biesheuvel
2016-08-26 15:39 ` David Brown
9 siblings, 1 reply; 37+ messages in thread
From: Kees Cook @ 2016-08-12 18:04 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Aug 12, 2016 at 8:27 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> This is the first (public) attempt at emulating PAN by disabling
> TTBR0_EL1 accesses on arm64. I chose to use a per-CPU saved_ttbr0_el1
> variable to store the actual TTBR0 as, IMO, it looks better w.r.t. the
> context switching code, to the detriment of a slightly more complex
> uaccess_enable() implementation. The alternative was storing the saved
> TTBR0 in thread_info but with more complex thread switching since TTBR0
> is normally tied to switch_mm() rather than switch_to(). The latter may
> also get more complicated if we are to decouple the kernel stack from
> thread_info at some point (vmalloc'ed stacks).
>
> The code requires more testing, especially for combinations where UAO is
> present but PAN is not.
>
> The patches are also available on this branch:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux ttbr0-pan
>
> Thanks for reviewing/testing.
So awesome! Thank you for working on this. I still lack real arm64
hardware to test this on, but the lkdtm test ACCESS_USERSPACE should
trip this protection (e.g. this "cat" should get killed and the Oops
appear in dmesg):
# cat <(echo ACCESS_USERSPACE) > /sys/kernel/debug/provoke-crash/DIRECT
-Kees
>
> Catalin Marinas (7):
> arm64: Factor out PAN enabling/disabling into separate uaccess_*
> macros
> arm64: Factor out TTBR0_EL1 setting into a specific asm macro
> arm64: Introduce uaccess_{disable,enable} functionality based on
> TTBR0_EL1
> arm64: Disable TTBR0_EL1 during normal kernel execution
> arm64: Handle faults caused by inadvertent user access with PAN
> enabled
> arm64: xen: Enable user access before a privcmd hvc call
> arm64: Enable CONFIG_ARM64_TTBR0_PAN
>
> arch/arm64/Kconfig | 8 +++
> arch/arm64/include/asm/assembler.h | 106 +++++++++++++++++++++++++++++++-
> arch/arm64/include/asm/cpufeature.h | 6 ++
> arch/arm64/include/asm/efi.h | 14 +++++
> arch/arm64/include/asm/futex.h | 14 ++---
> arch/arm64/include/asm/kernel-pgtable.h | 7 +++
> arch/arm64/include/asm/mmu_context.h | 3 +-
> arch/arm64/include/asm/uaccess.h | 57 ++++++++++++++---
> arch/arm64/include/uapi/asm/ptrace.h | 2 +
> arch/arm64/kernel/armv8_deprecated.c | 10 +--
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/entry.S | 62 +++++++++++++++++++
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/suspend.c | 12 ++--
> arch/arm64/kernel/vmlinux.lds.S | 5 ++
> arch/arm64/lib/clear_user.S | 7 +--
> arch/arm64/lib/copy_from_user.S | 7 +--
> arch/arm64/lib/copy_in_user.S | 7 +--
> arch/arm64/lib/copy_to_user.S | 7 +--
> arch/arm64/mm/context.c | 25 +++++++-
> arch/arm64/mm/fault.c | 21 ++++---
> arch/arm64/mm/proc.S | 16 +----
> arch/arm64/xen/hypercall.S | 18 ++++++
> 23 files changed, 347 insertions(+), 74 deletions(-)
>
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-12 18:04 ` [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Kees Cook
@ 2016-08-12 18:22 ` Catalin Marinas
0 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-12 18:22 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Aug 12, 2016 at 11:04:39AM -0700, Kees Cook wrote:
> On Fri, Aug 12, 2016 at 8:27 AM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > This is the first (public) attempt at emulating PAN by disabling
> > TTBR0_EL1 accesses on arm64. I chose to use a per-CPU saved_ttbr0_el1
> > variable to store the actual TTBR0 as, IMO, it looks better w.r.t. the
> > context switching code, to the detriment of a slightly more complex
> > uaccess_enable() implementation. The alternative was storing the saved
> > TTBR0 in thread_info but with more complex thread switching since TTBR0
> > is normally tied to switch_mm() rather than switch_to(). The latter may
> > also get more complicated if we are to decouple the kernel stack from
> > thread_info at some point (vmalloc'ed stacks).
> >
> > The code requires more testing, especially for combinations where UAO is
> > present but PAN is not.
> >
> > The patches are also available on this branch:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux ttbr0-pan
> >
> > Thanks for reviewing/testing.
>
> So awesome! Thank you for working on this. I still lack real arm64
> hardware to test this on, but the lkdtm test ACCESS_USERSPACE should
> trip this protection (e.g. this "cat" should get killed and the Oops
> appear in dmesg):
>
> # cat <(echo ACCESS_USERSPACE) > /sys/kernel/debug/provoke-crash/DIRECT
It seems to work ;)
~# echo ACCESS_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
[ 51.918454] lkdtm: Performing direct entry ACCESS_USERSPACE
[ 51.924018] lkdtm: attempting bad read at 0000ffff8e165000
[ 51.929476] Internal error: Accessing user space memory outside uaccess.h routines: 96000004 [#1] PREEMPT SMP
[ 51.963729] Hardware name: ARM Juno development board (r0) (DT)
[ 51.969586] task: ffff8009763bf080 task.stack: ffff800973870000
[ 51.975451] PC is at lkdtm_ACCESS_USERSPACE+0xb0/0x100
[ 51.980536] LR is at lkdtm_ACCESS_USERSPACE+0xb0/0x100
[ 51.985619] pc : [<ffff000008518638>] lr : [<ffff000008518638>] pstate: 60400145
[ 51.992936] sp : ffff800973873cf0
[ 51.996212] x29: ffff800973873cf0 x28: ffff800973870000
[ 52.001474] x27: ffff000008872000 x26: 0000000000000040
[ 52.006737] x25: 0000000000000120 x24: ffff000008d31290
[ 52.011998] x23: ffff800973873eb8 x22: 0000000000000011
[ 52.017259] x21: ffff800973cea000 x20: ffff000008d31400
[ 52.022520] x19: 0000ffff8e165000 x18: 0000000000000006
[ 52.027781] x17: 0000ffff8dfe6770 x16: ffff0000081da450
[ 52.033043] x15: ffff000008d9bd15 x14: 0000000000000107
[ 52.038304] x13: 0000000000000000 x12: 0000000005f5e0ff
[ 52.043565] x11: 0000000000000002 x10: 0000000000000108
[ 52.048826] x9 : ffff800973873a80 x8 : 6666303030302074
[ 52.054087] x7 : 6120646165722064 x6 : 000000000000000a
[ 52.059349] x5 : 0000000000000000 x4 : 0000000000000000
[ 52.064610] x3 : 0000000000000000 x2 : ffff80097fed7728
[ 52.069871] x1 : ffff800973870000 x0 : 000000000000002e
[ 52.075131]
[ 52.076602] Process bash (pid: 2739, stack limit = 0xffff800973870020)
[ 52.083062] Stack: (0xffff800973873cf0 to 0xffff800973874000)
[ 52.088748] 3ce0: ffff800973873d20 ffff000008739954
[ 52.096500] 3d00: 0000000000000170 00000000000000ba ffff7e0025d09f60 0000000000000000
[ 52.104251] 3d20: ffff800973873d30 ffff000008517b00 ffff800973873d70 ffff000008329200
[ 52.112002] 3d40: 0000000000000000 ffff8009759dc400 0000000034996408 0000000000000011
[ 52.119753] 3d60: ffff800973873eb8 ffff000008d314e8 ffff800973873dc0 ffff0000081d830c
[ 52.127504] 3d80: 0000000000000011 ffff8009759dc400 0000000000000000 ffff800973873eb8
[ 52.135255] 3da0: 0000000034996408 0000000000000015 0000000000000400 0000000073966780
[ 52.143005] 3dc0: ffff800973873e40 ffff0000081d9120 0000000000000011 ffff8009759dc400
[ 52.150756] 3de0: ffff800974018900 000000000000000a ffff800973873e10 ffff0000080fd800
[ 52.158507] 3e00: ffff80097680e280 0000000000000001 ffff800973873e30 ffff0000081dcde8
[ 52.166257] 3e20: 0000000000000011 ffff8009759dc400 ffff800973873e40 ffff0000081d91e0
[ 52.174008] 3e40: ffff800973873e80 ffff0000081da494 ffff8009759dc400 ffff8009759dc400
[ 52.181759] 3e60: 0000000034996408 0000000000000011 0000000020000000 ffff000008083930
[ 52.189509] 3e80: 0000000000000000 ffff000008083930 0000000000000000 0000000034996408
[ 52.197260] 3ea0: ffffffffffffffff 0000ffff8e0398b8 0000000000000000 0000000000000000
[ 52.205010] 3ec0: 0000000000000001 0000000034996408 0000000000000011 0000000000000000
[ 52.212761] 3ee0: 00000000fbad2a85 0001555100045400 0000000034996418 555f535345434341
[ 52.220512] 3f00: 0000000000000040 0000ffff8e0c4000 0000fffff925de90 7f7f7f7f7f7f7f7f
[ 52.228263] 3f20: 0101010101010101 0000000000000005 ffffffffffffffff 0000000000000078
[ 52.236013] 3f40: 0000000000000000 0000ffff8dfe6770 0000000000000003 0000000000000011
[ 52.243764] 3f60: 0000000034996408 0000ffff8e0c0488 0000000000000011 00000000004d3af0
[ 52.251515] 3f80: 00000000004f6000 00000000004f3000 0000000000000001 0000000034a99ec8
[ 52.259265] 3fa0: 0000000034aeefe8 0000fffff925de40 0000ffff8dfe9558 0000fffff925de40
[ 52.267016] 3fc0: 0000ffff8e0398b8 0000000020000000 0000000000000001 0000000000000040
[ 52.274766] 3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 52.282513] Call trace:
[ 52.284931] Exception stack(0xffff800973873b20 to 0xffff800973873c50)
[ 52.291306] 3b20: 0000ffff8e165000 0001000000000000 ffff800973873cf0 ffff000008518638
[ 52.299057] 3b40: ffff000008ceaf20 0000000000000006 0000000000000000 0000000000000000
[ 52.306808] 3b60: 000000000000002e ffff000008d9b7d8 0000000000000002 ffff000008d9e128
[ 52.314559] 3b80: ffff800973873ba0 ffff000008abd6c8 000000000000002e 0000000100000001
[ 52.322310] 3ba0: ffff800973873c40 ffff0000081669d4 0000ffff8e165000 ffff000008d31400
[ 52.330060] 3bc0: 000000000000002e ffff800973870000 ffff80097fed7728 0000000000000000
[ 52.337812] 3be0: 0000000000000000 0000000000000000 000000000000000a 6120646165722064
[ 52.345562] 3c00: 6666303030302074 ffff800973873a80 0000000000000108 0000000000000002
[ 52.353313] 3c20: 0000000005f5e0ff 0000000000000000 0000000000000107 ffff000008d9bd15
[ 52.361061] 3c40: ffff0000081da450 0000ffff8dfe6770
[ 52.365891] [<ffff000008518638>] lkdtm_ACCESS_USERSPACE+0xb0/0x100
[ 52.372009] [<ffff000008739954>] lkdtm_do_action+0x1c/0x24
[ 52.377438] [<ffff000008517b00>] direct_entry+0xe0/0x160
[ 52.382696] [<ffff000008329200>] full_proxy_write+0x58/0x88
[ 52.388214] [<ffff0000081d830c>] __vfs_write+0x1c/0x110
[ 52.393384] [<ffff0000081d9120>] vfs_write+0xa0/0x1b8
[ 52.398383] [<ffff0000081da494>] SyS_write+0x44/0xa0
[ 52.403296] [<ffff000008083930>] el0_svc_naked+0x24/0x28
[ 52.408555] Code: b0002f60 aa1303e1 9135c000 97f138ce (f9400263)
--
Catalin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
` (7 preceding siblings ...)
2016-08-12 18:04 ` [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Kees Cook
@ 2016-08-13 9:13 ` Ard Biesheuvel
2016-08-15 9:48 ` Catalin Marinas
2016-08-26 15:39 ` David Brown
9 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2016-08-13 9:13 UTC (permalink / raw)
To: linux-arm-kernel
On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> This is the first (public) attempt at emulating PAN by disabling
> TTBR0_EL1 accesses on arm64.
I take it using TCR_EL1.EPD0 is too expensive?
> I chose to use a per-CPU saved_ttbr0_el1
> variable to store the actual TTBR0 as, IMO, it looks better w.r.t. the
> context switching code, to the detriment of a slightly more complex
> uaccess_enable() implementation. The alternative was storing the saved
> TTBR0 in thread_info but with more complex thread switching since TTBR0
> is normally tied to switch_mm() rather than switch_to(). The latter may
> also get more complicated if we are to decouple the kernel stack from
> thread_info at some point (vmalloc'ed stacks).
>
> The code requires more testing, especially for combinations where UAO is
> present but PAN is not.
>
> The patches are also available on this branch:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux ttbr0-pan
>
> Thanks for reviewing/testing.
>
> Catalin Marinas (7):
> arm64: Factor out PAN enabling/disabling into separate uaccess_*
> macros
> arm64: Factor out TTBR0_EL1 setting into a specific asm macro
> arm64: Introduce uaccess_{disable,enable} functionality based on
> TTBR0_EL1
> arm64: Disable TTBR0_EL1 during normal kernel execution
> arm64: Handle faults caused by inadvertent user access with PAN
> enabled
> arm64: xen: Enable user access before a privcmd hvc call
> arm64: Enable CONFIG_ARM64_TTBR0_PAN
>
> arch/arm64/Kconfig | 8 +++
> arch/arm64/include/asm/assembler.h | 106 +++++++++++++++++++++++++++++++-
> arch/arm64/include/asm/cpufeature.h | 6 ++
> arch/arm64/include/asm/efi.h | 14 +++++
> arch/arm64/include/asm/futex.h | 14 ++---
> arch/arm64/include/asm/kernel-pgtable.h | 7 +++
> arch/arm64/include/asm/mmu_context.h | 3 +-
> arch/arm64/include/asm/uaccess.h | 57 ++++++++++++++---
> arch/arm64/include/uapi/asm/ptrace.h | 2 +
> arch/arm64/kernel/armv8_deprecated.c | 10 +--
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/entry.S | 62 +++++++++++++++++++
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/suspend.c | 12 ++--
> arch/arm64/kernel/vmlinux.lds.S | 5 ++
> arch/arm64/lib/clear_user.S | 7 +--
> arch/arm64/lib/copy_from_user.S | 7 +--
> arch/arm64/lib/copy_in_user.S | 7 +--
> arch/arm64/lib/copy_to_user.S | 7 +--
> arch/arm64/mm/context.c | 25 +++++++-
> arch/arm64/mm/fault.c | 21 ++++---
> arch/arm64/mm/proc.S | 16 +----
> arch/arm64/xen/hypercall.S | 18 ++++++
> 23 files changed, 347 insertions(+), 74 deletions(-)
>
^ permalink raw reply [flat|nested] 37+ messages in thread* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-13 9:13 ` [kernel-hardening] " Ard Biesheuvel
@ 2016-08-15 9:48 ` Catalin Marinas
2016-08-15 9:58 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Catalin Marinas @ 2016-08-15 9:48 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > This is the first (public) attempt at emulating PAN by disabling
> > TTBR0_EL1 accesses on arm64.
>
> I take it using TCR_EL1.EPD0 is too expensive?
It would require full TLB invalidation on entering/exiting the kernel
and again for any user access. That's because the architecture allows
this bit to be cached in the TLB so without TLBI we wouldn't have any
guarantee that the actual PAN was toggled. I'm not sure it's even clear
whether a TLBI by ASID or a local one would suffice (likely OK for the
latter).
While I don't have numbers currently, it would be hard to test on the
multitude of partner ARMv8 implementations, especially since that's not
something people would expect to optimise the hardware for.
--
Catalin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 9:48 ` Catalin Marinas
@ 2016-08-15 9:58 ` Mark Rutland
2016-08-15 10:02 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2016-08-15 9:58 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > This is the first (public) attempt at emulating PAN by disabling
> > > TTBR0_EL1 accesses on arm64.
> >
> > I take it using TCR_EL1.EPD0 is too expensive?
>
> It would require full TLB invalidation on entering/exiting the kernel
> and again for any user access. That's because the architecture allows
> this bit to be cached in the TLB so without TLBI we wouldn't have any
> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> whether a TLBI by ASID or a local one would suffice (likely OK for the
> latter).
It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
least need TLB invalidation by ASID to remove previously-allocated entries from
TLBs.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 9:58 ` Mark Rutland
@ 2016-08-15 10:02 ` Ard Biesheuvel
2016-08-15 10:06 ` Mark Rutland
2016-08-15 10:08 ` Will Deacon
0 siblings, 2 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2016-08-15 10:02 UTC (permalink / raw)
To: linux-arm-kernel
On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
>> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
>> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> > > This is the first (public) attempt at emulating PAN by disabling
>> > > TTBR0_EL1 accesses on arm64.
>> >
>> > I take it using TCR_EL1.EPD0 is too expensive?
>>
>> It would require full TLB invalidation on entering/exiting the kernel
>> and again for any user access. That's because the architecture allows
>> this bit to be cached in the TLB so without TLBI we wouldn't have any
>> guarantee that the actual PAN was toggled. I'm not sure it's even clear
>> whether a TLBI by ASID or a local one would suffice (likely OK for the
>> latter).
>
> It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> least need TLB invalidation by ASID to remove previously-allocated entries from
> TLBs.
>
... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
the actual TTBR address alone.
This would remove the need for a zero page, and for recording the
original TTBR address in a per-cpu variable.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:02 ` Ard Biesheuvel
@ 2016-08-15 10:06 ` Mark Rutland
2016-08-15 10:10 ` Will Deacon
2016-08-15 10:21 ` Ard Biesheuvel
2016-08-15 10:08 ` Will Deacon
1 sibling, 2 replies; 37+ messages in thread
From: Mark Rutland @ 2016-08-15 10:06 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >> > > This is the first (public) attempt at emulating PAN by disabling
> >> > > TTBR0_EL1 accesses on arm64.
> >> >
> >> > I take it using TCR_EL1.EPD0 is too expensive?
> >>
> >> It would require full TLB invalidation on entering/exiting the kernel
> >> and again for any user access. That's because the architecture allows
> >> this bit to be cached in the TLB so without TLBI we wouldn't have any
> >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> >> whether a TLBI by ASID or a local one would suffice (likely OK for the
> >> latter).
> >
> > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> > least need TLB invalidation by ASID to remove previously-allocated entries from
> > TLBs.
>
> ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
> the actual TTBR address alone.
>
> This would remove the need for a zero page, and for recording the
> original TTBR address in a per-cpu variable.
That's a good point, and a better approach.
Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
and may require something stronger, given the precise rules for TLB-cached
fields isn't clear.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:06 ` Mark Rutland
@ 2016-08-15 10:10 ` Will Deacon
2016-08-15 10:15 ` Mark Rutland
2016-08-15 10:21 ` Ard Biesheuvel
1 sibling, 1 reply; 37+ messages in thread
From: Will Deacon @ 2016-08-15 10:10 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 11:06:49AM +0100, Mark Rutland wrote:
> On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
> > On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> > > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> > >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> > >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >> > > This is the first (public) attempt at emulating PAN by disabling
> > >> > > TTBR0_EL1 accesses on arm64.
> > >> >
> > >> > I take it using TCR_EL1.EPD0 is too expensive?
> > >>
> > >> It would require full TLB invalidation on entering/exiting the kernel
> > >> and again for any user access. That's because the architecture allows
> > >> this bit to be cached in the TLB so without TLBI we wouldn't have any
> > >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> > >> whether a TLBI by ASID or a local one would suffice (likely OK for the
> > >> latter).
> > >
> > > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> > > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> > > least need TLB invalidation by ASID to remove previously-allocated entries from
> > > TLBs.
> >
> > ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
> > the actual TTBR address alone.
> >
> > This would remove the need for a zero page, and for recording the
> > original TTBR address in a per-cpu variable.
>
> That's a good point, and a better approach.
>
> Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
> a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
> and may require something stronger, given the precise rules for TLB-cached
> fields isn't clear.
I suggest we get this clarified before merging the patch, as even the
author admits that it's ugly ;)
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:10 ` Will Deacon
@ 2016-08-15 10:15 ` Mark Rutland
2016-08-15 10:21 ` Will Deacon
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2016-08-15 10:15 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 11:10:09AM +0100, Will Deacon wrote:
> On Mon, Aug 15, 2016 at 11:06:49AM +0100, Mark Rutland wrote:
> > On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
> > > On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> > > > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> > > >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> > > >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > >> > > This is the first (public) attempt at emulating PAN by disabling
> > > >> > > TTBR0_EL1 accesses on arm64.
> > > >> >
> > > >> > I take it using TCR_EL1.EPD0 is too expensive?
> > > >>
> > > >> It would require full TLB invalidation on entering/exiting the kernel
> > > >> and again for any user access. That's because the architecture allows
> > > >> this bit to be cached in the TLB so without TLBI we wouldn't have any
> > > >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> > > >> whether a TLBI by ASID or a local one would suffice (likely OK for the
> > > >> latter).
> > > >
> > > > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> > > > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> > > > least need TLB invalidation by ASID to remove previously-allocated entries from
> > > > TLBs.
> > >
> > > ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
> > > the actual TTBR address alone.
> > >
> > > This would remove the need for a zero page, and for recording the
> > > original TTBR address in a per-cpu variable.
> >
> > That's a good point, and a better approach.
> >
> > Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
> > a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
> > and may require something stronger, given the precise rules for TLB-cached
> > fields isn't clear.
>
> I suggest we get this clarified before merging the patch, as even the
> author admits that it's ugly ;)
Just to be clear, you want to try the EPD0 approach in preference to Catalin's
current zero-page approach (which is safe regardless as it doesn't poke TCR.*)?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:15 ` Mark Rutland
@ 2016-08-15 10:21 ` Will Deacon
0 siblings, 0 replies; 37+ messages in thread
From: Will Deacon @ 2016-08-15 10:21 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 11:15:28AM +0100, Mark Rutland wrote:
> On Mon, Aug 15, 2016 at 11:10:09AM +0100, Will Deacon wrote:
> > On Mon, Aug 15, 2016 at 11:06:49AM +0100, Mark Rutland wrote:
> > > On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
> > > > On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> > > > > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> > > > >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> > > > >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >> > > This is the first (public) attempt at emulating PAN by disabling
> > > > >> > > TTBR0_EL1 accesses on arm64.
> > > > >> >
> > > > >> > I take it using TCR_EL1.EPD0 is too expensive?
> > > > >>
> > > > >> It would require full TLB invalidation on entering/exiting the kernel
> > > > >> and again for any user access. That's because the architecture allows
> > > > >> this bit to be cached in the TLB so without TLBI we wouldn't have any
> > > > >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> > > > >> whether a TLBI by ASID or a local one would suffice (likely OK for the
> > > > >> latter).
> > > > >
> > > > > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> > > > > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> > > > > least need TLB invalidation by ASID to remove previously-allocated entries from
> > > > > TLBs.
> > > >
> > > > ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
> > > > the actual TTBR address alone.
> > > >
> > > > This would remove the need for a zero page, and for recording the
> > > > original TTBR address in a per-cpu variable.
> > >
> > > That's a good point, and a better approach.
> > >
> > > Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
> > > a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
> > > and may require something stronger, given the precise rules for TLB-cached
> > > fields isn't clear.
> >
> > I suggest we get this clarified before merging the patch, as even the
> > author admits that it's ugly ;)
>
> Just to be clear, you want to try the EPD0 approach in preference to Catalin's
> current zero-page approach (which is safe regardless as it doesn't poke TCR.*)?
I'd like to be sure that there's not a cleaner approach per the
architecture, yes. That means getting clarification wrt what TLB
invalidation is actually required if we are to try setting EPD0. I have
a horrible feeling that it will be TLBIALLE1, but we should confirm that.
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:06 ` Mark Rutland
2016-08-15 10:10 ` Will Deacon
@ 2016-08-15 10:21 ` Ard Biesheuvel
2016-08-15 10:30 ` Will Deacon
2016-08-15 10:30 ` Mark Rutland
1 sibling, 2 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2016-08-15 10:21 UTC (permalink / raw)
To: linux-arm-kernel
On 15 August 2016 at 12:06, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
>> On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
>> >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
>> >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> >> > > This is the first (public) attempt at emulating PAN by disabling
>> >> > > TTBR0_EL1 accesses on arm64.
>> >> >
>> >> > I take it using TCR_EL1.EPD0 is too expensive?
>> >>
>> >> It would require full TLB invalidation on entering/exiting the kernel
>> >> and again for any user access. That's because the architecture allows
>> >> this bit to be cached in the TLB so without TLBI we wouldn't have any
>> >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
>> >> whether a TLBI by ASID or a local one would suffice (likely OK for the
>> >> latter).
>> >
>> > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
>> > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
>> > least need TLB invalidation by ASID to remove previously-allocated entries from
>> > TLBs.
>>
>> ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
>> the actual TTBR address alone.
>>
>> This would remove the need for a zero page, and for recording the
>> original TTBR address in a per-cpu variable.
>
> That's a good point, and a better approach.
>
> Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
> a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
> and may require something stronger, given the precise rules for TLB-cached
> fields isn't clear.
>
So how exactly would EPDn = 1 be cached in a TLB, given that the bit
specifically means that TTBRn_ELn is ignored on a TLB *miss*. Doesn't
that mean 'not covered by a TLB entry'? Or does it mean 'not covered
by a TLB entry describing a valid translation'?
I guess it indeed makes sense to get this clarified ...
As to Will's point, I suppose there is a window where a speculative
TLB fill could occur, so I suppose that means updating TTBR0_EL1.ASID
first, then TCR_EL1.EPD0, and finally perform the TLBI ASIDE1 on the
reserved ASID.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:21 ` Ard Biesheuvel
@ 2016-08-15 10:30 ` Will Deacon
2016-08-15 10:31 ` Ard Biesheuvel
2016-08-15 10:30 ` Mark Rutland
1 sibling, 1 reply; 37+ messages in thread
From: Will Deacon @ 2016-08-15 10:30 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:21:00PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 12:06, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
> >> On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> >> >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> >> >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >> >> > > This is the first (public) attempt at emulating PAN by disabling
> >> >> > > TTBR0_EL1 accesses on arm64.
> >> >> >
> >> >> > I take it using TCR_EL1.EPD0 is too expensive?
> >> >>
> >> >> It would require full TLB invalidation on entering/exiting the kernel
> >> >> and again for any user access. That's because the architecture allows
> >> >> this bit to be cached in the TLB so without TLBI we wouldn't have any
> >> >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> >> >> whether a TLBI by ASID or a local one would suffice (likely OK for the
> >> >> latter).
> >> >
> >> > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> >> > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> >> > least need TLB invalidation by ASID to remove previously-allocated entries from
> >> > TLBs.
> >>
> >> ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
> >> the actual TTBR address alone.
> >>
> >> This would remove the need for a zero page, and for recording the
> >> original TTBR address in a per-cpu variable.
> >
> > That's a good point, and a better approach.
> >
> > Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
> > a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
> > and may require something stronger, given the precise rules for TLB-cached
> > fields isn't clear.
> >
>
> So how exactly would EPDn = 1 be cached in a TLB, given that the bit
> specifically means that TTBRn_ELn is ignored on a TLB *miss*. Doesn't
> that mean 'not covered by a TLB entry'? Or does it mean 'not covered
> by a TLB entry describing a valid translation'?
>
> I guess it indeed makes sense to get this clarified ...
We'll put Rutland on the case.
> As to Will's point, I suppose there is a window where a speculative
> TLB fill could occur, so I suppose that means updating TTBR0_EL1.ASID
> first, then TCR_EL1.EPD0, and finally perform the TLBI ASIDE1 on the
> reserved ASID.
But then what do you gain from the reserved ASID?
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:30 ` Will Deacon
@ 2016-08-15 10:31 ` Ard Biesheuvel
2016-08-15 10:37 ` Will Deacon
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2016-08-15 10:31 UTC (permalink / raw)
To: linux-arm-kernel
On 15 August 2016 at 12:30, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Aug 15, 2016 at 12:21:00PM +0200, Ard Biesheuvel wrote:
>> On 15 August 2016 at 12:06, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
>> >> On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
>> >> >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
>> >> >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> >> >> > > This is the first (public) attempt at emulating PAN by disabling
>> >> >> > > TTBR0_EL1 accesses on arm64.
>> >> >> >
>> >> >> > I take it using TCR_EL1.EPD0 is too expensive?
>> >> >>
>> >> >> It would require full TLB invalidation on entering/exiting the kernel
>> >> >> and again for any user access. That's because the architecture allows
>> >> >> this bit to be cached in the TLB so without TLBI we wouldn't have any
>> >> >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
>> >> >> whether a TLBI by ASID or a local one would suffice (likely OK for the
>> >> >> latter).
>> >> >
>> >> > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
>> >> > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
>> >> > least need TLB invalidation by ASID to remove previously-allocated entries from
>> >> > TLBs.
>> >>
>> >> ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
>> >> the actual TTBR address alone.
>> >>
>> >> This would remove the need for a zero page, and for recording the
>> >> original TTBR address in a per-cpu variable.
>> >
>> > That's a good point, and a better approach.
>> >
>> > Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
>> > a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
>> > and may require something stronger, given the precise rules for TLB-cached
>> > fields isn't clear.
>> >
>>
>> So how exactly would EPDn = 1 be cached in a TLB, given that the bit
>> specifically means that TTBRn_ELn is ignored on a TLB *miss*. Doesn't
>> that mean 'not covered by a TLB entry'? Or does it mean 'not covered
>> by a TLB entry describing a valid translation'?
>>
>> I guess it indeed makes sense to get this clarified ...
>
> We'll put Rutland on the case.
>
>> As to Will's point, I suppose there is a window where a speculative
>> TLB fill could occur, so I suppose that means updating TTBR0_EL1.ASID
>> first, then TCR_EL1.EPD0, and finally perform the TLBI ASIDE1 on the
>> reserved ASID.
>
> But then what do you gain from the reserved ASID?
>
To prevent TLB hits against the ASID of the current (disabled)
userland translation
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:31 ` Ard Biesheuvel
@ 2016-08-15 10:37 ` Will Deacon
2016-08-15 10:43 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Will Deacon @ 2016-08-15 10:37 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:31:29PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 12:30, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 12:21:00PM +0200, Ard Biesheuvel wrote:
> >> As to Will's point, I suppose there is a window where a speculative
> >> TLB fill could occur, so I suppose that means updating TTBR0_EL1.ASID
> >> first, then TCR_EL1.EPD0, and finally perform the TLBI ASIDE1 on the
> >> reserved ASID.
> >
> > But then what do you gain from the reserved ASID?
> >
>
> To prevent TLB hits against the ASID of the current (disabled)
> userland translation
Right, but if the sequence you described ensures that, then why not just
set TCR_EL1.EPD0 and do TLBI ASIDE1 on the current ASID?
I don't see the difference between a TLB entry formed from a speculative
fill using the reserved ASID and one formed using a non-reserved ASID --
the page table is the same.
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:37 ` Will Deacon
@ 2016-08-15 10:43 ` Ard Biesheuvel
2016-08-15 10:52 ` Catalin Marinas
2016-08-15 11:00 ` Will Deacon
0 siblings, 2 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2016-08-15 10:43 UTC (permalink / raw)
To: linux-arm-kernel
On 15 August 2016 at 12:37, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Aug 15, 2016 at 12:31:29PM +0200, Ard Biesheuvel wrote:
>> On 15 August 2016 at 12:30, Will Deacon <will.deacon@arm.com> wrote:
>> > On Mon, Aug 15, 2016 at 12:21:00PM +0200, Ard Biesheuvel wrote:
>> >> As to Will's point, I suppose there is a window where a speculative
>> >> TLB fill could occur, so I suppose that means updating TTBR0_EL1.ASID
>> >> first, then TCR_EL1.EPD0, and finally perform the TLBI ASIDE1 on the
>> >> reserved ASID.
>> >
>> > But then what do you gain from the reserved ASID?
>> >
>>
>> To prevent TLB hits against the ASID of the current (disabled)
>> userland translation
>
> Right, but if the sequence you described ensures that, then why not just
> set TCR_EL1.EPD0 and do TLBI ASIDE1 on the current ASID?
>
... because then you wipe all the cached translations for current
userland, which I suppose is best avoided. Wiping the reserved ASID
only discards TLB entries that should not exist in the first place.
> I don't see the difference between a TLB entry formed from a speculative
> fill using the reserved ASID and one formed using a non-reserved ASID --
> the page table is the same.
>
No, but EPD0 does not disable translations, it disable translation
table walks on TLB misses, so we need to switch ASIDs to prevent user
space accesses via TLB hits.
But, how about we store the reserved ASID in TTBR1_EL1 instead, and
switch TCR_EL1.A1 and TCR_EL1.EPD0 in a single write? That way, we can
switch ASIDs and disable table walks atomically (I hope), and we
wouldn't need to change TTBR0_EL1 at all.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:43 ` Ard Biesheuvel
@ 2016-08-15 10:52 ` Catalin Marinas
2016-08-15 10:56 ` Ard Biesheuvel
2016-08-15 11:00 ` Will Deacon
1 sibling, 1 reply; 37+ messages in thread
From: Catalin Marinas @ 2016-08-15 10:52 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:43:31PM +0200, Ard Biesheuvel wrote:
> But, how about we store the reserved ASID in TTBR1_EL1 instead, and
> switch TCR_EL1.A1 and TCR_EL1.EPD0 in a single write? That way, we can
> switch ASIDs and disable table walks atomically (I hope), and we
> wouldn't need to change TTBR0_EL1 at all.
I did this before for AArch32 + LPAE (patches on the list sometime last
year I think). But the idea was nak'ed by the ARM architects. The
TCR_EL1.A1 can be cached somewhere in the TLB state machine, so you need
TLBI (IOW, toggling A1 does not guarantee an ASID switch).
--
Catalin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:52 ` Catalin Marinas
@ 2016-08-15 10:56 ` Ard Biesheuvel
2016-08-15 11:02 ` Will Deacon
2016-08-15 16:13 ` Catalin Marinas
0 siblings, 2 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2016-08-15 10:56 UTC (permalink / raw)
To: linux-arm-kernel
On 15 August 2016 at 12:52, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Mon, Aug 15, 2016 at 12:43:31PM +0200, Ard Biesheuvel wrote:
>> But, how about we store the reserved ASID in TTBR1_EL1 instead, and
>> switch TCR_EL1.A1 and TCR_EL1.EPD0 in a single write? That way, we can
>> switch ASIDs and disable table walks atomically (I hope), and we
>> wouldn't need to change TTBR0_EL1 at all.
>
> I did this before for AArch32 + LPAE (patches on the list sometime last
> year I think). But the idea was nak'ed by the ARM architects. The
> TCR_EL1.A1 can be cached somewhere in the TLB state machine, so you need
> TLBI (IOW, toggling A1 does not guarantee an ASID switch).
>
But how is TTBR0_EL1 any different? The ARM ARM equally mentions that
any of its field can be cached in a TLB, so by that reasoning, setting
a new ASID in TTBR0_EL1 would also require TLB maintenance.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:56 ` Ard Biesheuvel
@ 2016-08-15 11:02 ` Will Deacon
2016-08-15 16:13 ` Catalin Marinas
1 sibling, 0 replies; 37+ messages in thread
From: Will Deacon @ 2016-08-15 11:02 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:56:58PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 12:52, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 12:43:31PM +0200, Ard Biesheuvel wrote:
> >> But, how about we store the reserved ASID in TTBR1_EL1 instead, and
> >> switch TCR_EL1.A1 and TCR_EL1.EPD0 in a single write? That way, we can
> >> switch ASIDs and disable table walks atomically (I hope), and we
> >> wouldn't need to change TTBR0_EL1 at all.
> >
> > I did this before for AArch32 + LPAE (patches on the list sometime last
> > year I think). But the idea was nak'ed by the ARM architects. The
> > TCR_EL1.A1 can be cached somewhere in the TLB state machine, so you need
> > TLBI (IOW, toggling A1 does not guarantee an ASID switch).
> >
>
> But how is TTBR0_EL1 any different? The ARM ARM equally mentions that
> any of its field can be cached in a TLB, so by that reasoning, setting
> a new ASID in TTBR0_EL1 would also require TLB maintenance.
The underlying issue is that "cached in a TLB" doesn't distinguish between
"cached in a TLB *entry*" and "cached by the logic used to form TLB entries"
(i.e. the table walker).
That's where we need to seek clarification from the architects, because
existing microarchitectures do utilise both of these types of caching.
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:56 ` Ard Biesheuvel
2016-08-15 11:02 ` Will Deacon
@ 2016-08-15 16:13 ` Catalin Marinas
2016-08-15 19:04 ` Ard Biesheuvel
1 sibling, 1 reply; 37+ messages in thread
From: Catalin Marinas @ 2016-08-15 16:13 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:56:58PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 12:52, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 12:43:31PM +0200, Ard Biesheuvel wrote:
> >> But, how about we store the reserved ASID in TTBR1_EL1 instead, and
> >> switch TCR_EL1.A1 and TCR_EL1.EPD0 in a single write? That way, we can
> >> switch ASIDs and disable table walks atomically (I hope), and we
> >> wouldn't need to change TTBR0_EL1 at all.
> >
> > I did this before for AArch32 + LPAE (patches on the list sometime last
> > year I think). But the idea was nak'ed by the ARM architects. The
> > TCR_EL1.A1 can be cached somewhere in the TLB state machine, so you need
> > TLBI (IOW, toggling A1 does not guarantee an ASID switch).
>
> But how is TTBR0_EL1 any different? The ARM ARM equally mentions that
> any of its field can be cached in a TLB, so by that reasoning, setting
> a new ASID in TTBR0_EL1 would also require TLB maintenance.
Not really because this register is also described as part of the
context switching operation, so that would be an exception to the
general rule of requiring TLB invalidation for cached registers.
If you keep reading the same paragraph, the ARM ARM becomes more
subjective ;) and you may come to the conclusion that the reserved ASID
(not TCR_EL1.A1 though) + TCR_EL1.EDP0 would do the trick but we need
clarification from the architects rather than my random interpretation:
Section "D4.7.1 General TLB maintenance requirements" states:
Some System register field descriptions state that the effect of the
field is permitted to be cached in a TLB. This means that all TLB
entries that might be affected by a change of the field must be
invalidated whenever that field is changed
So the above kind of implies that only TLB *entries* that might be
affected by a change of a control bit need to be invalidated and only
the effect of such bit is cached (rather than the bit itself). The
effect of EDP0==1 is that there is no page table walk on a miss, so
there won't be any new entries cached in the TLB that would
reflect/cache the effect of EDP0==1. We still need to follow this by a
switch to the reserved ASID to make sure there are no other TLB entries
for TTBR0 (and we shouldn't care about the window between EDP0=1 and
ASID=reserved).
Anyway, the above wouldn't help the code much since we still need to
preserve/restore/switch the ASID of the current thread (that's unless we
temporarily store TTBR0_EL1.ASID into the TTBR1_EL1.ASID field). The
TCR_EL1.A1 trick would have been nice but explicitly rejected by the
architects (I guess it's not part of the context switching sequence, so
the hardware may not notice the A1 bit change).
--
Catalin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 16:13 ` Catalin Marinas
@ 2016-08-15 19:04 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2016-08-15 19:04 UTC (permalink / raw)
To: linux-arm-kernel
On 15 August 2016 at 18:13, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Mon, Aug 15, 2016 at 12:56:58PM +0200, Ard Biesheuvel wrote:
>> On 15 August 2016 at 12:52, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> > On Mon, Aug 15, 2016 at 12:43:31PM +0200, Ard Biesheuvel wrote:
>> >> But, how about we store the reserved ASID in TTBR1_EL1 instead, and
>> >> switch TCR_EL1.A1 and TCR_EL1.EPD0 in a single write? That way, we can
>> >> switch ASIDs and disable table walks atomically (I hope), and we
>> >> wouldn't need to change TTBR0_EL1 at all.
>> >
>> > I did this before for AArch32 + LPAE (patches on the list sometime last
>> > year I think). But the idea was nak'ed by the ARM architects. The
>> > TCR_EL1.A1 can be cached somewhere in the TLB state machine, so you need
>> > TLBI (IOW, toggling A1 does not guarantee an ASID switch).
>>
>> But how is TTBR0_EL1 any different? The ARM ARM equally mentions that
>> any of its field can be cached in a TLB, so by that reasoning, setting
>> a new ASID in TTBR0_EL1 would also require TLB maintenance.
>
> Not really because this register is also described as part of the
> context switching operation, so that would be an exception to the
> general rule of requiring TLB invalidation for cached registers.
>
Well, of course, requiring TLB maintenance to change the current ASID
would be silly. But the ASID is obviously cached in each TLB entry
that is generated while the ASID is active, which makes the sentence
'Any of the fields in this register are permitted to be cached in a
TLB.' a bit ambiguous, since it obviously does not mean that a stale
ASID may be cached by the table walker.
> If you keep reading the same paragraph, the ARM ARM becomes more
> subjective ;) and you may come to the conclusion that the reserved ASID
> (not TCR_EL1.A1 though) + TCR_EL1.EDP0 would do the trick but we need
> clarification from the architects rather than my random interpretation:
>
> Section "D4.7.1 General TLB maintenance requirements" states:
>
> Some System register field descriptions state that the effect of the
> field is permitted to be cached in a TLB. This means that all TLB
> entries that might be affected by a change of the field must be
> invalidated whenever that field is changed
>
> So the above kind of implies that only TLB *entries* that might be
> affected by a change of a control bit need to be invalidated and only
> the effect of such bit is cached (rather than the bit itself). The
> effect of EDP0==1 is that there is no page table walk on a miss, so
> there won't be any new entries cached in the TLB that would
> reflect/cache the effect of EDP0==1. We still need to follow this by a
> switch to the reserved ASID to make sure there are no other TLB entries
> for TTBR0 (and we shouldn't care about the window between EDP0=1 and
> ASID=reserved).
>
Indeed.
> Anyway, the above wouldn't help the code much since we still need to
> preserve/restore/switch the ASID of the current thread (that's unless we
> temporarily store TTBR0_EL1.ASID into the TTBR1_EL1.ASID field).
That is actually not such a bad idea. We could assign both fields at
context switch time, and simply copy it from TTBR1_EL1 to TTBR0_EL1
before doing the user access.
> The
> TCR_EL1.A1 trick would have been nice but explicitly rejected by the
> architects (I guess it's not part of the context switching sequence, so
> the hardware may not notice the A1 bit change).
>
Yeah, that's unfortunate. But I think this feature is important, and
combined with the hardened usercopy feature drastically reduces the
attack surface of the kernel, so I expect it to quickly make its way
into various 'stable' downstream trees. So I would really like to get
to the bottom of this.
--
Ard.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:43 ` Ard Biesheuvel
2016-08-15 10:52 ` Catalin Marinas
@ 2016-08-15 11:00 ` Will Deacon
1 sibling, 0 replies; 37+ messages in thread
From: Will Deacon @ 2016-08-15 11:00 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:43:31PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 12:37, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 12:31:29PM +0200, Ard Biesheuvel wrote:
> >> On 15 August 2016 at 12:30, Will Deacon <will.deacon@arm.com> wrote:
> >> > On Mon, Aug 15, 2016 at 12:21:00PM +0200, Ard Biesheuvel wrote:
> >> >> As to Will's point, I suppose there is a window where a speculative
> >> >> TLB fill could occur, so I suppose that means updating TTBR0_EL1.ASID
> >> >> first, then TCR_EL1.EPD0, and finally perform the TLBI ASIDE1 on the
> >> >> reserved ASID.
> >> >
> >> > But then what do you gain from the reserved ASID?
> >> >
> >>
> >> To prevent TLB hits against the ASID of the current (disabled)
> >> userland translation
> >
> > Right, but if the sequence you described ensures that, then why not just
> > set TCR_EL1.EPD0 and do TLBI ASIDE1 on the current ASID?
> >
>
> ... because then you wipe all the cached translations for current
> userland, which I suppose is best avoided. Wiping the reserved ASID
> only discards TLB entries that should not exist in the first place.
True, I guess we'd need to measure that vs. the extra cost of switching
to/from the reserved ASID.
> > I don't see the difference between a TLB entry formed from a speculative
> > fill using the reserved ASID and one formed using a non-reserved ASID --
> > the page table is the same.
> >
>
> No, but EPD0 does not disable translations, it disable translation
> table walks on TLB misses, so we need to switch ASIDs to prevent user
> space accesses via TLB hits.
Right, only if you don't do the invalidation for the current ASID.
> But, how about we store the reserved ASID in TTBR1_EL1 instead, and
> switch TCR_EL1.A1 and TCR_EL1.EPD0 in a single write? That way, we can
> switch ASIDs and disable table walks atomically (I hope), and we
> wouldn't need to change TTBR0_EL1 at all.
I doubt that would work, unfortunately. Whilst the writes are atomic
from the point-of-view of the TCR, if the fields can be cached in the
TLB logic, then we can't guarantee the atomicity of changes to the
cached state. Nice idea, though!
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:21 ` Ard Biesheuvel
2016-08-15 10:30 ` Will Deacon
@ 2016-08-15 10:30 ` Mark Rutland
1 sibling, 0 replies; 37+ messages in thread
From: Mark Rutland @ 2016-08-15 10:30 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:21:00PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 12:06, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
> >> On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> >> >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> >> >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >> >> > > This is the first (public) attempt at emulating PAN by disabling
> >> >> > > TTBR0_EL1 accesses on arm64.
> >> >> >
> >> >> > I take it using TCR_EL1.EPD0 is too expensive?
> >> >>
> >> >> It would require full TLB invalidation on entering/exiting the kernel
> >> >> and again for any user access. That's because the architecture allows
> >> >> this bit to be cached in the TLB so without TLBI we wouldn't have any
> >> >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> >> >> whether a TLBI by ASID or a local one would suffice (likely OK for the
> >> >> latter).
> >> >
> >> > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> >> > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> >> > least need TLB invalidation by ASID to remove previously-allocated entries from
> >> > TLBs.
> >>
> >> ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
> >> the actual TTBR address alone.
> >>
> >> This would remove the need for a zero page, and for recording the
> >> original TTBR address in a per-cpu variable.
> >
> > That's a good point, and a better approach.
> >
> > Unfortunately, we're still left with the issue that TCR_EL1.* can be cached in
> > a TLB, as Catalin pointed out. Which at minimum would require a TLBI ASIDE1,
> > and may require something stronger, given the precise rules for TLB-cached
> > fields isn't clear.
>
> So how exactly would EPDn = 1 be cached in a TLB, given that the bit
> specifically means that TTBRn_ELn is ignored on a TLB *miss*. Doesn't
> that mean 'not covered by a TLB entry'? Or does it mean 'not covered
> by a TLB entry describing a valid translation'?
The ARM ARM uses 'TLB' to mean any internal storage used by the translation,
including internal registers. So it's not necessarily a TLB entry in the usual
sense.
Most of these make more sense if you consider that Stage-2 'TLBs' might cache
this information for the purpose of walking Stage-1 entries. There are other
potential reasons beyond that, though.
> I guess it indeed makes sense to get this clarified ...
>
> As to Will's point, I suppose there is a window where a speculative
> TLB fill could occur, so I suppose that means updating TTBR0_EL1.ASID
> first, then TCR_EL1.EPD0, and finally perform the TLBI ASIDE1 on the
> reserved ASID.
I thought that too, but this could race with concurrent modification on another
CPU. That other CPU would perform maintenance with the usual ASID, then perhaps
modify/free entries.
There would be a window where the CPU doing the EPD0 + reserved-ASID trick
could have an erroneous view of the page tables, and walk garbage or allocate
conflicting entries.
Perhaps the ASID allocation logic could be modified to avoid that race, though.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-15 10:02 ` Ard Biesheuvel
2016-08-15 10:06 ` Mark Rutland
@ 2016-08-15 10:08 ` Will Deacon
1 sibling, 0 replies; 37+ messages in thread
From: Will Deacon @ 2016-08-15 10:08 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 15, 2016 at 12:02:33PM +0200, Ard Biesheuvel wrote:
> On 15 August 2016 at 11:58, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Aug 15, 2016 at 10:48:42AM +0100, Catalin Marinas wrote:
> >> On Sat, Aug 13, 2016 at 11:13:58AM +0200, Ard Biesheuvel wrote:
> >> > On 12 August 2016 at 17:27, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >> > > This is the first (public) attempt at emulating PAN by disabling
> >> > > TTBR0_EL1 accesses on arm64.
> >> >
> >> > I take it using TCR_EL1.EPD0 is too expensive?
> >>
> >> It would require full TLB invalidation on entering/exiting the kernel
> >> and again for any user access. That's because the architecture allows
> >> this bit to be cached in the TLB so without TLBI we wouldn't have any
> >> guarantee that the actual PAN was toggled. I'm not sure it's even clear
> >> whether a TLBI by ASID or a local one would suffice (likely OK for the
> >> latter).
> >
> > It's worth noting that even ignoring the TLB-caching of TCR_EL1.EPD0, the
> > control only affects the behaviour on a TLB miss. Thus to use EPD0 we'd at
> > least need TLB invalidation by ASID to remove previously-allocated entries from
> > TLBs.
> >
>
> ... or update the ASID to the reserved ASID in TTBR0_EL1, but leave
> the actual TTBR address alone.
>
> This would remove the need for a zero page, and for recording the
> original TTBR address in a per-cpu variable.
Hmm, how does that work? The ASID and EPDx are in different registers,
so there's still a window where we could get speculative TLB fills.
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-12 15:27 [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching Catalin Marinas
` (8 preceding siblings ...)
2016-08-13 9:13 ` [kernel-hardening] " Ard Biesheuvel
@ 2016-08-26 15:39 ` David Brown
2016-08-26 17:24 ` Catalin Marinas
9 siblings, 1 reply; 37+ messages in thread
From: David Brown @ 2016-08-26 15:39 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Aug 12, 2016 at 04:27:39PM +0100, Catalin Marinas wrote:
>This is the first (public) attempt at emulating PAN by disabling
>TTBR0_EL1 accesses on arm64. I chose to use a per-CPU saved_ttbr0_el1
>variable to store the actual TTBR0 as, IMO, it looks better w.r.t. the
>context switching code, to the detriment of a slightly more complex
>uaccess_enable() implementation. The alternative was storing the saved
>TTBR0 in thread_info but with more complex thread switching since TTBR0
>is normally tied to switch_mm() rather than switch_to(). The latter may
>also get more complicated if we are to decouple the kernel stack from
>thread_info at some point (vmalloc'ed stacks).
>
>The code requires more testing, especially for combinations where UAO is
>present but PAN is not.
I briefly tried to run these patches on my HiKey board and I get a
panic on boot. Unfortunately, I've had to head off to the Linux
Security Summit, so I haven't been able to try to figure out what is
going on (and I don't seem to be able to even get a capture of the log
output). But I ran into Mark Rutland who convinced me to at least
state the failure on the list here.
The same kernel boots fine in Qemu.
David
^ permalink raw reply [flat|nested] 37+ messages in thread* [kernel-hardening] [PATCH 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-08-26 15:39 ` David Brown
@ 2016-08-26 17:24 ` Catalin Marinas
0 siblings, 0 replies; 37+ messages in thread
From: Catalin Marinas @ 2016-08-26 17:24 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Aug 26, 2016 at 11:39:04AM -0400, David Brown wrote:
> On Fri, Aug 12, 2016 at 04:27:39PM +0100, Catalin Marinas wrote:
> >This is the first (public) attempt at emulating PAN by disabling
> >TTBR0_EL1 accesses on arm64. I chose to use a per-CPU saved_ttbr0_el1
> >variable to store the actual TTBR0 as, IMO, it looks better w.r.t. the
> >context switching code, to the detriment of a slightly more complex
> >uaccess_enable() implementation. The alternative was storing the saved
> >TTBR0 in thread_info but with more complex thread switching since TTBR0
> >is normally tied to switch_mm() rather than switch_to(). The latter may
> >also get more complicated if we are to decouple the kernel stack from
> >thread_info at some point (vmalloc'ed stacks).
> >
> >The code requires more testing, especially for combinations where UAO is
> >present but PAN is not.
>
> I briefly tried to run these patches on my HiKey board and I get a
> panic on boot. Unfortunately, I've had to head off to the Linux
> Security Summit, so I haven't been able to try to figure out what is
> going on (and I don't seem to be able to even get a capture of the log
> output). But I ran into Mark Rutland who convinced me to at least
> state the failure on the list here.
Thanks for trying to test this series. Without more information, I can't
really say where the problem is or why it fails that early (in theory it
should only affect user space). Is it with recent/latest mainline?
defconfig? Anyway, I'll wait until you get back from the conference,
maybe you get some early console output.
--
Catalin
^ permalink raw reply [flat|nested] 37+ messages in thread