* [kernel-hardening] [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
This is the third version of the arm64 PAN emulation using TTBR0_EL1
switching. The series has not yet included the alternative nop patches
from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
in a subsequent version once 4.9-rc1 is out (which will include Mark's
alternative nop patches).
Changes since v2:
- efi_set_pgd() reworked to update the saved ttbr0 during run-time
services as this value is used during exception return
- uaccess_(enable|disable) C macros no longer take an 'alt' parameter
Instead, uaccess_(enable|disable)_not_uao are introduced for the case
where hardware PAN switching is not required when UAO is present
- post_ttbr0_update_workaround macro no longer takes a 'ret' parameter
- system_supports_ttbr0_pan renamed to system_uses_ttbr0_pan
- init_thread_info.ttbr0 moved towards the end of the setup_arch()
function and comment updated
- vmlinux.lds.S fixed to use RESERVED_TTBR0_SIZE
- Config option changed to ARM64_SW_TTBR0_PAN
- Some comment clean-ups and commit log updates
Catalin Marinas (7):
arm64: Factor out PAN enabling/disabling into separate uaccess_*
macros
arm64: Factor out TTBR0_EL1 post-update workaround 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_SW_TTBR0_PAN
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.
arch/arm64/Kconfig | 8 ++
arch/arm64/include/asm/assembler.h | 33 +++++++
arch/arm64/include/asm/cpufeature.h | 6 ++
arch/arm64/include/asm/efi.h | 26 ++++-
arch/arm64/include/asm/futex.h | 14 +--
arch/arm64/include/asm/kernel-pgtable.h | 7 ++
arch/arm64/include/asm/mmu_context.h | 51 +++++++---
arch/arm64/include/asm/ptrace.h | 2 +
arch/arm64/include/asm/thread_info.h | 3 +
arch/arm64/include/asm/uaccess.h | 163 ++++++++++++++++++++++++++++++--
arch/arm64/kernel/armv8_deprecated.c | 10 +-
arch/arm64/kernel/asm-offsets.c | 3 +
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/entry.S | 71 +++++++++++++-
arch/arm64/kernel/head.S | 6 +-
arch/arm64/kernel/setup.c | 9 ++
arch/arm64/kernel/vmlinux.lds.S | 5 +
arch/arm64/lib/clear_user.S | 8 +-
arch/arm64/lib/copy_from_user.S | 8 +-
arch/arm64/lib/copy_in_user.S | 8 +-
arch/arm64/lib/copy_to_user.S | 8 +-
arch/arm64/mm/context.c | 7 +-
arch/arm64/mm/fault.c | 22 +++--
arch/arm64/mm/proc.S | 11 +--
arch/arm64/xen/hypercall.S | 19 ++++
25 files changed, 428 insertions(+), 81 deletions(-)
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH v3 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-13 17:46 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 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.
Note that any (unlikely) access that the compiler might generate between
uaccess_enable() and uaccess_disable(), other than those explicitly
specified by the user access code, will not be protected by PAN.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/futex.h | 14 +++----
arch/arm64/include/asm/uaccess.h | 79 ++++++++++++++++++++++++++++++++----
arch/arm64/kernel/armv8_deprecated.c | 10 ++---
arch/arm64/lib/clear_user.S | 8 ++--
arch/arm64/lib/copy_from_user.S | 8 ++--
arch/arm64/lib/copy_in_user.S | 8 ++--
arch/arm64/lib/copy_to_user.S | 8 ++--
7 files changed, 95 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index f2585cdd32c2..71dfa3b42313 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(); \
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(); \
+} 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();
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();
*uval = val;
return ret;
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index c47257c91b77..cc6c32d4dcc4 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -18,6 +18,8 @@
#ifndef __ASM_UACCESS_H
#define __ASM_UACCESS_H
+#ifndef __ASSEMBLY__
+
/*
* User space memory access functions
*/
@@ -112,6 +114,44 @@ 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)
+
+static inline void uaccess_disable(void)
+{
+ __uaccess_disable(ARM64_HAS_PAN);
+}
+
+static inline void uaccess_enable(void)
+{
+ __uaccess_enable(ARM64_HAS_PAN);
+}
+
+/*
+ * These functions are no-ops when UAO is present.
+ */
+static inline void uaccess_disable_not_uao(void)
+{
+ __uaccess_disable(ARM64_ALT_PAN_NOT_UAO);
+}
+
+static inline void uaccess_enable_not_uao(void)
+{
+ __uaccess_enable(ARM64_ALT_PAN_NOT_UAO);
+}
+
+/*
* 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 +178,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_not_uao(); \
switch (sizeof(*(ptr))) { \
case 1: \
__get_user_asm("ldrb", "ldtrb", "%w", __gu_val, (ptr), \
@@ -160,9 +199,8 @@ do { \
default: \
BUILD_BUG(); \
} \
+ uaccess_disable_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 +245,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_not_uao(); \
switch (sizeof(*(ptr))) { \
case 1: \
__put_user_asm("strb", "sttrb", "%w", __pu_val, (ptr), \
@@ -229,8 +266,7 @@ do { \
default: \
BUILD_BUG(); \
} \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_disable_not_uao(); \
} while (0)
#define __put_user(x, ptr) \
@@ -321,4 +357,31 @@ extern long strncpy_from_user(char *dest, const char __user *src, long count);
extern __must_check long strlen_user(const char __user *str);
extern __must_check long strnlen_user(const char __user *str, long n);
+#else /* __ASSEMBLY__ */
+
+#include <asm/alternative.h>
+#include <asm/assembler.h>
+
+/*
+ * User access enabling/disabling macros. These are no-ops when UAO is
+ * present.
+ */
+ .macro uaccess_disable_not_uao, tmp1
+alternative_if_not ARM64_ALT_PAN_NOT_UAO
+ nop
+alternative_else
+ SET_PSTATE_PAN(1)
+alternative_endif
+ .endm
+
+ .macro uaccess_enable_not_uao, tmp1, tmp2
+alternative_if_not ARM64_ALT_PAN_NOT_UAO
+ nop
+alternative_else
+ SET_PSTATE_PAN(0)
+alternative_endif
+ .endm
+
+#endif /* __ASSEMBLY__ */
+
#endif /* __ASM_UACCESS_H */
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 42ffdb54e162..7156e7bfcd89 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(); \
__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(); \
+} 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..08b5f18ba604 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -17,10 +17,10 @@
*/
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
.text
@@ -33,8 +33,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_not_uao x2, x3
mov x2, x1 // save the size for fixup return
subs x1, x1, #8
b.mi 2f
@@ -54,8 +53,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_not_uao 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..6505ec81f1da 100644
--- a/arch/arm64/lib/copy_from_user.S
+++ b/arch/arm64/lib/copy_from_user.S
@@ -16,11 +16,11 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy from user space to a kernel buffer (alignment handled by the hardware)
@@ -67,12 +67,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_not_uao x3, x4
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_not_uao 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..9b04ff3ab610 100644
--- a/arch/arm64/lib/copy_in_user.S
+++ b/arch/arm64/lib/copy_in_user.S
@@ -18,11 +18,11 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy from user space to user space (alignment handled by the hardware)
@@ -68,12 +68,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_not_uao x3, x4
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_not_uao 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..8077e4f34d56 100644
--- a/arch/arm64/lib/copy_to_user.S
+++ b/arch/arm64/lib/copy_to_user.S
@@ -16,11 +16,11 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy to user space from a kernel buffer (alignment handled by the hardware)
@@ -66,12 +66,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_not_uao x3, x4
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_not_uao x3
mov x0, #0
ret
ENDPROC(__arch_copy_to_user)
^ permalink raw reply related [flat|nested] 64+ messages in thread* [kernel-hardening] [PATCH v3 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
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.
Note that any (unlikely) access that the compiler might generate between
uaccess_enable() and uaccess_disable(), other than those explicitly
specified by the user access code, will not be protected by PAN.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/futex.h | 14 +++----
arch/arm64/include/asm/uaccess.h | 79 ++++++++++++++++++++++++++++++++----
arch/arm64/kernel/armv8_deprecated.c | 10 ++---
arch/arm64/lib/clear_user.S | 8 ++--
arch/arm64/lib/copy_from_user.S | 8 ++--
arch/arm64/lib/copy_in_user.S | 8 ++--
arch/arm64/lib/copy_to_user.S | 8 ++--
7 files changed, 95 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index f2585cdd32c2..71dfa3b42313 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(); \
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(); \
+} 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();
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();
*uval = val;
return ret;
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index c47257c91b77..cc6c32d4dcc4 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -18,6 +18,8 @@
#ifndef __ASM_UACCESS_H
#define __ASM_UACCESS_H
+#ifndef __ASSEMBLY__
+
/*
* User space memory access functions
*/
@@ -112,6 +114,44 @@ 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)
+
+static inline void uaccess_disable(void)
+{
+ __uaccess_disable(ARM64_HAS_PAN);
+}
+
+static inline void uaccess_enable(void)
+{
+ __uaccess_enable(ARM64_HAS_PAN);
+}
+
+/*
+ * These functions are no-ops when UAO is present.
+ */
+static inline void uaccess_disable_not_uao(void)
+{
+ __uaccess_disable(ARM64_ALT_PAN_NOT_UAO);
+}
+
+static inline void uaccess_enable_not_uao(void)
+{
+ __uaccess_enable(ARM64_ALT_PAN_NOT_UAO);
+}
+
+/*
* 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 +178,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_not_uao(); \
switch (sizeof(*(ptr))) { \
case 1: \
__get_user_asm("ldrb", "ldtrb", "%w", __gu_val, (ptr), \
@@ -160,9 +199,8 @@ do { \
default: \
BUILD_BUG(); \
} \
+ uaccess_disable_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 +245,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_not_uao(); \
switch (sizeof(*(ptr))) { \
case 1: \
__put_user_asm("strb", "sttrb", "%w", __pu_val, (ptr), \
@@ -229,8 +266,7 @@ do { \
default: \
BUILD_BUG(); \
} \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_disable_not_uao(); \
} while (0)
#define __put_user(x, ptr) \
@@ -321,4 +357,31 @@ extern long strncpy_from_user(char *dest, const char __user *src, long count);
extern __must_check long strlen_user(const char __user *str);
extern __must_check long strnlen_user(const char __user *str, long n);
+#else /* __ASSEMBLY__ */
+
+#include <asm/alternative.h>
+#include <asm/assembler.h>
+
+/*
+ * User access enabling/disabling macros. These are no-ops when UAO is
+ * present.
+ */
+ .macro uaccess_disable_not_uao, tmp1
+alternative_if_not ARM64_ALT_PAN_NOT_UAO
+ nop
+alternative_else
+ SET_PSTATE_PAN(1)
+alternative_endif
+ .endm
+
+ .macro uaccess_enable_not_uao, tmp1, tmp2
+alternative_if_not ARM64_ALT_PAN_NOT_UAO
+ nop
+alternative_else
+ SET_PSTATE_PAN(0)
+alternative_endif
+ .endm
+
+#endif /* __ASSEMBLY__ */
+
#endif /* __ASM_UACCESS_H */
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 42ffdb54e162..7156e7bfcd89 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(); \
__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(); \
+} 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..08b5f18ba604 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -17,10 +17,10 @@
*/
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
.text
@@ -33,8 +33,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_not_uao x2, x3
mov x2, x1 // save the size for fixup return
subs x1, x1, #8
b.mi 2f
@@ -54,8 +53,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_not_uao 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..6505ec81f1da 100644
--- a/arch/arm64/lib/copy_from_user.S
+++ b/arch/arm64/lib/copy_from_user.S
@@ -16,11 +16,11 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy from user space to a kernel buffer (alignment handled by the hardware)
@@ -67,12 +67,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_not_uao x3, x4
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_not_uao 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..9b04ff3ab610 100644
--- a/arch/arm64/lib/copy_in_user.S
+++ b/arch/arm64/lib/copy_in_user.S
@@ -18,11 +18,11 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy from user space to user space (alignment handled by the hardware)
@@ -68,12 +68,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_not_uao x3, x4
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_not_uao 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..8077e4f34d56 100644
--- a/arch/arm64/lib/copy_to_user.S
+++ b/arch/arm64/lib/copy_to_user.S
@@ -16,11 +16,11 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
#include <asm/cpufeature.h>
#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy to user space from a kernel buffer (alignment handled by the hardware)
@@ -66,12 +66,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_not_uao x3, x4
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_not_uao x3
mov x0, #0
ret
ENDPROC(__arch_copy_to_user)
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH v3 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-15 15:10 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 15:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Catalin,
On Tue, Sep 13, 2016 at 06:46:31PM +0100, Catalin Marinas wrote:
> 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.
>
> Note that any (unlikely) access that the compiler might generate between
> uaccess_enable() and uaccess_disable(), other than those explicitly
> specified by the user access code, will not be protected by PAN.
There's one missing include that I've noted below, along with a number
we can now drop. Otherwise, the important parts all look good to me.
With the includes fixed up:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
[...]
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/futex.h | 14 +++----
> arch/arm64/include/asm/uaccess.h | 79 ++++++++++++++++++++++++++++++++----
> arch/arm64/kernel/armv8_deprecated.c | 10 ++---
> arch/arm64/lib/clear_user.S | 8 ++--
> arch/arm64/lib/copy_from_user.S | 8 ++--
> arch/arm64/lib/copy_in_user.S | 8 ++--
> arch/arm64/lib/copy_to_user.S | 8 ++--
> 7 files changed, 95 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f2585cdd32c2..71dfa3b42313 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(); \
> 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(); \
> +} while (0)
With the uaccess_{enable,disable}* macros, we can drop the includes for
<asm/alternative.h>, <asm/cpufeature.h>, and <asm/sysreg.h> from
futex.h, as we no longer (directly) use anything from those.
[...]
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index c47257c91b77..cc6c32d4dcc4 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -321,4 +357,31 @@ extern long strncpy_from_user(char *dest, const char __user *src, long count);
> extern __must_check long strlen_user(const char __user *str);
> extern __must_check long strnlen_user(const char __user *str, long n);
>
> +#else /* __ASSEMBLY__ */
> +
> +#include <asm/alternative.h>
> +#include <asm/assembler.h>
We also need <asm/sysreg.h> for SET_PSTATE_PAN()
Given we use that and <asm/alternative.h> either way, should we pull
those above the #ifdef __ASSEMBLY__? I have no feeling either way.
[...]
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 42ffdb54e162..7156e7bfcd89 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(); \
> __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(); \
> +} while (0)
With this, we can drop the include of <asm/alternative.h>, though unlike
futex.h we need to keep <asm/cpufeature.h> and <asm/sysreg.h>.
[...]
> diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
> index 5d1cad3ce6d6..08b5f18ba604 100644
> --- a/arch/arm64/lib/clear_user.S
> +++ b/arch/arm64/lib/clear_user.S
> @@ -17,10 +17,10 @@
> */
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
As with futex.h, we can drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h>, as the uaccess_{enable,disable}* macros mean we no
longer use anything from those directly.
[...]
> diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
> index 0b90497d4424..6505ec81f1da 100644
> --- a/arch/arm64/lib/copy_from_user.S
> +++ b/arch/arm64/lib/copy_from_user.S
> @@ -16,11 +16,11 @@
>
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cache.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
Likewise, we can also drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h> here.
[...]
> diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S
> index f7292dd08c84..9b04ff3ab610 100644
> --- a/arch/arm64/lib/copy_in_user.S
> +++ b/arch/arm64/lib/copy_in_user.S
> @@ -18,11 +18,11 @@
>
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cache.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
Likewise, we can also drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h> here.
[...]
> diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
> index 7a7efe255034..8077e4f34d56 100644
> --- a/arch/arm64/lib/copy_to_user.S
> +++ b/arch/arm64/lib/copy_to_user.S
> @@ -16,11 +16,11 @@
>
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cache.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
Likewise, we can also drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h> here.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros
@ 2016-09-15 15:10 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 15:10 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, Will Deacon, James Morse, Kees Cook,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
Hi Catalin,
On Tue, Sep 13, 2016 at 06:46:31PM +0100, Catalin Marinas wrote:
> 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.
>
> Note that any (unlikely) access that the compiler might generate between
> uaccess_enable() and uaccess_disable(), other than those explicitly
> specified by the user access code, will not be protected by PAN.
There's one missing include that I've noted below, along with a number
we can now drop. Otherwise, the important parts all look good to me.
With the includes fixed up:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
[...]
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/futex.h | 14 +++----
> arch/arm64/include/asm/uaccess.h | 79 ++++++++++++++++++++++++++++++++----
> arch/arm64/kernel/armv8_deprecated.c | 10 ++---
> arch/arm64/lib/clear_user.S | 8 ++--
> arch/arm64/lib/copy_from_user.S | 8 ++--
> arch/arm64/lib/copy_in_user.S | 8 ++--
> arch/arm64/lib/copy_to_user.S | 8 ++--
> 7 files changed, 95 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index f2585cdd32c2..71dfa3b42313 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(); \
> 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(); \
> +} while (0)
With the uaccess_{enable,disable}* macros, we can drop the includes for
<asm/alternative.h>, <asm/cpufeature.h>, and <asm/sysreg.h> from
futex.h, as we no longer (directly) use anything from those.
[...]
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index c47257c91b77..cc6c32d4dcc4 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -321,4 +357,31 @@ extern long strncpy_from_user(char *dest, const char __user *src, long count);
> extern __must_check long strlen_user(const char __user *str);
> extern __must_check long strnlen_user(const char __user *str, long n);
>
> +#else /* __ASSEMBLY__ */
> +
> +#include <asm/alternative.h>
> +#include <asm/assembler.h>
We also need <asm/sysreg.h> for SET_PSTATE_PAN()
Given we use that and <asm/alternative.h> either way, should we pull
those above the #ifdef __ASSEMBLY__? I have no feeling either way.
[...]
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 42ffdb54e162..7156e7bfcd89 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(); \
> __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(); \
> +} while (0)
With this, we can drop the include of <asm/alternative.h>, though unlike
futex.h we need to keep <asm/cpufeature.h> and <asm/sysreg.h>.
[...]
> diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
> index 5d1cad3ce6d6..08b5f18ba604 100644
> --- a/arch/arm64/lib/clear_user.S
> +++ b/arch/arm64/lib/clear_user.S
> @@ -17,10 +17,10 @@
> */
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
As with futex.h, we can drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h>, as the uaccess_{enable,disable}* macros mean we no
longer use anything from those directly.
[...]
> diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
> index 0b90497d4424..6505ec81f1da 100644
> --- a/arch/arm64/lib/copy_from_user.S
> +++ b/arch/arm64/lib/copy_from_user.S
> @@ -16,11 +16,11 @@
>
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cache.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
Likewise, we can also drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h> here.
[...]
> diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S
> index f7292dd08c84..9b04ff3ab610 100644
> --- a/arch/arm64/lib/copy_in_user.S
> +++ b/arch/arm64/lib/copy_in_user.S
> @@ -18,11 +18,11 @@
>
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cache.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
Likewise, we can also drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h> here.
[...]
> diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
> index 7a7efe255034..8077e4f34d56 100644
> --- a/arch/arm64/lib/copy_to_user.S
> +++ b/arch/arm64/lib/copy_to_user.S
> @@ -16,11 +16,11 @@
>
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> #include <asm/assembler.h>
> #include <asm/cache.h>
> #include <asm/cpufeature.h>
> #include <asm/sysreg.h>
> +#include <asm/uaccess.h>
Likewise, we can also drop <asm/assembler.h>, <asm/cpufeature.h>, and
<asm/sysreg.h> here.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 2/7] arm64: Factor out TTBR0_EL1 post-update workaround into a specific asm macro
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-13 17:46 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
This patch takes the errata workaround code out of cpu_do_switch_mm into
a dedicated post_ttbr0_update_workaround 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>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 17 +++++++++++++++++
arch/arm64/mm/proc.S | 11 +----------
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index d5025c69ca81..0a47632787d9 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -350,4 +350,21 @@ alternative_endif
movk \reg, :abs_g0_nc:\val
.endm
+/*
+ * Errata workaround post TTBR0_EL1 update.
+ */
+ .macro post_ttbr0_update_workaround
+#ifdef CONFIG_CAVIUM_ERRATUM_27456
+alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
+ nop
+ nop
+ nop
+alternative_else
+ ic iallu
+ dsb nsh
+ isb
+alternative_endif
+#endif
+ .endm
+
#endif /* __ASM_ASSEMBLER_H */
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5bb61de23201..8292784d44c9 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -125,17 +125,8 @@ ENTRY(cpu_do_switch_mm)
bfi x0, x1, #48, #16 // set the ASID
msr ttbr0_el1, x0 // set TTBR0
isb
-alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
+ post_ttbr0_update_workaround
ret
- nop
- nop
- nop
-alternative_else
- ic iallu
- dsb nsh
- isb
- ret
-alternative_endif
ENDPROC(cpu_do_switch_mm)
.pushsection ".idmap.text", "ax"
^ permalink raw reply related [flat|nested] 64+ messages in thread
* [kernel-hardening] [PATCH v3 2/7] arm64: Factor out TTBR0_EL1 post-update workaround into a specific asm macro
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
This patch takes the errata workaround code out of cpu_do_switch_mm into
a dedicated post_ttbr0_update_workaround 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>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 17 +++++++++++++++++
arch/arm64/mm/proc.S | 11 +----------
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index d5025c69ca81..0a47632787d9 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -350,4 +350,21 @@ alternative_endif
movk \reg, :abs_g0_nc:\val
.endm
+/*
+ * Errata workaround post TTBR0_EL1 update.
+ */
+ .macro post_ttbr0_update_workaround
+#ifdef CONFIG_CAVIUM_ERRATUM_27456
+alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
+ nop
+ nop
+ nop
+alternative_else
+ ic iallu
+ dsb nsh
+ isb
+alternative_endif
+#endif
+ .endm
+
#endif /* __ASM_ASSEMBLER_H */
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5bb61de23201..8292784d44c9 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -125,17 +125,8 @@ ENTRY(cpu_do_switch_mm)
bfi x0, x1, #48, #16 // set the ASID
msr ttbr0_el1, x0 // set TTBR0
isb
-alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
+ post_ttbr0_update_workaround
ret
- nop
- nop
- nop
-alternative_else
- ic iallu
- dsb nsh
- isb
- ret
-alternative_endif
ENDPROC(cpu_do_switch_mm)
.pushsection ".idmap.text", "ax"
^ permalink raw reply related [flat|nested] 64+ messages in thread
* [PATCH v3 2/7] arm64: Factor out TTBR0_EL1 post-update workaround into a specific asm macro
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-15 15:19 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 15:19 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 13, 2016 at 06:46:32PM +0100, Catalin Marinas wrote:
> This patch takes the errata workaround code out of cpu_do_switch_mm into
> a dedicated post_ttbr0_update_workaround 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>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/assembler.h | 17 +++++++++++++++++
> arch/arm64/mm/proc.S | 11 +----------
> 2 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index d5025c69ca81..0a47632787d9 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -350,4 +350,21 @@ alternative_endif
> movk \reg, :abs_g0_nc:\val
> .endm
>
> +/*
> + * Errata workaround post TTBR0_EL1 update.
> + */
> + .macro post_ttbr0_update_workaround
> +#ifdef CONFIG_CAVIUM_ERRATUM_27456
> +alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
> + nop
> + nop
> + nop
> +alternative_else
> + ic iallu
> + dsb nsh
> + isb
> +alternative_endif
> +#endif
> + .endm
> +
> #endif /* __ASM_ASSEMBLER_H */
As a general note, it's unfortunate that we put asm with alternatives in
<asm/assembler.h>, as that's included by <asm/alternative.h>, so we're
relying on things that aren't defined yet, and users have to remember to
include <asm/alternative.h>.
That's already the case for dcache_by_line_op, so I guess that's
something to deal with another day.
Otherwise, this looks good, and I guess this wil be auto-nopped in a
subsqeuent version, so FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 2/7] arm64: Factor out TTBR0_EL1 post-update workaround into a specific asm macro
@ 2016-09-15 15:19 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 15:19 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, Will Deacon, James Morse, Kees Cook,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
On Tue, Sep 13, 2016 at 06:46:32PM +0100, Catalin Marinas wrote:
> This patch takes the errata workaround code out of cpu_do_switch_mm into
> a dedicated post_ttbr0_update_workaround 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>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/assembler.h | 17 +++++++++++++++++
> arch/arm64/mm/proc.S | 11 +----------
> 2 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index d5025c69ca81..0a47632787d9 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -350,4 +350,21 @@ alternative_endif
> movk \reg, :abs_g0_nc:\val
> .endm
>
> +/*
> + * Errata workaround post TTBR0_EL1 update.
> + */
> + .macro post_ttbr0_update_workaround
> +#ifdef CONFIG_CAVIUM_ERRATUM_27456
> +alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
> + nop
> + nop
> + nop
> +alternative_else
> + ic iallu
> + dsb nsh
> + isb
> +alternative_endif
> +#endif
> + .endm
> +
> #endif /* __ASM_ASSEMBLER_H */
As a general note, it's unfortunate that we put asm with alternatives in
<asm/assembler.h>, as that's included by <asm/alternative.h>, so we're
relying on things that aren't defined yet, and users have to remember to
include <asm/alternative.h>.
That's already the case for dcache_by_line_op, so I guess that's
something to deal with another day.
Otherwise, this looks good, and I guess this wil be auto-nopped in a
subsqeuent version, so FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 3/7] arm64: Introduce uaccess_{disable, enable} functionality based on TTBR0_EL1
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-13 17:46 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 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
patch 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 the struct thread_info ttbr0 variable. Interrupts must be disabled
during the uaccess_ttbr0_enable code to ensure the atomicity of the
thread_info.ttbr0 read and TTBR0_EL1 write. This patch also moves the
get_thread_info asm macro from entry.S to assembler.h for reuse in the
uaccess_ttbr0_* macros.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 16 ++++++
arch/arm64/include/asm/cpufeature.h | 6 +++
arch/arm64/include/asm/kernel-pgtable.h | 7 +++
arch/arm64/include/asm/thread_info.h | 3 ++
arch/arm64/include/asm/uaccess.h | 96 ++++++++++++++++++++++++++++++---
arch/arm64/kernel/asm-offsets.c | 3 ++
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/entry.S | 4 --
arch/arm64/kernel/head.S | 6 +--
arch/arm64/kernel/vmlinux.lds.S | 5 ++
10 files changed, 134 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 0a47632787d9..581ee4ab2a34 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -41,6 +41,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.
*/
@@ -351,6 +360,13 @@ alternative_endif
.endm
/*
+ * Return the current thread_info.
+ */
+ .macro get_thread_info, rd
+ mrs \rd, sp_el0
+ .endm
+
+/*
* Errata workaround post TTBR0_EL1 update.
*/
.macro post_ttbr0_update_workaround
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 7099f26e3702..042d49c7b231 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_uses_ttbr0_pan(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_SW_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..7803343e5881 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_SW_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/thread_info.h b/arch/arm64/include/asm/thread_info.h
index abd64bd1f6d9..b3325a9cb90f 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -47,6 +47,9 @@ typedef unsigned long mm_segment_t;
struct thread_info {
unsigned long flags; /* low level flags */
mm_segment_t addr_limit; /* address limit */
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ u64 ttbr0; /* saved TTBR0_EL1 */
+#endif
struct task_struct *task; /* main task structure */
int preempt_count; /* 0 => preemptable, <0 => bug */
int cpu; /* cpu */
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index cc6c32d4dcc4..115b5fa8dc3f 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -29,6 +29,7 @@
#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>
@@ -116,16 +117,57 @@ static inline void set_fs(mm_segment_t fs)
/*
* User access enabling/disabling.
*/
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+static inline void uaccess_ttbr0_disable(void)
+{
+ unsigned long ttbr;
+
+ /* reserved_ttbr0 placed at the end of swapper_pg_dir */
+ ttbr = read_sysreg(ttbr1_el1) + SWAPPER_DIR_SIZE;
+ write_sysreg(ttbr, ttbr0_el1);
+ isb();
+}
+
+static inline void uaccess_ttbr0_enable(void)
+{
+ unsigned long flags;
+
+ /*
+ * Disable interrupts to avoid preemption between reading the 'ttbr0'
+ * variable and the MSR. A context switch could trigger an ASID
+ * roll-over and an update of 'ttbr0'.
+ */
+ local_irq_save(flags);
+ write_sysreg(current_thread_info()->ttbr0, ttbr0_el1);
+ isb();
+ local_irq_restore(flags);
+}
+#else
+static inline void uaccess_ttbr0_disable(void)
+{
+}
+
+static inline void uaccess_ttbr0_enable(void)
+{
+}
+#endif
+
#define __uaccess_disable(alt) \
do { \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
- CONFIG_ARM64_PAN)); \
+ if (system_uses_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_uses_ttbr0_pan()) \
+ uaccess_ttbr0_enable(); \
+ else \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
+ CONFIG_ARM64_PAN)); \
} while (0)
static inline void uaccess_disable(void)
@@ -361,12 +403,39 @@ extern __must_check long strnlen_user(const char __user *str, long n);
#include <asm/alternative.h>
#include <asm/assembler.h>
+#include <asm/kernel-pgtable.h>
+
+/*
+ * 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
+ msr ttbr0_el1, \tmp1 // set reserved TTBR0_EL1
+ isb
+ .endm
+
+ .macro uaccess_ttbr0_enable, tmp1
+ get_thread_info \tmp1
+ ldr \tmp1, [\tmp1, #TI_TTBR0] // load saved TTBR0_EL1
+ msr ttbr0_el1, \tmp1 // set the non-PAN TTBR0_EL1
+ isb
+ .endm
/*
- * User access enabling/disabling macros. These are no-ops when UAO is
- * present.
+ * These macros are no-ops when UAO is present.
*/
.macro uaccess_disable_not_uao, tmp1
+#ifdef CONFIG_ARM64_SW_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
@@ -375,6 +444,21 @@ alternative_endif
.endm
.macro uaccess_enable_not_uao, tmp1, tmp2
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+alternative_if_not ARM64_HAS_PAN
+ save_and_disable_irq \tmp2 // avoid preemption
+ uaccess_ttbr0_enable \tmp1
+ restore_irq \tmp2
+alternative_else
+ 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/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 05070b72fc28..82f85af070f8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -38,6 +38,9 @@ int main(void)
DEFINE(TI_FLAGS, offsetof(struct thread_info, flags));
DEFINE(TI_PREEMPT, offsetof(struct thread_info, preempt_count));
DEFINE(TI_ADDR_LIMIT, offsetof(struct thread_info, addr_limit));
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ DEFINE(TI_TTBR0, offsetof(struct thread_info, ttbr0));
+#endif
DEFINE(TI_TASK, offsetof(struct thread_info, task));
DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
BLANK();
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/entry.S b/arch/arm64/kernel/entry.S
index 441420ca7d08..be1e3987c07a 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -190,10 +190,6 @@ alternative_endif
eret // return to kernel
.endm
- .macro get_thread_info, rd
- mrs \rd, sp_el0
- .endm
-
.macro irq_stack_entry
mov x19, sp // preserve the original sp
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 3e7b050e99dc..d4188396302f 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..8c612d30ff3c 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_SW_TTBR0_PAN
+ reserved_ttbr0 = .;
+ . += RESERVED_TTBR0_SIZE;
+#endif
+
_end = .;
STABS_DEBUG
^ permalink raw reply related [flat|nested] 64+ messages in thread* [kernel-hardening] [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
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
patch 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 the struct thread_info ttbr0 variable. Interrupts must be disabled
during the uaccess_ttbr0_enable code to ensure the atomicity of the
thread_info.ttbr0 read and TTBR0_EL1 write. This patch also moves the
get_thread_info asm macro from entry.S to assembler.h for reuse in the
uaccess_ttbr0_* macros.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 16 ++++++
arch/arm64/include/asm/cpufeature.h | 6 +++
arch/arm64/include/asm/kernel-pgtable.h | 7 +++
arch/arm64/include/asm/thread_info.h | 3 ++
arch/arm64/include/asm/uaccess.h | 96 ++++++++++++++++++++++++++++++---
arch/arm64/kernel/asm-offsets.c | 3 ++
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/entry.S | 4 --
arch/arm64/kernel/head.S | 6 +--
arch/arm64/kernel/vmlinux.lds.S | 5 ++
10 files changed, 134 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 0a47632787d9..581ee4ab2a34 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -41,6 +41,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.
*/
@@ -351,6 +360,13 @@ alternative_endif
.endm
/*
+ * Return the current thread_info.
+ */
+ .macro get_thread_info, rd
+ mrs \rd, sp_el0
+ .endm
+
+/*
* Errata workaround post TTBR0_EL1 update.
*/
.macro post_ttbr0_update_workaround
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 7099f26e3702..042d49c7b231 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_uses_ttbr0_pan(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_SW_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..7803343e5881 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_SW_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/thread_info.h b/arch/arm64/include/asm/thread_info.h
index abd64bd1f6d9..b3325a9cb90f 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -47,6 +47,9 @@ typedef unsigned long mm_segment_t;
struct thread_info {
unsigned long flags; /* low level flags */
mm_segment_t addr_limit; /* address limit */
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ u64 ttbr0; /* saved TTBR0_EL1 */
+#endif
struct task_struct *task; /* main task structure */
int preempt_count; /* 0 => preemptable, <0 => bug */
int cpu; /* cpu */
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index cc6c32d4dcc4..115b5fa8dc3f 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -29,6 +29,7 @@
#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>
@@ -116,16 +117,57 @@ static inline void set_fs(mm_segment_t fs)
/*
* User access enabling/disabling.
*/
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+static inline void uaccess_ttbr0_disable(void)
+{
+ unsigned long ttbr;
+
+ /* reserved_ttbr0 placed at the end of swapper_pg_dir */
+ ttbr = read_sysreg(ttbr1_el1) + SWAPPER_DIR_SIZE;
+ write_sysreg(ttbr, ttbr0_el1);
+ isb();
+}
+
+static inline void uaccess_ttbr0_enable(void)
+{
+ unsigned long flags;
+
+ /*
+ * Disable interrupts to avoid preemption between reading the 'ttbr0'
+ * variable and the MSR. A context switch could trigger an ASID
+ * roll-over and an update of 'ttbr0'.
+ */
+ local_irq_save(flags);
+ write_sysreg(current_thread_info()->ttbr0, ttbr0_el1);
+ isb();
+ local_irq_restore(flags);
+}
+#else
+static inline void uaccess_ttbr0_disable(void)
+{
+}
+
+static inline void uaccess_ttbr0_enable(void)
+{
+}
+#endif
+
#define __uaccess_disable(alt) \
do { \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
- CONFIG_ARM64_PAN)); \
+ if (system_uses_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_uses_ttbr0_pan()) \
+ uaccess_ttbr0_enable(); \
+ else \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
+ CONFIG_ARM64_PAN)); \
} while (0)
static inline void uaccess_disable(void)
@@ -361,12 +403,39 @@ extern __must_check long strnlen_user(const char __user *str, long n);
#include <asm/alternative.h>
#include <asm/assembler.h>
+#include <asm/kernel-pgtable.h>
+
+/*
+ * 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
+ msr ttbr0_el1, \tmp1 // set reserved TTBR0_EL1
+ isb
+ .endm
+
+ .macro uaccess_ttbr0_enable, tmp1
+ get_thread_info \tmp1
+ ldr \tmp1, [\tmp1, #TI_TTBR0] // load saved TTBR0_EL1
+ msr ttbr0_el1, \tmp1 // set the non-PAN TTBR0_EL1
+ isb
+ .endm
/*
- * User access enabling/disabling macros. These are no-ops when UAO is
- * present.
+ * These macros are no-ops when UAO is present.
*/
.macro uaccess_disable_not_uao, tmp1
+#ifdef CONFIG_ARM64_SW_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
@@ -375,6 +444,21 @@ alternative_endif
.endm
.macro uaccess_enable_not_uao, tmp1, tmp2
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+alternative_if_not ARM64_HAS_PAN
+ save_and_disable_irq \tmp2 // avoid preemption
+ uaccess_ttbr0_enable \tmp1
+ restore_irq \tmp2
+alternative_else
+ 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/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 05070b72fc28..82f85af070f8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -38,6 +38,9 @@ int main(void)
DEFINE(TI_FLAGS, offsetof(struct thread_info, flags));
DEFINE(TI_PREEMPT, offsetof(struct thread_info, preempt_count));
DEFINE(TI_ADDR_LIMIT, offsetof(struct thread_info, addr_limit));
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ DEFINE(TI_TTBR0, offsetof(struct thread_info, ttbr0));
+#endif
DEFINE(TI_TASK, offsetof(struct thread_info, task));
DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
BLANK();
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/entry.S b/arch/arm64/kernel/entry.S
index 441420ca7d08..be1e3987c07a 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -190,10 +190,6 @@ alternative_endif
eret // return to kernel
.endm
- .macro get_thread_info, rd
- mrs \rd, sp_el0
- .endm
-
.macro irq_stack_entry
mov x19, sp // preserve the original sp
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 3e7b050e99dc..d4188396302f 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..8c612d30ff3c 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_SW_TTBR0_PAN
+ reserved_ttbr0 = .;
+ . += RESERVED_TTBR0_SIZE;
+#endif
+
_end = .;
STABS_DEBUG
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
2016-09-13 17:46 ` [kernel-hardening] [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} " Catalin Marinas
@ 2016-09-13 20:45 ` Kees Cook
-1 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-09-13 20:45 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> 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
> patch 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 the struct thread_info ttbr0 variable. Interrupts must be disabled
> during the uaccess_ttbr0_enable code to ensure the atomicity of the
> thread_info.ttbr0 read and TTBR0_EL1 write. This patch also moves the
> get_thread_info asm macro from entry.S to assembler.h for reuse in the
> uaccess_ttbr0_* macros.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> [...]
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 7099f26e3702..042d49c7b231 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_uses_ttbr0_pan(void)
> +{
> + return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
> + !cpus_have_cap(ARM64_HAS_PAN);
> +}
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
> [...]
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index cc6c32d4dcc4..115b5fa8dc3f 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> [...]
> @@ -116,16 +117,57 @@ static inline void set_fs(mm_segment_t fs)
> [...]
> #define __uaccess_disable(alt) \
> do { \
> - asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
> - CONFIG_ARM64_PAN)); \
> + if (system_uses_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_uses_ttbr0_pan()) \
> + uaccess_ttbr0_enable(); \
> + else \
> + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> + CONFIG_ARM64_PAN)); \
> } while (0)
Does this mean that with CONFIG_ARM64_SW_TTBR0_PAN, even with ARMv8.1,
a cpu capability bitmask check is done each time we go through
__uaccess_{en,dis}able?
Could the alternative get moved around slightly to avoid this, or am I
misunderstanding something here?
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
@ 2016-09-13 20:45 ` Kees Cook
0 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-09-13 20:45 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, Will Deacon, James Morse, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> 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
> patch 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 the struct thread_info ttbr0 variable. Interrupts must be disabled
> during the uaccess_ttbr0_enable code to ensure the atomicity of the
> thread_info.ttbr0 read and TTBR0_EL1 write. This patch also moves the
> get_thread_info asm macro from entry.S to assembler.h for reuse in the
> uaccess_ttbr0_* macros.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> [...]
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 7099f26e3702..042d49c7b231 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_uses_ttbr0_pan(void)
> +{
> + return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
> + !cpus_have_cap(ARM64_HAS_PAN);
> +}
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
> [...]
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index cc6c32d4dcc4..115b5fa8dc3f 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> [...]
> @@ -116,16 +117,57 @@ static inline void set_fs(mm_segment_t fs)
> [...]
> #define __uaccess_disable(alt) \
> do { \
> - asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
> - CONFIG_ARM64_PAN)); \
> + if (system_uses_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_uses_ttbr0_pan()) \
> + uaccess_ttbr0_enable(); \
> + else \
> + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> + CONFIG_ARM64_PAN)); \
> } while (0)
Does this mean that with CONFIG_ARM64_SW_TTBR0_PAN, even with ARMv8.1,
a cpu capability bitmask check is done each time we go through
__uaccess_{en,dis}able?
Could the alternative get moved around slightly to avoid this, or am I
misunderstanding something here?
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
2016-09-13 20:45 ` [kernel-hardening] " Kees Cook
@ 2016-09-14 8:52 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 8:52 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 13, 2016 at 01:45:21PM -0700, Kees Cook wrote:
> On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
> > +static inline bool system_uses_ttbr0_pan(void)
> > +{
> > + return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
> > + !cpus_have_cap(ARM64_HAS_PAN);
> > +}
> > +
[...]
> > #define __uaccess_enable(alt) \
> > do { \
> > - asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> > - CONFIG_ARM64_PAN)); \
> > + if (system_uses_ttbr0_pan()) \
> > + uaccess_ttbr0_enable(); \
> > + else \
> > + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> > + CONFIG_ARM64_PAN)); \
> > } while (0)
>
> Does this mean that with CONFIG_ARM64_SW_TTBR0_PAN, even with ARMv8.1,
> a cpu capability bitmask check is done each time we go through
> __uaccess_{en,dis}able?
Catalin reworked cpus_have_cap() to use static keys [1], and that's
queued in the arm64 for-next/core branch [2].
So this should expand to a single branch or nop that we patch when we
detect the presence/absence of PAN. There should be no bitmap check.
Thanks,
Mark.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454025.html
[2] https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
@ 2016-09-14 8:52 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 8:52 UTC (permalink / raw)
To: Kees Cook
Cc: Catalin Marinas, linux-arm-kernel, Will Deacon, James Morse,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
On Tue, Sep 13, 2016 at 01:45:21PM -0700, Kees Cook wrote:
> On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
> > +static inline bool system_uses_ttbr0_pan(void)
> > +{
> > + return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
> > + !cpus_have_cap(ARM64_HAS_PAN);
> > +}
> > +
[...]
> > #define __uaccess_enable(alt) \
> > do { \
> > - asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> > - CONFIG_ARM64_PAN)); \
> > + if (system_uses_ttbr0_pan()) \
> > + uaccess_ttbr0_enable(); \
> > + else \
> > + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> > + CONFIG_ARM64_PAN)); \
> > } while (0)
>
> Does this mean that with CONFIG_ARM64_SW_TTBR0_PAN, even with ARMv8.1,
> a cpu capability bitmask check is done each time we go through
> __uaccess_{en,dis}able?
Catalin reworked cpus_have_cap() to use static keys [1], and that's
queued in the arm64 for-next/core branch [2].
So this should expand to a single branch or nop that we patch when we
detect the presence/absence of PAN. There should be no bitmap check.
Thanks,
Mark.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454025.html
[2] https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
2016-09-14 8:52 ` [kernel-hardening] " Mark Rutland
@ 2016-09-14 16:27 ` Kees Cook
-1 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-09-14 16:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 14, 2016 at 1:52 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Sep 13, 2016 at 01:45:21PM -0700, Kees Cook wrote:
>> On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
>> > +static inline bool system_uses_ttbr0_pan(void)
>> > +{
>> > + return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
>> > + !cpus_have_cap(ARM64_HAS_PAN);
>> > +}
>> > +
>
> [...]
>
>> > #define __uaccess_enable(alt) \
>> > do { \
>> > - asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
>> > - CONFIG_ARM64_PAN)); \
>> > + if (system_uses_ttbr0_pan()) \
>> > + uaccess_ttbr0_enable(); \
>> > + else \
>> > + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
>> > + CONFIG_ARM64_PAN)); \
>> > } while (0)
>>
>> Does this mean that with CONFIG_ARM64_SW_TTBR0_PAN, even with ARMv8.1,
>> a cpu capability bitmask check is done each time we go through
>> __uaccess_{en,dis}able?
>
> Catalin reworked cpus_have_cap() to use static keys [1], and that's
> queued in the arm64 for-next/core branch [2].
Oh awesome! Okay, thanks.
> So this should expand to a single branch or nop that we patch when we
> detect the presence/absence of PAN. There should be no bitmap check.
/me is looking forward to v4.9 :)
>
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454025.html
> [2] https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [kernel-hardening] Re: [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
@ 2016-09-14 16:27 ` Kees Cook
0 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-09-14 16:27 UTC (permalink / raw)
To: kernel-hardening
Cc: Catalin Marinas, linux-arm-kernel, Will Deacon, James Morse,
Ard Biesheuvel, AKASHI Takahiro
On Wed, Sep 14, 2016 at 1:52 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Sep 13, 2016 at 01:45:21PM -0700, Kees Cook wrote:
>> On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
>> > +static inline bool system_uses_ttbr0_pan(void)
>> > +{
>> > + return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
>> > + !cpus_have_cap(ARM64_HAS_PAN);
>> > +}
>> > +
>
> [...]
>
>> > #define __uaccess_enable(alt) \
>> > do { \
>> > - asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
>> > - CONFIG_ARM64_PAN)); \
>> > + if (system_uses_ttbr0_pan()) \
>> > + uaccess_ttbr0_enable(); \
>> > + else \
>> > + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
>> > + CONFIG_ARM64_PAN)); \
>> > } while (0)
>>
>> Does this mean that with CONFIG_ARM64_SW_TTBR0_PAN, even with ARMv8.1,
>> a cpu capability bitmask check is done each time we go through
>> __uaccess_{en,dis}able?
>
> Catalin reworked cpus_have_cap() to use static keys [1], and that's
> queued in the arm64 for-next/core branch [2].
Oh awesome! Okay, thanks.
> So this should expand to a single branch or nop that we patch when we
> detect the presence/absence of PAN. There should be no bitmap check.
/me is looking forward to v4.9 :)
>
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454025.html
> [2] https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-13 17:46 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 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.
This patch also removes a stale comment on the switch_mm() function.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/efi.h | 26 +++++++++++++-
arch/arm64/include/asm/mmu_context.h | 51 +++++++++++++++++++--------
arch/arm64/include/asm/ptrace.h | 2 ++
arch/arm64/kernel/entry.S | 67 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/setup.c | 9 +++++
arch/arm64/mm/context.c | 7 +++-
6 files changed, 146 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a9e54aad15ef..3a405dccb6cf 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>
@@ -75,7 +76,30 @@ 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);
+ __switch_mm(mm);
+
+ if (system_uses_ttbr0_pan()) {
+ if (mm != current->active_mm) {
+ /*
+ * Update the current thread's saved ttbr0 since it is
+ * restored as part of a return from exception. Set
+ * the hardware TTBR0_EL1 using cpu_switch_mm()
+ * directly to enable potential errata workarounds.
+ */
+ update_saved_ttbr0(current, mm);
+ cpu_switch_mm(mm->pgd, mm);
+ } else {
+ /*
+ * Defer the switch to the current thread's TTBR0_EL1
+ * until uaccess_enable(). Restore the current
+ * thread's saved ttbr0 corresponding to its active_mm
+ * (if different from init_mm).
+ */
+ cpu_set_reserved_ttbr0();
+ if (current->active_mm != &init_mm)
+ update_saved_ttbr0(current, current->active_mm);
+ }
+ }
}
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..da0ecca8353c 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_uses_ttbr0_pan())
cpu_switch_mm(mm->pgd, mm);
}
@@ -173,20 +174,26 @@ enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
{
}
-/*
- * This is the actual mm switch as far as the scheduler
- * is concerned. No registers are touched. We avoid
- * calling the CPU specific function when the mm hasn't
- * actually changed.
- */
-static inline void
-switch_mm(struct mm_struct *prev, struct mm_struct *next,
- struct task_struct *tsk)
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+static inline void update_saved_ttbr0(struct task_struct *tsk,
+ struct mm_struct *mm)
{
- unsigned int cpu = smp_processor_id();
+ if (system_uses_ttbr0_pan()) {
+ BUG_ON(mm->pgd == swapper_pg_dir);
+ task_thread_info(tsk)->ttbr0 =
+ virt_to_phys(mm->pgd) | ASID(mm) << 48;
+ }
+}
+#else
+static inline void update_saved_ttbr0(struct task_struct *tsk,
+ struct mm_struct *mm)
+{
+}
+#endif
- if (prev == next)
- return;
+static inline void __switch_mm(struct mm_struct *next)
+{
+ unsigned int cpu = smp_processor_id();
/*
* init_mm.pgd does not contain any user mappings and it is always
@@ -200,8 +207,24 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
check_and_switch_context(next, cpu);
}
+static inline void
+switch_mm(struct mm_struct *prev, struct mm_struct *next,
+ struct task_struct *tsk)
+{
+ if (prev != next)
+ __switch_mm(next);
+
+ /*
+ * Update the saved TTBR0_EL1 of the scheduled-in task as the previous
+ * value may have not been initialised yet (activate_mm caller) or the
+ * ASID has changed since the last run (following the context switch
+ * of another thread of the same process).
+ */
+ update_saved_ttbr0(tsk, next);
+}
+
#define deactivate_mm(tsk,mm) do { } while (0)
-#define activate_mm(prev,next) switch_mm(prev, next, NULL)
+#define activate_mm(prev,next) switch_mm(prev, next, current)
void verify_cpu_asid_bits(void);
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index ada08b5b036d..458773ac5ec9 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -21,6 +21,8 @@
#include <uapi/asm/ptrace.h>
+#define _PSR_PAN_BIT 22
+
/* Current Exception Level values, as contained in CurrentEL */
#define CurrentEL_EL1 (1 << 2)
#define CurrentEL_EL2 (2 << 2)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index be1e3987c07a..2bbb5756f621 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -29,7 +29,9 @@
#include <asm/esr.h>
#include <asm/irq.h>
#include <asm/memory.h>
+#include <asm/ptrace.h>
#include <asm/thread_info.h>
+#include <asm/uaccess.h>
#include <asm/unistd.h>
/*
@@ -109,6 +111,34 @@
mrs x22, elr_el1
mrs x23, spsr_el1
stp lr, x21, [sp, #S_LR]
+
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Set the TTBR0 PAN bit 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 x21, ttbr0_el1
+ tst x21, #0xffff << 48 // Check for the reserved ASID
+ orr x23, x23, #PSR_PAN_BIT // Set the emulated PAN in the saved SPSR
+ b.eq 1f // TTBR0 access already disabled
+ and x23, x23, #~PSR_PAN_BIT // Clear the emulated PAN in the saved SPSR
+ .endif
+
+ uaccess_ttbr0_disable x21
+1:
+#endif
+
stp x22, x23, [sp, #S_PC]
/*
@@ -147,6 +177,42 @@
ldp x21, x22, [sp, #S_PC] // load ELR, SPSR
.if \el == 0
ct_user_enter
+ .endif
+
+#ifdef CONFIG_ARM64_SW_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 // Skip re-enabling TTBR0 access if previously disabled
+ .endif
+
+ uaccess_ttbr0_enable x0
+
+ .if \el == 0
+ /*
+ * 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).
+ */
+ post_ttbr0_update_workaround
+ .endif
+1:
+ .if \el != 0
+ and x22, x22, #~PSR_PAN_BIT // ARMv8.0 CPUs do not understand this bit
+ .endif
+2:
+#endif
+
+ .if \el == 0
ldr x23, [sp, #S_SP] // load return stack pointer
msr sp_el0, x23
#ifdef CONFIG_ARM64_ERRATUM_845719
@@ -168,6 +234,7 @@ alternative_else
alternative_endif
#endif
.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/setup.c b/arch/arm64/kernel/setup.c
index 536dce22fe76..59b900d42cec 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -286,6 +286,15 @@ void __init setup_arch(char **cmdline_p)
smp_init_cpus();
smp_build_mpidr_hash();
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Make sure init_thread_info.ttbr0 always generates translation
+ * faults in case uaccess_enable() is inadvertently called by the init
+ * thread.
+ */
+ init_thread_info.ttbr0 = virt_to_phys(empty_zero_page);
+#endif
+
#ifdef CONFIG_VT
#if defined(CONFIG_VGA_CONSOLE)
conswitchp = &vga_con;
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index efcf1f7ef1e4..4c63cb154859 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -221,7 +221,12 @@ 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);
+ /*
+ * Defer TTBR0_EL1 setting for user threads to uaccess_enable() when
+ * emulating PAN.
+ */
+ if (!system_uses_ttbr0_pan())
+ cpu_switch_mm(mm->pgd, mm);
}
static int asids_init(void)
^ permalink raw reply related [flat|nested] 64+ messages in thread* [kernel-hardening] [PATCH v3 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
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.
This patch also removes a stale comment on the switch_mm() function.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/efi.h | 26 +++++++++++++-
arch/arm64/include/asm/mmu_context.h | 51 +++++++++++++++++++--------
arch/arm64/include/asm/ptrace.h | 2 ++
arch/arm64/kernel/entry.S | 67 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/setup.c | 9 +++++
arch/arm64/mm/context.c | 7 +++-
6 files changed, 146 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a9e54aad15ef..3a405dccb6cf 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>
@@ -75,7 +76,30 @@ 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);
+ __switch_mm(mm);
+
+ if (system_uses_ttbr0_pan()) {
+ if (mm != current->active_mm) {
+ /*
+ * Update the current thread's saved ttbr0 since it is
+ * restored as part of a return from exception. Set
+ * the hardware TTBR0_EL1 using cpu_switch_mm()
+ * directly to enable potential errata workarounds.
+ */
+ update_saved_ttbr0(current, mm);
+ cpu_switch_mm(mm->pgd, mm);
+ } else {
+ /*
+ * Defer the switch to the current thread's TTBR0_EL1
+ * until uaccess_enable(). Restore the current
+ * thread's saved ttbr0 corresponding to its active_mm
+ * (if different from init_mm).
+ */
+ cpu_set_reserved_ttbr0();
+ if (current->active_mm != &init_mm)
+ update_saved_ttbr0(current, current->active_mm);
+ }
+ }
}
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..da0ecca8353c 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_uses_ttbr0_pan())
cpu_switch_mm(mm->pgd, mm);
}
@@ -173,20 +174,26 @@ enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
{
}
-/*
- * This is the actual mm switch as far as the scheduler
- * is concerned. No registers are touched. We avoid
- * calling the CPU specific function when the mm hasn't
- * actually changed.
- */
-static inline void
-switch_mm(struct mm_struct *prev, struct mm_struct *next,
- struct task_struct *tsk)
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+static inline void update_saved_ttbr0(struct task_struct *tsk,
+ struct mm_struct *mm)
{
- unsigned int cpu = smp_processor_id();
+ if (system_uses_ttbr0_pan()) {
+ BUG_ON(mm->pgd == swapper_pg_dir);
+ task_thread_info(tsk)->ttbr0 =
+ virt_to_phys(mm->pgd) | ASID(mm) << 48;
+ }
+}
+#else
+static inline void update_saved_ttbr0(struct task_struct *tsk,
+ struct mm_struct *mm)
+{
+}
+#endif
- if (prev == next)
- return;
+static inline void __switch_mm(struct mm_struct *next)
+{
+ unsigned int cpu = smp_processor_id();
/*
* init_mm.pgd does not contain any user mappings and it is always
@@ -200,8 +207,24 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
check_and_switch_context(next, cpu);
}
+static inline void
+switch_mm(struct mm_struct *prev, struct mm_struct *next,
+ struct task_struct *tsk)
+{
+ if (prev != next)
+ __switch_mm(next);
+
+ /*
+ * Update the saved TTBR0_EL1 of the scheduled-in task as the previous
+ * value may have not been initialised yet (activate_mm caller) or the
+ * ASID has changed since the last run (following the context switch
+ * of another thread of the same process).
+ */
+ update_saved_ttbr0(tsk, next);
+}
+
#define deactivate_mm(tsk,mm) do { } while (0)
-#define activate_mm(prev,next) switch_mm(prev, next, NULL)
+#define activate_mm(prev,next) switch_mm(prev, next, current)
void verify_cpu_asid_bits(void);
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index ada08b5b036d..458773ac5ec9 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -21,6 +21,8 @@
#include <uapi/asm/ptrace.h>
+#define _PSR_PAN_BIT 22
+
/* Current Exception Level values, as contained in CurrentEL */
#define CurrentEL_EL1 (1 << 2)
#define CurrentEL_EL2 (2 << 2)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index be1e3987c07a..2bbb5756f621 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -29,7 +29,9 @@
#include <asm/esr.h>
#include <asm/irq.h>
#include <asm/memory.h>
+#include <asm/ptrace.h>
#include <asm/thread_info.h>
+#include <asm/uaccess.h>
#include <asm/unistd.h>
/*
@@ -109,6 +111,34 @@
mrs x22, elr_el1
mrs x23, spsr_el1
stp lr, x21, [sp, #S_LR]
+
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Set the TTBR0 PAN bit 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 x21, ttbr0_el1
+ tst x21, #0xffff << 48 // Check for the reserved ASID
+ orr x23, x23, #PSR_PAN_BIT // Set the emulated PAN in the saved SPSR
+ b.eq 1f // TTBR0 access already disabled
+ and x23, x23, #~PSR_PAN_BIT // Clear the emulated PAN in the saved SPSR
+ .endif
+
+ uaccess_ttbr0_disable x21
+1:
+#endif
+
stp x22, x23, [sp, #S_PC]
/*
@@ -147,6 +177,42 @@
ldp x21, x22, [sp, #S_PC] // load ELR, SPSR
.if \el == 0
ct_user_enter
+ .endif
+
+#ifdef CONFIG_ARM64_SW_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 // Skip re-enabling TTBR0 access if previously disabled
+ .endif
+
+ uaccess_ttbr0_enable x0
+
+ .if \el == 0
+ /*
+ * 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).
+ */
+ post_ttbr0_update_workaround
+ .endif
+1:
+ .if \el != 0
+ and x22, x22, #~PSR_PAN_BIT // ARMv8.0 CPUs do not understand this bit
+ .endif
+2:
+#endif
+
+ .if \el == 0
ldr x23, [sp, #S_SP] // load return stack pointer
msr sp_el0, x23
#ifdef CONFIG_ARM64_ERRATUM_845719
@@ -168,6 +234,7 @@ alternative_else
alternative_endif
#endif
.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/setup.c b/arch/arm64/kernel/setup.c
index 536dce22fe76..59b900d42cec 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -286,6 +286,15 @@ void __init setup_arch(char **cmdline_p)
smp_init_cpus();
smp_build_mpidr_hash();
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Make sure init_thread_info.ttbr0 always generates translation
+ * faults in case uaccess_enable() is inadvertently called by the init
+ * thread.
+ */
+ init_thread_info.ttbr0 = virt_to_phys(empty_zero_page);
+#endif
+
#ifdef CONFIG_VT
#if defined(CONFIG_VGA_CONSOLE)
conswitchp = &vga_con;
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index efcf1f7ef1e4..4c63cb154859 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -221,7 +221,12 @@ 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);
+ /*
+ * Defer TTBR0_EL1 setting for user threads to uaccess_enable() when
+ * emulating PAN.
+ */
+ if (!system_uses_ttbr0_pan())
+ cpu_switch_mm(mm->pgd, mm);
}
static int asids_init(void)
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH v3 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-14 16:45 ` Will Deacon
-1 siblings, 0 replies; 64+ messages in thread
From: Will Deacon @ 2016-09-14 16:45 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 13, 2016 at 06:46:34PM +0100, Catalin Marinas wrote:
> 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.
>
> This patch also removes a stale comment on the switch_mm() function.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/efi.h | 26 +++++++++++++-
> arch/arm64/include/asm/mmu_context.h | 51 +++++++++++++++++++--------
> arch/arm64/include/asm/ptrace.h | 2 ++
> arch/arm64/kernel/entry.S | 67 ++++++++++++++++++++++++++++++++++++
> arch/arm64/kernel/setup.c | 9 +++++
> arch/arm64/mm/context.c | 7 +++-
> 6 files changed, 146 insertions(+), 16 deletions(-)
[...]
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index ada08b5b036d..458773ac5ec9 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -21,6 +21,8 @@
>
> #include <uapi/asm/ptrace.h>
>
> +#define _PSR_PAN_BIT 22
[...]
> + .if \el != 0
> + tbnz x22, #_PSR_PAN_BIT, 1f // Skip re-enabling TTBR0 access if previously disabled
> + .endif
I really don't like the duplication of the #defines, just because tbnz
takes a bit number rather than a mask. I think for now we should follow
the status quo and use the immediate with a comment (we already do this
with PSR_I_BIT), but perhaps as a followup we should adjust the UAPI
headers to generate the PSR masks using shifts instead?
Not taken a proper look at the series yet, but this jumped out at me :)
Will
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 4/7] arm64: Disable TTBR0_EL1 during normal kernel execution
@ 2016-09-14 16:45 ` Will Deacon
0 siblings, 0 replies; 64+ messages in thread
From: Will Deacon @ 2016-09-14 16:45 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
On Tue, Sep 13, 2016 at 06:46:34PM +0100, Catalin Marinas wrote:
> 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.
>
> This patch also removes a stale comment on the switch_mm() function.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/efi.h | 26 +++++++++++++-
> arch/arm64/include/asm/mmu_context.h | 51 +++++++++++++++++++--------
> arch/arm64/include/asm/ptrace.h | 2 ++
> arch/arm64/kernel/entry.S | 67 ++++++++++++++++++++++++++++++++++++
> arch/arm64/kernel/setup.c | 9 +++++
> arch/arm64/mm/context.c | 7 +++-
> 6 files changed, 146 insertions(+), 16 deletions(-)
[...]
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index ada08b5b036d..458773ac5ec9 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -21,6 +21,8 @@
>
> #include <uapi/asm/ptrace.h>
>
> +#define _PSR_PAN_BIT 22
[...]
> + .if \el != 0
> + tbnz x22, #_PSR_PAN_BIT, 1f // Skip re-enabling TTBR0 access if previously disabled
> + .endif
I really don't like the duplication of the #defines, just because tbnz
takes a bit number rather than a mask. I think for now we should follow
the status quo and use the immediate with a comment (we already do this
with PSR_I_BIT), but perhaps as a followup we should adjust the UAPI
headers to generate the PSR masks using shifts instead?
Not taken a proper look at the series yet, but this jumped out at me :)
Will
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-13 17:46 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 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>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/mm/fault.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 05d2bd776c69..2be46e6794e1 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -268,13 +268,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) ||
- (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
+ if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
+ return false;
+
+ if (system_uses_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)
@@ -314,7 +320,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);
@@ -506,10 +512,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] 64+ messages in thread* [kernel-hardening] [PATCH v3 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
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>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/mm/fault.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 05d2bd776c69..2be46e6794e1 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -268,13 +268,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) ||
- (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
+ if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
+ return false;
+
+ if (system_uses_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)
@@ -314,7 +320,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);
@@ -506,10 +512,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] 64+ messages in thread* [PATCH v3 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-16 11:33 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-16 11:33 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 13, 2016 at 06:46:35PM +0100, Catalin Marinas wrote:
> 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.
Why do we need to treat this case as a permission fault? It looks like a
fault that wasn't a deliberate uaccess (which thus doesn't have a fixup
handler) should have do_page_fault call __do_kernel_fault, where we
should die().
Is this just for more consistent reporting of erroneous accesses to user
memory? Or does this fix some other issue?
> This patch also updates the description of the synchronous external
> aborts on translation table walks.
This sounds fine, though it might be better as a separate/preparatory
patch.
> -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) ||
> - (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
> + if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
> + return false;
> +
> + if (system_uses_ttbr0_pan())
> + return fsc_type == ESR_ELx_FSC_FAULT &&
> + (regs->pstate & PSR_PAN_BIT);
> + else
> + return fsc_type == ESR_ELx_FSC_PERM;
> }
Since the entry code will clear the PAN bit in the SPSR when we see the
reserved ASID, faults in EFI runtime services will still be correctly
reported, though that's somewhat subtle (and it took me a while to
convince myself that was the case).
Also, I think that faults elsewhere may be misreported, e.g. in cold
entry to kernel paths (idle, hotplug) where we have a global mapping in
TTBR0, and it looks like we're using the reserved ASID.
I'm not immediately sure how to distinguish those cases.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled
@ 2016-09-16 11:33 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-16 11:33 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, Will Deacon, James Morse, Kees Cook,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
On Tue, Sep 13, 2016 at 06:46:35PM +0100, Catalin Marinas wrote:
> 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.
Why do we need to treat this case as a permission fault? It looks like a
fault that wasn't a deliberate uaccess (which thus doesn't have a fixup
handler) should have do_page_fault call __do_kernel_fault, where we
should die().
Is this just for more consistent reporting of erroneous accesses to user
memory? Or does this fix some other issue?
> This patch also updates the description of the synchronous external
> aborts on translation table walks.
This sounds fine, though it might be better as a separate/preparatory
patch.
> -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) ||
> - (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
> + if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
> + return false;
> +
> + if (system_uses_ttbr0_pan())
> + return fsc_type == ESR_ELx_FSC_FAULT &&
> + (regs->pstate & PSR_PAN_BIT);
> + else
> + return fsc_type == ESR_ELx_FSC_PERM;
> }
Since the entry code will clear the PAN bit in the SPSR when we see the
reserved ASID, faults in EFI runtime services will still be correctly
reported, though that's somewhat subtle (and it took me a while to
convince myself that was the case).
Also, I think that faults elsewhere may be misreported, e.g. in cold
entry to kernel paths (idle, hotplug) where we have a global mapping in
TTBR0, and it looks like we're using the reserved ASID.
I'm not immediately sure how to distinguish those cases.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH v3 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled
2016-09-16 11:33 ` [kernel-hardening] " Mark Rutland
@ 2016-09-16 15:55 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-16 15:55 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 16, 2016 at 12:33:25PM +0100, Mark Rutland wrote:
> On Tue, Sep 13, 2016 at 06:46:35PM +0100, Catalin Marinas wrote:
> > 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.
>
> Why do we need to treat this case as a permission fault? It looks like a
> fault that wasn't a deliberate uaccess (which thus doesn't have a fixup
> handler) should have do_page_fault call __do_kernel_fault, where we
> should die().
We don't always check the exception table for a dedicated uaccess. The
only cases where the exception table is checked is when
down_read_trylock(mmap_sem) fails or CONFIG_DEBUG_VM is enabled. This is
a slight optimisation of the fast path of the page fault handling. So,
without is_permission_fault() (which much quicker than
search_exception_tables()) the kernel would keep restarting the same
instruction.
> > -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) ||
> > - (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
> > + if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
> > + return false;
> > +
> > + if (system_uses_ttbr0_pan())
> > + return fsc_type == ESR_ELx_FSC_FAULT &&
> > + (regs->pstate & PSR_PAN_BIT);
> > + else
> > + return fsc_type == ESR_ELx_FSC_PERM;
> > }
>
> Since the entry code will clear the PAN bit in the SPSR when we see the
> reserved ASID,
It actually sets the PAN bit in regs->pstate when it sees the reserved
ASID.
> faults in EFI runtime services will still be correctly reported,
> though that's somewhat subtle (and it took me a while to convince
> myself that was the case).
The EFI runtime services use a non-reserved ASID, so a fault taken while
executing EFI services would clear the PAN bit in regs->pstate. Such
faults would not trigger is_permission_fault() == true above.
> Also, I think that faults elsewhere may be misreported, e.g. in cold
> entry to kernel paths (idle, hotplug) where we have a global mapping in
> TTBR0, and it looks like we're using the reserved ASID.
>
> I'm not immediately sure how to distinguish those cases.
We don't normally expect faults on those paths. If we get one, they are
usually fatal, so the kernel still dies but with a slightly misleading
message.
We could improve this I think if we can distinguished between
reserved_ttbr0 after swapper (set on exception entry) and the reserved
TTBR0_EL1 pointing to empty_zero_page (and not merging Ard's patch). Is
it worth having such distinction? I'm not convinced, the only thing you
probably save is not printing "Accessing user space memory outside
uaccess.h routines" during fault (there may be other ways to mark these
special cases maybe by checking the current thread_info but it needs
more thinking).
--
Catalin
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 5/7] arm64: Handle faults caused by inadvertent user access with PAN enabled
@ 2016-09-16 15:55 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-16 15:55 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Ard Biesheuvel, kernel-hardening, Will Deacon,
AKASHI Takahiro, James Morse, linux-arm-kernel
On Fri, Sep 16, 2016 at 12:33:25PM +0100, Mark Rutland wrote:
> On Tue, Sep 13, 2016 at 06:46:35PM +0100, Catalin Marinas wrote:
> > 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.
>
> Why do we need to treat this case as a permission fault? It looks like a
> fault that wasn't a deliberate uaccess (which thus doesn't have a fixup
> handler) should have do_page_fault call __do_kernel_fault, where we
> should die().
We don't always check the exception table for a dedicated uaccess. The
only cases where the exception table is checked is when
down_read_trylock(mmap_sem) fails or CONFIG_DEBUG_VM is enabled. This is
a slight optimisation of the fast path of the page fault handling. So,
without is_permission_fault() (which much quicker than
search_exception_tables()) the kernel would keep restarting the same
instruction.
> > -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) ||
> > - (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
> > + if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
> > + return false;
> > +
> > + if (system_uses_ttbr0_pan())
> > + return fsc_type == ESR_ELx_FSC_FAULT &&
> > + (regs->pstate & PSR_PAN_BIT);
> > + else
> > + return fsc_type == ESR_ELx_FSC_PERM;
> > }
>
> Since the entry code will clear the PAN bit in the SPSR when we see the
> reserved ASID,
It actually sets the PAN bit in regs->pstate when it sees the reserved
ASID.
> faults in EFI runtime services will still be correctly reported,
> though that's somewhat subtle (and it took me a while to convince
> myself that was the case).
The EFI runtime services use a non-reserved ASID, so a fault taken while
executing EFI services would clear the PAN bit in regs->pstate. Such
faults would not trigger is_permission_fault() == true above.
> Also, I think that faults elsewhere may be misreported, e.g. in cold
> entry to kernel paths (idle, hotplug) where we have a global mapping in
> TTBR0, and it looks like we're using the reserved ASID.
>
> I'm not immediately sure how to distinguish those cases.
We don't normally expect faults on those paths. If we get one, they are
usually fatal, so the kernel still dies but with a slightly misleading
message.
We could improve this I think if we can distinguished between
reserved_ttbr0 after swapper (set on exception entry) and the reserved
TTBR0_EL1 pointing to empty_zero_page (and not merging Ard's patch). Is
it worth having such distinction? I'm not convinced, the only thing you
probably save is not printing "Accessing user space memory outside
uaccess.h routines" during fault (there may be other ways to mark these
special cases maybe by checking the current thread_info but it needs
more thinking).
--
Catalin
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 6/7] arm64: xen: Enable user access before a privcmd hvc call
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-13 17:46 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 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.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/xen/hypercall.S | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 329c8027b0a9..a23b2e8f2647 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -49,6 +49,7 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
+#include <asm/uaccess.h>
#include <xen/interface/xen.h>
@@ -91,6 +92,24 @@ ENTRY(privcmd_call)
mov x2, x3
mov x3, x4
mov x4, x5
+#ifdef CONFIG_ARM64_SW_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 emulation
+ * is enabled (it implies that hardware UAO and PAN disabled).
+ */
+ uaccess_enable_not_uao x6, x7
+#endif
hvc XEN_IMM
+
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Disable userspace access from kernel once the hyp call completed.
+ */
+ uaccess_disable_not_uao x6
+#endif
ret
ENDPROC(privcmd_call);
^ permalink raw reply related [flat|nested] 64+ messages in thread
* [kernel-hardening] [PATCH v3 6/7] arm64: xen: Enable user access before a privcmd hvc call
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
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.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/xen/hypercall.S | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 329c8027b0a9..a23b2e8f2647 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -49,6 +49,7 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
+#include <asm/uaccess.h>
#include <xen/interface/xen.h>
@@ -91,6 +92,24 @@ ENTRY(privcmd_call)
mov x2, x3
mov x3, x4
mov x4, x5
+#ifdef CONFIG_ARM64_SW_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 emulation
+ * is enabled (it implies that hardware UAO and PAN disabled).
+ */
+ uaccess_enable_not_uao x6, x7
+#endif
hvc XEN_IMM
+
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Disable userspace access from kernel once the hyp call completed.
+ */
+ uaccess_disable_not_uao x6
+#endif
ret
ENDPROC(privcmd_call);
^ permalink raw reply related [flat|nested] 64+ messages in thread
* [PATCH v3 7/7] arm64: Enable CONFIG_ARM64_SW_TTBR0_PAN
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-13 17:46 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds the Kconfig option to enable support for TTBR0 PAN
emulation. 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>
Cc: Mark Rutland <mark.rutland@arm.com>
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 bc3f00f586f1..3640f9ffcc1e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -785,6 +785,14 @@ config SETEND_EMULATION
If unsure, say Y
endif
+config ARM64_SW_TTBR0_PAN
+ bool "Emulate 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] 64+ messages in thread
* [kernel-hardening] [PATCH v3 7/7] arm64: Enable CONFIG_ARM64_SW_TTBR0_PAN
@ 2016-09-13 17:46 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-13 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, James Morse, Kees Cook, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
This patch adds the Kconfig option to enable support for TTBR0 PAN
emulation. 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>
Cc: Mark Rutland <mark.rutland@arm.com>
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 bc3f00f586f1..3640f9ffcc1e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -785,6 +785,14 @@ config SETEND_EMULATION
If unsure, say Y
endif
+config ARM64_SW_TTBR0_PAN
+ bool "Emulate 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] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-14 10:13 ` Ard Biesheuvel
-1 siblings, 0 replies; 64+ messages in thread
From: Ard Biesheuvel @ 2016-09-14 10:13 UTC (permalink / raw)
To: linux-arm-kernel
On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> This is the third version of the arm64 PAN emulation using TTBR0_EL1
> switching. The series has not yet included the alternative nop patches
> from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> in a subsequent version once 4.9-rc1 is out (which will include Mark's
> alternative nop patches).
>
> Changes since v2:
>
> - efi_set_pgd() reworked to update the saved ttbr0 during run-time
> services as this value is used during exception return
>
> - uaccess_(enable|disable) C macros no longer take an 'alt' parameter
> Instead, uaccess_(enable|disable)_not_uao are introduced for the case
> where hardware PAN switching is not required when UAO is present
>
> - post_ttbr0_update_workaround macro no longer takes a 'ret' parameter
>
> - system_supports_ttbr0_pan renamed to system_uses_ttbr0_pan
>
> - init_thread_info.ttbr0 moved towards the end of the setup_arch()
> function and comment updated
>
> - vmlinux.lds.S fixed to use RESERVED_TTBR0_SIZE
>
> - Config option changed to ARM64_SW_TTBR0_PAN
>
> - Some comment clean-ups and commit log updates
>
Given that every __get_user() call now incurs the PAN switch overhead,
I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
only for compat tasks, and not nearly as often, obviously.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-14 10:13 ` Ard Biesheuvel
0 siblings, 0 replies; 64+ messages in thread
From: Ard Biesheuvel @ 2016-09-14 10:13 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, Will Deacon, James Morse, Kees Cook,
Mark Rutland, AKASHI Takahiro, kernel-hardening
On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> This is the third version of the arm64 PAN emulation using TTBR0_EL1
> switching. The series has not yet included the alternative nop patches
> from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> in a subsequent version once 4.9-rc1 is out (which will include Mark's
> alternative nop patches).
>
> Changes since v2:
>
> - efi_set_pgd() reworked to update the saved ttbr0 during run-time
> services as this value is used during exception return
>
> - uaccess_(enable|disable) C macros no longer take an 'alt' parameter
> Instead, uaccess_(enable|disable)_not_uao are introduced for the case
> where hardware PAN switching is not required when UAO is present
>
> - post_ttbr0_update_workaround macro no longer takes a 'ret' parameter
>
> - system_supports_ttbr0_pan renamed to system_uses_ttbr0_pan
>
> - init_thread_info.ttbr0 moved towards the end of the setup_arch()
> function and comment updated
>
> - vmlinux.lds.S fixed to use RESERVED_TTBR0_SIZE
>
> - Config option changed to ARM64_SW_TTBR0_PAN
>
> - Some comment clean-ups and commit log updates
>
Given that every __get_user() call now incurs the PAN switch overhead,
I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
only for compat tasks, and not nearly as often, obviously.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-14 10:13 ` [kernel-hardening] " Ard Biesheuvel
@ 2016-09-14 10:27 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 10:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
> > switching.
> Given that every __get_user() call now incurs the PAN switch overhead,
> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
> only for compat tasks, and not nearly as often, obviously.
FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
upon kernel entry to reliably detect/handle stack overflow (as we need
to free up GPR to detect overflow, and we need to detect that before we
try to store to the stack).
For non-compat tasks we must restore zero, so either way we'll end up
with a load (to determine compat-ness or to load the precise value).
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-14 10:27 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 10:27 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Catalin Marinas, linux-arm-kernel, Will Deacon, James Morse,
Kees Cook, AKASHI Takahiro, kernel-hardening
On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
> > switching.
> Given that every __get_user() call now incurs the PAN switch overhead,
> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
> only for compat tasks, and not nearly as often, obviously.
FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
upon kernel entry to reliably detect/handle stack overflow (as we need
to free up GPR to detect overflow, and we need to detect that before we
try to store to the stack).
For non-compat tasks we must restore zero, so either way we'll end up
with a load (to determine compat-ness or to load the precise value).
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-14 10:27 ` [kernel-hardening] " Mark Rutland
@ 2016-09-14 10:30 ` Ard Biesheuvel
-1 siblings, 0 replies; 64+ messages in thread
From: Ard Biesheuvel @ 2016-09-14 10:30 UTC (permalink / raw)
To: linux-arm-kernel
On 14 September 2016 at 11:27, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
>> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
>> > switching.
>
>> Given that every __get_user() call now incurs the PAN switch overhead,
>> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
>> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
>> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
>> only for compat tasks, and not nearly as often, obviously.
>
> FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
> upon kernel entry to reliably detect/handle stack overflow (as we need
> to free up GPR to detect overflow, and we need to detect that before we
> try to store to the stack).
>
> For non-compat tasks we must restore zero, so either way we'll end up
> with a load (to determine compat-ness or to load the precise value).
>
Are you saying that with vmapped stacks, we'll end up clobbering it
(and thus restoring it) anyway when entering the kernel, and so we
could use it for free afterwards while running in the kernel,
potentially for the real value of TTBR0_EL1?
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-14 10:30 ` Ard Biesheuvel
0 siblings, 0 replies; 64+ messages in thread
From: Ard Biesheuvel @ 2016-09-14 10:30 UTC (permalink / raw)
To: Mark Rutland
Cc: Catalin Marinas, linux-arm-kernel, Will Deacon, James Morse,
Kees Cook, AKASHI Takahiro, kernel-hardening
On 14 September 2016 at 11:27, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
>> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
>> > switching.
>
>> Given that every __get_user() call now incurs the PAN switch overhead,
>> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
>> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
>> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
>> only for compat tasks, and not nearly as often, obviously.
>
> FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
> upon kernel entry to reliably detect/handle stack overflow (as we need
> to free up GPR to detect overflow, and we need to detect that before we
> try to store to the stack).
>
> For non-compat tasks we must restore zero, so either way we'll end up
> with a load (to determine compat-ness or to load the precise value).
>
Are you saying that with vmapped stacks, we'll end up clobbering it
(and thus restoring it) anyway when entering the kernel, and so we
could use it for free afterwards while running in the kernel,
potentially for the real value of TTBR0_EL1?
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-14 10:30 ` [kernel-hardening] " Ard Biesheuvel
@ 2016-09-14 10:36 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 10:36 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 14, 2016 at 11:30:05AM +0100, Ard Biesheuvel wrote:
> On 14 September 2016 at 11:27, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
> >> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
> >> > switching.
> >
> >> Given that every __get_user() call now incurs the PAN switch overhead,
> >> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
> >> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
> >> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
> >> only for compat tasks, and not nearly as often, obviously.
> >
> > FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
> > upon kernel entry to reliably detect/handle stack overflow (as we need
> > to free up GPR to detect overflow, and we need to detect that before we
> > try to store to the stack).
> >
> > For non-compat tasks we must restore zero, so either way we'll end up
> > with a load (to determine compat-ness or to load the precise value).
>
> Are you saying that with vmapped stacks, we'll end up clobbering it
> (and thus restoring it) anyway when entering the kernel, and so we
> could use it for free afterwards while running in the kernel,
> potentially for the real value of TTBR0_EL1?
Yes, assuming that we end up following my current plan for how to
implement that.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-14 10:36 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 10:36 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Catalin Marinas, linux-arm-kernel, Will Deacon, James Morse,
Kees Cook, AKASHI Takahiro, kernel-hardening
On Wed, Sep 14, 2016 at 11:30:05AM +0100, Ard Biesheuvel wrote:
> On 14 September 2016 at 11:27, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
> >> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
> >> > switching.
> >
> >> Given that every __get_user() call now incurs the PAN switch overhead,
> >> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
> >> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
> >> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
> >> only for compat tasks, and not nearly as often, obviously.
> >
> > FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
> > upon kernel entry to reliably detect/handle stack overflow (as we need
> > to free up GPR to detect overflow, and we need to detect that before we
> > try to store to the stack).
> >
> > For non-compat tasks we must restore zero, so either way we'll end up
> > with a load (to determine compat-ness or to load the precise value).
>
> Are you saying that with vmapped stacks, we'll end up clobbering it
> (and thus restoring it) anyway when entering the kernel, and so we
> could use it for free afterwards while running in the kernel,
> potentially for the real value of TTBR0_EL1?
Yes, assuming that we end up following my current plan for how to
implement that.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-14 10:36 ` [kernel-hardening] " Mark Rutland
@ 2016-09-14 10:48 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 10:48 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 14, 2016 at 11:36:46AM +0100, Mark Rutland wrote:
> On Wed, Sep 14, 2016 at 11:30:05AM +0100, Ard Biesheuvel wrote:
> > On 14 September 2016 at 11:27, Mark Rutland <mark.rutland@arm.com> wrote:
> > > On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
> > >> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
> > >> > switching.
> > >
> > >> Given that every __get_user() call now incurs the PAN switch overhead,
> > >> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
> > >> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
> > >> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
> > >> only for compat tasks, and not nearly as often, obviously.
> > >
> > > FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
> > > upon kernel entry to reliably detect/handle stack overflow (as we need
> > > to free up GPR to detect overflow, and we need to detect that before we
> > > try to store to the stack).
> > >
> > > For non-compat tasks we must restore zero, so either way we'll end up
> > > with a load (to determine compat-ness or to load the precise value).
> >
> > Are you saying that with vmapped stacks, we'll end up clobbering it
> > (and thus restoring it) anyway when entering the kernel, and so we
> > could use it for free afterwards while running in the kernel,
> > potentially for the real value of TTBR0_EL1?
>
> Yes, assuming that we end up following my current plan for how to
> implement that.
Actually, after thinknig for more than a picosecond, no we can't.
We need to be able to clbober it on EL1 -> EL1 exceptions, to catch
kernel stack overflow.
So if anything, the two approaches are mutually exclusive, unless we
restore the stashed TTBR0 value back into TPIDRRO_EL0 before returning
from EL1 -> EL1.
/me fetches some more coffee.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-14 10:48 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-14 10:48 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Kees Cook, kernel-hardening, Catalin Marinas, Will Deacon,
AKASHI Takahiro, James Morse, linux-arm-kernel
On Wed, Sep 14, 2016 at 11:36:46AM +0100, Mark Rutland wrote:
> On Wed, Sep 14, 2016 at 11:30:05AM +0100, Ard Biesheuvel wrote:
> > On 14 September 2016 at 11:27, Mark Rutland <mark.rutland@arm.com> wrote:
> > > On Wed, Sep 14, 2016 at 11:13:33AM +0100, Ard Biesheuvel wrote:
> > >> On 13 September 2016 at 18:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >> > This is the third version of the arm64 PAN emulation using TTBR0_EL1
> > >> > switching.
> > >
> > >> Given that every __get_user() call now incurs the PAN switch overhead,
> > >> I wonder if it would be worth it to stash the real TTBR0_EL1 value in,
> > >> e.g., TPIDRRO_EL0 rather than load it from memory each time. We'd have
> > >> to reload the real value of TPIDRRO_EL0 at kernel exit every time, but
> > >> only for compat tasks, and not nearly as often, obviously.
> > >
> > > FWIW, my plan for vmap'd stacks involves clobbering TPIDRRO_EL0 early
> > > upon kernel entry to reliably detect/handle stack overflow (as we need
> > > to free up GPR to detect overflow, and we need to detect that before we
> > > try to store to the stack).
> > >
> > > For non-compat tasks we must restore zero, so either way we'll end up
> > > with a load (to determine compat-ness or to load the precise value).
> >
> > Are you saying that with vmapped stacks, we'll end up clobbering it
> > (and thus restoring it) anyway when entering the kernel, and so we
> > could use it for free afterwards while running in the kernel,
> > potentially for the real value of TTBR0_EL1?
>
> Yes, assuming that we end up following my current plan for how to
> implement that.
Actually, after thinknig for more than a picosecond, no we can't.
We need to be able to clbober it on EL1 -> EL1 exceptions, to catch
kernel stack overflow.
So if anything, the two approaches are mutually exclusive, unless we
restore the stashed TTBR0 value back into TPIDRRO_EL0 before returning
from EL1 -> EL1.
/me fetches some more coffee.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-14 20:54 ` David Brown
-1 siblings, 0 replies; 64+ messages in thread
From: David Brown @ 2016-09-14 20:54 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 13, 2016 at 06:46:30PM +0100, Catalin Marinas wrote:
>This is the third version of the arm64 PAN emulation using TTBR0_EL1
>switching. The series has not yet included the alternative nop patches
>from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
>in a subsequent version once 4.9-rc1 is out (which will include Mark's
>alternative nop patches).
I've tested these with a bunch of reboots in both qemu and HiKey and
have not seen the efi failure.
David
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [kernel-hardening] [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-14 20:54 ` David Brown
0 siblings, 0 replies; 64+ messages in thread
From: David Brown @ 2016-09-14 20:54 UTC (permalink / raw)
To: kernel-hardening
Cc: linux-arm-kernel, Will Deacon, James Morse, Kees Cook,
Mark Rutland, Ard Biesheuvel, AKASHI Takahiro
On Tue, Sep 13, 2016 at 06:46:30PM +0100, Catalin Marinas wrote:
>This is the third version of the arm64 PAN emulation using TTBR0_EL1
>switching. The series has not yet included the alternative nop patches
>from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
>in a subsequent version once 4.9-rc1 is out (which will include Mark's
>alternative nop patches).
I've tested these with a bunch of reboots in both qemu and HiKey and
have not seen the efi failure.
David
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-14 20:54 ` David Brown
@ 2016-09-15 9:52 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-15 9:52 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 14, 2016 at 02:54:25PM -0600, David Brown wrote:
> On Tue, Sep 13, 2016 at 06:46:30PM +0100, Catalin Marinas wrote:
>
> >This is the third version of the arm64 PAN emulation using TTBR0_EL1
> >switching. The series has not yet included the alternative nop patches
> >from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> >in a subsequent version once 4.9-rc1 is out (which will include Mark's
> >alternative nop patches).
>
> I've tested these with a bunch of reboots in both qemu and HiKey and
> have not seen the efi failure.
Thanks for testing. I'll defer adding your tested-by until I post the
hopefully final version after 4.9-rc1 (it's pretty late to merge these
patches now, I still need to do more testing on models with PAN or
PAN+UAO present).
--
Catalin
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [kernel-hardening] [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-15 9:52 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-09-15 9:52 UTC (permalink / raw)
To: David Brown
Cc: kernel-hardening, Mark Rutland, Kees Cook, Ard Biesheuvel,
Will Deacon, AKASHI Takahiro, James Morse, linux-arm-kernel
On Wed, Sep 14, 2016 at 02:54:25PM -0600, David Brown wrote:
> On Tue, Sep 13, 2016 at 06:46:30PM +0100, Catalin Marinas wrote:
>
> >This is the third version of the arm64 PAN emulation using TTBR0_EL1
> >switching. The series has not yet included the alternative nop patches
> >from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> >in a subsequent version once 4.9-rc1 is out (which will include Mark's
> >alternative nop patches).
>
> I've tested these with a bunch of reboots in both qemu and HiKey and
> have not seen the efi failure.
Thanks for testing. I'll defer adding your tested-by until I post the
hopefully final version after 4.9-rc1 (it's pretty late to merge these
patches now, I still need to do more testing on models with PAN or
PAN+UAO present).
--
Catalin
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-09-15 16:20 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 16:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Catalin,
On Tue, Sep 13, 2016 at 06:46:30PM +0100, Catalin Marinas wrote:
> This is the third version of the arm64 PAN emulation using TTBR0_EL1
> switching. The series has not yet included the alternative nop patches
> from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> in a subsequent version once 4.9-rc1 is out (which will include Mark's
> alternative nop patches).
> arch/arm64/Kconfig | 8 ++
> arch/arm64/include/asm/assembler.h | 33 +++++++
> arch/arm64/include/asm/cpufeature.h | 6 ++
> arch/arm64/include/asm/efi.h | 26 ++++-
> arch/arm64/include/asm/futex.h | 14 +--
> arch/arm64/include/asm/kernel-pgtable.h | 7 ++
> arch/arm64/include/asm/mmu_context.h | 51 +++++++---
> arch/arm64/include/asm/ptrace.h | 2 +
> arch/arm64/include/asm/thread_info.h | 3 +
> arch/arm64/include/asm/uaccess.h | 163 ++++++++++++++++++++++++++++++--
> arch/arm64/kernel/armv8_deprecated.c | 10 +-
> arch/arm64/kernel/asm-offsets.c | 3 +
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/entry.S | 71 +++++++++++++-
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/setup.c | 9 ++
> arch/arm64/kernel/vmlinux.lds.S | 5 +
> arch/arm64/lib/clear_user.S | 8 +-
> arch/arm64/lib/copy_from_user.S | 8 +-
> arch/arm64/lib/copy_in_user.S | 8 +-
> arch/arm64/lib/copy_to_user.S | 8 +-
> arch/arm64/mm/context.c | 7 +-
> arch/arm64/mm/fault.c | 22 +++--
> arch/arm64/mm/proc.S | 11 +--
> arch/arm64/xen/hypercall.S | 19 ++++
> 25 files changed, 428 insertions(+), 81 deletions(-)
I don't seee traps.c there. Don't we need to fix up do_sysinstr() (which
performs raw cache maintenance ops on __user addresses) with
uaccess_{enable,disable}_not_uao()?
Likewise, how do we handle __flush_cache_user_range and
flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
__user addresses.
I hope we have some tests lying around for those. ;)
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-15 16:20 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 16:20 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, Kees Cook, Ard Biesheuvel, kernel-hardening,
Will Deacon, AKASHI Takahiro, James Morse, andre.przywara,
suzuki.poulose
Hi Catalin,
On Tue, Sep 13, 2016 at 06:46:30PM +0100, Catalin Marinas wrote:
> This is the third version of the arm64 PAN emulation using TTBR0_EL1
> switching. The series has not yet included the alternative nop patches
> from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> in a subsequent version once 4.9-rc1 is out (which will include Mark's
> alternative nop patches).
> arch/arm64/Kconfig | 8 ++
> arch/arm64/include/asm/assembler.h | 33 +++++++
> arch/arm64/include/asm/cpufeature.h | 6 ++
> arch/arm64/include/asm/efi.h | 26 ++++-
> arch/arm64/include/asm/futex.h | 14 +--
> arch/arm64/include/asm/kernel-pgtable.h | 7 ++
> arch/arm64/include/asm/mmu_context.h | 51 +++++++---
> arch/arm64/include/asm/ptrace.h | 2 +
> arch/arm64/include/asm/thread_info.h | 3 +
> arch/arm64/include/asm/uaccess.h | 163 ++++++++++++++++++++++++++++++--
> arch/arm64/kernel/armv8_deprecated.c | 10 +-
> arch/arm64/kernel/asm-offsets.c | 3 +
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/entry.S | 71 +++++++++++++-
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/setup.c | 9 ++
> arch/arm64/kernel/vmlinux.lds.S | 5 +
> arch/arm64/lib/clear_user.S | 8 +-
> arch/arm64/lib/copy_from_user.S | 8 +-
> arch/arm64/lib/copy_in_user.S | 8 +-
> arch/arm64/lib/copy_to_user.S | 8 +-
> arch/arm64/mm/context.c | 7 +-
> arch/arm64/mm/fault.c | 22 +++--
> arch/arm64/mm/proc.S | 11 +--
> arch/arm64/xen/hypercall.S | 19 ++++
> 25 files changed, 428 insertions(+), 81 deletions(-)
I don't seee traps.c there. Don't we need to fix up do_sysinstr() (which
performs raw cache maintenance ops on __user addresses) with
uaccess_{enable,disable}_not_uao()?
Likewise, how do we handle __flush_cache_user_range and
flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
__user addresses.
I hope we have some tests lying around for those. ;)
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-15 16:20 ` [kernel-hardening] " Mark Rutland
@ 2016-09-15 16:41 ` Mark Rutland
-1 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 16:41 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
> Don't we need to fix up do_sysinstr() (which performs raw cache
> maintenance ops on __user addresses) with
> uaccess_{enable,disable}_not_uao()?
Actually, we don't even need to do anything in the case of PAN since
cache ops aren't affected by PAN/UAO; we just need to map/unmap in the
TTBR0-switching case to ensure we don't get stup fixing things up
forever.
/me commences name bikeshedding.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-15 16:41 ` Mark Rutland
0 siblings, 0 replies; 64+ messages in thread
From: Mark Rutland @ 2016-09-15 16:41 UTC (permalink / raw)
To: Catalin Marinas
Cc: Kees Cook, kernel-hardening, andre.przywara, Ard Biesheuvel,
Will Deacon, AKASHI Takahiro, James Morse, suzuki.poulose,
linux-arm-kernel
On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
> Don't we need to fix up do_sysinstr() (which performs raw cache
> maintenance ops on __user addresses) with
> uaccess_{enable,disable}_not_uao()?
Actually, we don't even need to do anything in the case of PAN since
cache ops aren't affected by PAN/UAO; we just need to map/unmap in the
TTBR0-switching case to ensure we don't get stup fixing things up
forever.
/me commences name bikeshedding.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-15 16:20 ` [kernel-hardening] " Mark Rutland
@ 2016-09-29 22:44 ` Sami Tolvanen
-1 siblings, 0 replies; 64+ messages in thread
From: Sami Tolvanen @ 2016-09-29 22:44 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
> Likewise, how do we handle __flush_cache_user_range and
> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
> __user addresses.
Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
and causes the process to hang when I tested these patches on HiKey.
Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
fix the problem.
Sami
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-29 22:44 ` Sami Tolvanen
0 siblings, 0 replies; 64+ messages in thread
From: Sami Tolvanen @ 2016-09-29 22:44 UTC (permalink / raw)
To: kernel-hardening
Cc: Catalin Marinas, linux-arm-kernel, Kees Cook, Ard Biesheuvel,
Will Deacon, AKASHI Takahiro, James Morse, andre.przywara,
suzuki.poulose
On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
> Likewise, how do we handle __flush_cache_user_range and
> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
> __user addresses.
Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
and causes the process to hang when I tested these patches on HiKey.
Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
fix the problem.
Sami
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-29 22:44 ` Sami Tolvanen
@ 2016-09-30 18:42 ` Kees Cook
-1 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-09-30 18:42 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 29, 2016 at 3:44 PM, Sami Tolvanen <samitolvanen@google.com> wrote:
> On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
>> Likewise, how do we handle __flush_cache_user_range and
>> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
>> __user addresses.
>
> Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
> and causes the process to hang when I tested these patches on HiKey.
>
> Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
> fix the problem.
I had a thought just now on this: is lkdtm maybe doing the wrong thing
here? i.e. should lkdtm be the one do to the uaccess_en/disable
instead of flush_icache_range() itself, since it's the one abusing the
API?
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-09-30 18:42 ` Kees Cook
0 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-09-30 18:42 UTC (permalink / raw)
To: Sami Tolvanen
Cc: kernel-hardening, Catalin Marinas, linux-arm-kernel,
Ard Biesheuvel, Will Deacon, AKASHI Takahiro, James Morse,
andre.przywara, Suzuki K. Poulose
On Thu, Sep 29, 2016 at 3:44 PM, Sami Tolvanen <samitolvanen@google.com> wrote:
> On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
>> Likewise, how do we handle __flush_cache_user_range and
>> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
>> __user addresses.
>
> Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
> and causes the process to hang when I tested these patches on HiKey.
>
> Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
> fix the problem.
I had a thought just now on this: is lkdtm maybe doing the wrong thing
here? i.e. should lkdtm be the one do to the uaccess_en/disable
instead of flush_icache_range() itself, since it's the one abusing the
API?
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-30 18:42 ` Kees Cook
@ 2016-10-27 14:54 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-10-27 14:54 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 30, 2016 at 11:42:02AM -0700, Kees Cook wrote:
> On Thu, Sep 29, 2016 at 3:44 PM, Sami Tolvanen <samitolvanen@google.com> wrote:
> > On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
> >> Likewise, how do we handle __flush_cache_user_range and
> >> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
> >> __user addresses.
> >
> > Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
> > and causes the process to hang when I tested these patches on HiKey.
> >
> > Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
> > fix the problem.
>
> I had a thought just now on this: is lkdtm maybe doing the wrong thing
> here? i.e. should lkdtm be the one do to the uaccess_en/disable
> instead of flush_icache_range() itself, since it's the one abusing the
> API?
(preparing the v4 series)
I think lkdtm is using the API incorrectly here. The documentation for
flush_icache_range() (Documentation/cachetlb.txt) states that it is to
be used on kernel addresses. Even with uaccess_enable/disable in lkdtm,
faults on user space can still happen and the flush_icache_range()
function must be able to handle them. It happens to work on arm64
because of the fall through __flush_cache_user_range() but that's not
guaranteed on other architectures.
A potential solution is to use access_process_vm() and let the arch code
handle the cache maintenance automatically:
---------------------8<--------------------------------
>From fcbb7c9c30daf9bfc2a215ec10dba79c109ab835 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Thu, 27 Oct 2016 15:47:20 +0100
Subject: [PATCH] lkdtm: Do not use flush_icache_range() on user addresses
The flush_icache_range() API is meant to be used on kernel addresses
only as it may not have the infrastructure (exception entries) to handle
user memory faults.
The lkdtm execute_user_location() function tests the kernel execution of
user space addresses by mmap'ing an anonymous page, copying some code
together with cache maintenance and attempting to run it. However, the
cache maintenance step may fail because of the incorrect API usage
described above. The patch changes lkdtm to use access_process_vm() for
copying the code into user space which would take care of the necessary
cache maintenance.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
drivers/misc/lkdtm_perms.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
index 45f1c0f96612..c7635a79341f 100644
--- a/drivers/misc/lkdtm_perms.c
+++ b/drivers/misc/lkdtm_perms.c
@@ -60,15 +60,18 @@ static noinline void execute_location(void *dst, bool write)
static void execute_user_location(void *dst)
{
+ int copied;
+
/* Intentionally crossing kernel/user memory boundary. */
void (*func)(void) = dst;
pr_info("attempting ok execution at %p\n", do_nothing);
do_nothing();
- if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE))
+ copied = access_process_vm(current, (unsigned long)dst, do_nothing,
+ EXEC_SIZE, FOLL_WRITE);
+ if (copied < EXEC_SIZE)
return;
- flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE);
pr_info("attempting bad execution@%p\n", func);
func();
}
^ permalink raw reply related [flat|nested] 64+ messages in thread* Re: [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-10-27 14:54 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-10-27 14:54 UTC (permalink / raw)
To: Kees Cook
Cc: Sami Tolvanen, Ard Biesheuvel, kernel-hardening, Will Deacon,
AKASHI Takahiro, James Morse, andre.przywara, Suzuki K. Poulose,
linux-arm-kernel
On Fri, Sep 30, 2016 at 11:42:02AM -0700, Kees Cook wrote:
> On Thu, Sep 29, 2016 at 3:44 PM, Sami Tolvanen <samitolvanen@google.com> wrote:
> > On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
> >> Likewise, how do we handle __flush_cache_user_range and
> >> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
> >> __user addresses.
> >
> > Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
> > and causes the process to hang when I tested these patches on HiKey.
> >
> > Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
> > fix the problem.
>
> I had a thought just now on this: is lkdtm maybe doing the wrong thing
> here? i.e. should lkdtm be the one do to the uaccess_en/disable
> instead of flush_icache_range() itself, since it's the one abusing the
> API?
(preparing the v4 series)
I think lkdtm is using the API incorrectly here. The documentation for
flush_icache_range() (Documentation/cachetlb.txt) states that it is to
be used on kernel addresses. Even with uaccess_enable/disable in lkdtm,
faults on user space can still happen and the flush_icache_range()
function must be able to handle them. It happens to work on arm64
because of the fall through __flush_cache_user_range() but that's not
guaranteed on other architectures.
A potential solution is to use access_process_vm() and let the arch code
handle the cache maintenance automatically:
---------------------8<--------------------------------
>From fcbb7c9c30daf9bfc2a215ec10dba79c109ab835 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Thu, 27 Oct 2016 15:47:20 +0100
Subject: [PATCH] lkdtm: Do not use flush_icache_range() on user addresses
The flush_icache_range() API is meant to be used on kernel addresses
only as it may not have the infrastructure (exception entries) to handle
user memory faults.
The lkdtm execute_user_location() function tests the kernel execution of
user space addresses by mmap'ing an anonymous page, copying some code
together with cache maintenance and attempting to run it. However, the
cache maintenance step may fail because of the incorrect API usage
described above. The patch changes lkdtm to use access_process_vm() for
copying the code into user space which would take care of the necessary
cache maintenance.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
drivers/misc/lkdtm_perms.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
index 45f1c0f96612..c7635a79341f 100644
--- a/drivers/misc/lkdtm_perms.c
+++ b/drivers/misc/lkdtm_perms.c
@@ -60,15 +60,18 @@ static noinline void execute_location(void *dst, bool write)
static void execute_user_location(void *dst)
{
+ int copied;
+
/* Intentionally crossing kernel/user memory boundary. */
void (*func)(void) = dst;
pr_info("attempting ok execution at %p\n", do_nothing);
do_nothing();
- if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE))
+ copied = access_process_vm(current, (unsigned long)dst, do_nothing,
+ EXEC_SIZE, FOLL_WRITE);
+ if (copied < EXEC_SIZE)
return;
- flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE);
pr_info("attempting bad execution at %p\n", func);
func();
}
^ permalink raw reply related [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-10-27 14:54 ` Catalin Marinas
@ 2016-10-27 21:23 ` Kees Cook
-1 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-10-27 21:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 27, 2016 at 7:54 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Fri, Sep 30, 2016 at 11:42:02AM -0700, Kees Cook wrote:
>> On Thu, Sep 29, 2016 at 3:44 PM, Sami Tolvanen <samitolvanen@google.com> wrote:
>> > On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
>> >> Likewise, how do we handle __flush_cache_user_range and
>> >> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
>> >> __user addresses.
>> >
>> > Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
>> > and causes the process to hang when I tested these patches on HiKey.
>> >
>> > Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
>> > fix the problem.
>>
>> I had a thought just now on this: is lkdtm maybe doing the wrong thing
>> here? i.e. should lkdtm be the one do to the uaccess_en/disable
>> instead of flush_icache_range() itself, since it's the one abusing the
>> API?
>
> (preparing the v4 series)
>
> I think lkdtm is using the API incorrectly here. The documentation for
> flush_icache_range() (Documentation/cachetlb.txt) states that it is to
> be used on kernel addresses. Even with uaccess_enable/disable in lkdtm,
> faults on user space can still happen and the flush_icache_range()
> function must be able to handle them. It happens to work on arm64
> because of the fall through __flush_cache_user_range() but that's not
> guaranteed on other architectures.
>
> A potential solution is to use access_process_vm() and let the arch code
> handle the cache maintenance automatically:
Ah, perfect! I'll give this a spin, thanks!
-Kees
> ---------------------8<--------------------------------
> From fcbb7c9c30daf9bfc2a215ec10dba79c109ab835 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Thu, 27 Oct 2016 15:47:20 +0100
> Subject: [PATCH] lkdtm: Do not use flush_icache_range() on user addresses
>
> The flush_icache_range() API is meant to be used on kernel addresses
> only as it may not have the infrastructure (exception entries) to handle
> user memory faults.
>
> The lkdtm execute_user_location() function tests the kernel execution of
> user space addresses by mmap'ing an anonymous page, copying some code
> together with cache maintenance and attempting to run it. However, the
> cache maintenance step may fail because of the incorrect API usage
> described above. The patch changes lkdtm to use access_process_vm() for
> copying the code into user space which would take care of the necessary
> cache maintenance.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> drivers/misc/lkdtm_perms.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
> index 45f1c0f96612..c7635a79341f 100644
> --- a/drivers/misc/lkdtm_perms.c
> +++ b/drivers/misc/lkdtm_perms.c
> @@ -60,15 +60,18 @@ static noinline void execute_location(void *dst, bool write)
>
> static void execute_user_location(void *dst)
> {
> + int copied;
> +
> /* Intentionally crossing kernel/user memory boundary. */
> void (*func)(void) = dst;
>
> pr_info("attempting ok execution at %p\n", do_nothing);
> do_nothing();
>
> - if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE))
> + copied = access_process_vm(current, (unsigned long)dst, do_nothing,
> + EXEC_SIZE, FOLL_WRITE);
> + if (copied < EXEC_SIZE)
> return;
> - flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE);
> pr_info("attempting bad execution at %p\n", func);
> func();
> }
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-10-27 21:23 ` Kees Cook
0 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-10-27 21:23 UTC (permalink / raw)
To: Catalin Marinas
Cc: Sami Tolvanen, Ard Biesheuvel, kernel-hardening, Will Deacon,
AKASHI Takahiro, James Morse, andre.przywara, Suzuki K. Poulose,
linux-arm-kernel
On Thu, Oct 27, 2016 at 7:54 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Fri, Sep 30, 2016 at 11:42:02AM -0700, Kees Cook wrote:
>> On Thu, Sep 29, 2016 at 3:44 PM, Sami Tolvanen <samitolvanen@google.com> wrote:
>> > On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote:
>> >> Likewise, how do we handle __flush_cache_user_range and
>> >> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in
>> >> __user addresses.
>> >
>> > Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range
>> > and causes the process to hang when I tested these patches on HiKey.
>> >
>> > Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to
>> > fix the problem.
>>
>> I had a thought just now on this: is lkdtm maybe doing the wrong thing
>> here? i.e. should lkdtm be the one do to the uaccess_en/disable
>> instead of flush_icache_range() itself, since it's the one abusing the
>> API?
>
> (preparing the v4 series)
>
> I think lkdtm is using the API incorrectly here. The documentation for
> flush_icache_range() (Documentation/cachetlb.txt) states that it is to
> be used on kernel addresses. Even with uaccess_enable/disable in lkdtm,
> faults on user space can still happen and the flush_icache_range()
> function must be able to handle them. It happens to work on arm64
> because of the fall through __flush_cache_user_range() but that's not
> guaranteed on other architectures.
>
> A potential solution is to use access_process_vm() and let the arch code
> handle the cache maintenance automatically:
Ah, perfect! I'll give this a spin, thanks!
-Kees
> ---------------------8<--------------------------------
> From fcbb7c9c30daf9bfc2a215ec10dba79c109ab835 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Thu, 27 Oct 2016 15:47:20 +0100
> Subject: [PATCH] lkdtm: Do not use flush_icache_range() on user addresses
>
> The flush_icache_range() API is meant to be used on kernel addresses
> only as it may not have the infrastructure (exception entries) to handle
> user memory faults.
>
> The lkdtm execute_user_location() function tests the kernel execution of
> user space addresses by mmap'ing an anonymous page, copying some code
> together with cache maintenance and attempting to run it. However, the
> cache maintenance step may fail because of the incorrect API usage
> described above. The patch changes lkdtm to use access_process_vm() for
> copying the code into user space which would take care of the necessary
> cache maintenance.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> drivers/misc/lkdtm_perms.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
> index 45f1c0f96612..c7635a79341f 100644
> --- a/drivers/misc/lkdtm_perms.c
> +++ b/drivers/misc/lkdtm_perms.c
> @@ -60,15 +60,18 @@ static noinline void execute_location(void *dst, bool write)
>
> static void execute_user_location(void *dst)
> {
> + int copied;
> +
> /* Intentionally crossing kernel/user memory boundary. */
> void (*func)(void) = dst;
>
> pr_info("attempting ok execution at %p\n", do_nothing);
> do_nothing();
>
> - if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE))
> + copied = access_process_vm(current, (unsigned long)dst, do_nothing,
> + EXEC_SIZE, FOLL_WRITE);
> + if (copied < EXEC_SIZE)
> return;
> - flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE);
> pr_info("attempting bad execution at %p\n", func);
> func();
> }
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-09-13 17:46 ` [kernel-hardening] " Catalin Marinas
@ 2016-10-14 21:44 ` Kees Cook
-1 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-10-14 21:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Just checking in on this feature -- I don't see it in -next nor
already in the tree. Is there any chance this is going to make the
v4.9 merge window?
It didn't sound like there were any unresolved issues remaining?
Thanks!
-Kees
On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> This is the third version of the arm64 PAN emulation using TTBR0_EL1
> switching. The series has not yet included the alternative nop patches
> from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> in a subsequent version once 4.9-rc1 is out (which will include Mark's
> alternative nop patches).
>
> Changes since v2:
>
> - efi_set_pgd() reworked to update the saved ttbr0 during run-time
> services as this value is used during exception return
>
> - uaccess_(enable|disable) C macros no longer take an 'alt' parameter
> Instead, uaccess_(enable|disable)_not_uao are introduced for the case
> where hardware PAN switching is not required when UAO is present
>
> - post_ttbr0_update_workaround macro no longer takes a 'ret' parameter
>
> - system_supports_ttbr0_pan renamed to system_uses_ttbr0_pan
>
> - init_thread_info.ttbr0 moved towards the end of the setup_arch()
> function and comment updated
>
> - vmlinux.lds.S fixed to use RESERVED_TTBR0_SIZE
>
> - Config option changed to ARM64_SW_TTBR0_PAN
>
> - Some comment clean-ups and commit log updates
>
> Catalin Marinas (7):
> arm64: Factor out PAN enabling/disabling into separate uaccess_*
> macros
> arm64: Factor out TTBR0_EL1 post-update workaround 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_SW_TTBR0_PAN
>
> 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.
>
> arch/arm64/Kconfig | 8 ++
> arch/arm64/include/asm/assembler.h | 33 +++++++
> arch/arm64/include/asm/cpufeature.h | 6 ++
> arch/arm64/include/asm/efi.h | 26 ++++-
> arch/arm64/include/asm/futex.h | 14 +--
> arch/arm64/include/asm/kernel-pgtable.h | 7 ++
> arch/arm64/include/asm/mmu_context.h | 51 +++++++---
> arch/arm64/include/asm/ptrace.h | 2 +
> arch/arm64/include/asm/thread_info.h | 3 +
> arch/arm64/include/asm/uaccess.h | 163 ++++++++++++++++++++++++++++++--
> arch/arm64/kernel/armv8_deprecated.c | 10 +-
> arch/arm64/kernel/asm-offsets.c | 3 +
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/entry.S | 71 +++++++++++++-
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/setup.c | 9 ++
> arch/arm64/kernel/vmlinux.lds.S | 5 +
> arch/arm64/lib/clear_user.S | 8 +-
> arch/arm64/lib/copy_from_user.S | 8 +-
> arch/arm64/lib/copy_in_user.S | 8 +-
> arch/arm64/lib/copy_to_user.S | 8 +-
> arch/arm64/mm/context.c | 7 +-
> arch/arm64/mm/fault.c | 22 +++--
> arch/arm64/mm/proc.S | 11 +--
> arch/arm64/xen/hypercall.S | 19 ++++
> 25 files changed, 428 insertions(+), 81 deletions(-)
>
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-10-14 21:44 ` Kees Cook
0 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-10-14 21:44 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arm-kernel, Will Deacon, James Morse, Mark Rutland,
Ard Biesheuvel, AKASHI Takahiro, kernel-hardening
Hi,
Just checking in on this feature -- I don't see it in -next nor
already in the tree. Is there any chance this is going to make the
v4.9 merge window?
It didn't sound like there were any unresolved issues remaining?
Thanks!
-Kees
On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> This is the third version of the arm64 PAN emulation using TTBR0_EL1
> switching. The series has not yet included the alternative nop patches
> from Mark Rutland, nor the empty_zero_page from Ard B. This will be done
> in a subsequent version once 4.9-rc1 is out (which will include Mark's
> alternative nop patches).
>
> Changes since v2:
>
> - efi_set_pgd() reworked to update the saved ttbr0 during run-time
> services as this value is used during exception return
>
> - uaccess_(enable|disable) C macros no longer take an 'alt' parameter
> Instead, uaccess_(enable|disable)_not_uao are introduced for the case
> where hardware PAN switching is not required when UAO is present
>
> - post_ttbr0_update_workaround macro no longer takes a 'ret' parameter
>
> - system_supports_ttbr0_pan renamed to system_uses_ttbr0_pan
>
> - init_thread_info.ttbr0 moved towards the end of the setup_arch()
> function and comment updated
>
> - vmlinux.lds.S fixed to use RESERVED_TTBR0_SIZE
>
> - Config option changed to ARM64_SW_TTBR0_PAN
>
> - Some comment clean-ups and commit log updates
>
> Catalin Marinas (7):
> arm64: Factor out PAN enabling/disabling into separate uaccess_*
> macros
> arm64: Factor out TTBR0_EL1 post-update workaround 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_SW_TTBR0_PAN
>
> 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.
>
> arch/arm64/Kconfig | 8 ++
> arch/arm64/include/asm/assembler.h | 33 +++++++
> arch/arm64/include/asm/cpufeature.h | 6 ++
> arch/arm64/include/asm/efi.h | 26 ++++-
> arch/arm64/include/asm/futex.h | 14 +--
> arch/arm64/include/asm/kernel-pgtable.h | 7 ++
> arch/arm64/include/asm/mmu_context.h | 51 +++++++---
> arch/arm64/include/asm/ptrace.h | 2 +
> arch/arm64/include/asm/thread_info.h | 3 +
> arch/arm64/include/asm/uaccess.h | 163 ++++++++++++++++++++++++++++++--
> arch/arm64/kernel/armv8_deprecated.c | 10 +-
> arch/arm64/kernel/asm-offsets.c | 3 +
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/entry.S | 71 +++++++++++++-
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/setup.c | 9 ++
> arch/arm64/kernel/vmlinux.lds.S | 5 +
> arch/arm64/lib/clear_user.S | 8 +-
> arch/arm64/lib/copy_from_user.S | 8 +-
> arch/arm64/lib/copy_in_user.S | 8 +-
> arch/arm64/lib/copy_to_user.S | 8 +-
> arch/arm64/mm/context.c | 7 +-
> arch/arm64/mm/fault.c | 22 +++--
> arch/arm64/mm/proc.S | 11 +--
> arch/arm64/xen/hypercall.S | 19 ++++
> 25 files changed, 428 insertions(+), 81 deletions(-)
>
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-10-14 21:44 ` [kernel-hardening] " Kees Cook
@ 2016-10-15 14:35 ` Catalin Marinas
-1 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-10-15 14:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi Kees,
On Fri, Oct 14, 2016 at 02:44:33PM -0700, Kees Cook wrote:
> Just checking in on this feature -- I don't see it in -next nor
> already in the tree. Is there any chance this is going to make the
> v4.9 merge window?
As I said in the cover letter, I'll rebase it on top of 4.9-rc1 as there
are some clean-ups that this series would conflict with. So I am
targeting 4.10 with this patch series.
> It didn't sound like there were any unresolved issues remaining?
There are a few issues spotted by Mark which I'll address in the next
version, but nothing major.
--
Catalin
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-10-15 14:35 ` Catalin Marinas
0 siblings, 0 replies; 64+ messages in thread
From: Catalin Marinas @ 2016-10-15 14:35 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, Ard Biesheuvel, kernel-hardening, Will Deacon,
AKASHI Takahiro, James Morse, linux-arm-kernel
Hi Kees,
On Fri, Oct 14, 2016 at 02:44:33PM -0700, Kees Cook wrote:
> Just checking in on this feature -- I don't see it in -next nor
> already in the tree. Is there any chance this is going to make the
> v4.9 merge window?
As I said in the cover letter, I'll rebase it on top of 4.9-rc1 as there
are some clean-ups that this series would conflict with. So I am
targeting 4.10 with this patch series.
> It didn't sound like there were any unresolved issues remaining?
There are a few issues spotted by Mark which I'll address in the next
version, but nothing major.
--
Catalin
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
2016-10-15 14:35 ` [kernel-hardening] " Catalin Marinas
@ 2016-10-16 2:04 ` Kees Cook
-1 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-10-16 2:04 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Oct 15, 2016 at 7:35 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> Hi Kees,
>
> On Fri, Oct 14, 2016 at 02:44:33PM -0700, Kees Cook wrote:
>> Just checking in on this feature -- I don't see it in -next nor
>> already in the tree. Is there any chance this is going to make the
>> v4.9 merge window?
>
> As I said in the cover letter, I'll rebase it on top of 4.9-rc1 as there
> are some clean-ups that this series would conflict with. So I am
> targeting 4.10 with this patch series.
>
>> It didn't sound like there were any unresolved issues remaining?
>
> There are a few issues spotted by Mark which I'll address in the next
> version, but nothing major.
Ah-ha! I misunderstood that to mean -rc2. :) Thanks for the update!
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread
* [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching
@ 2016-10-16 2:04 ` Kees Cook
0 siblings, 0 replies; 64+ messages in thread
From: Kees Cook @ 2016-10-16 2:04 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, Ard Biesheuvel, kernel-hardening, Will Deacon,
AKASHI Takahiro, James Morse, linux-arm-kernel
On Sat, Oct 15, 2016 at 7:35 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> Hi Kees,
>
> On Fri, Oct 14, 2016 at 02:44:33PM -0700, Kees Cook wrote:
>> Just checking in on this feature -- I don't see it in -next nor
>> already in the tree. Is there any chance this is going to make the
>> v4.9 merge window?
>
> As I said in the cover letter, I'll rebase it on top of 4.9-rc1 as there
> are some clean-ups that this series would conflict with. So I am
> targeting 4.10 with this patch series.
>
>> It didn't sound like there were any unresolved issues remaining?
>
> There are a few issues spotted by Mark which I'll address in the next
> version, but nothing major.
Ah-ha! I misunderstood that to mean -rc2. :) Thanks for the update!
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 64+ messages in thread