* [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
This is a fairly quick'n'dirty attempt at enabling virtually mapped
stacks for arm64, after learning from Mark yesterday that progress
had more or less stalled on that front.
Since having actual patches to poke at is infinitely better than having
abstract discussions about how to approach it, I threw some code together
that:
1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
the kernel; flames welcome :-) (#7)
2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
correctly as a platform register (#2, #3, #4, #5, #6)
3. dump the entire task stack if regs->sp points elsewhere (#8)
4. add the code that checks for a stack overflow and dumps the task context
before panic()king (#9, #10)
(#1 is an unrelated cleanup patch for copy_page())
So instead of unconditionally switching between stacks, this code simply uses
tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
we have a couple of general purpose registers to play with.
Tested with the following hunk applied
--- a/arch/arm64/crypto/aes-cipher-core.S
+++ b/arch/arm64/crypto/aes-cipher-core.S
@@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
.align 5
ENTRY(__aes_arm64_encrypt)
+ stp x29, x30, [sp, #-400]!
+ mov x29, sp
+
+ bl __aes_arm64_encrypt
+
do_crypt fround, crypto_ft_tab, crypto_fl_tab
ENDPROC(__aes_arm64_encrypt)
which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
missing: I suppose it should be possible to display something meaningful if x29
still points to a valid location, but I haven't looked into it yet.
BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
Internal error: Oops: 96000047 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
Hardware name: linux,dummy-virt (DT)
task: ffff80007c031a00 task.stack: ffff000009bdc000
PC is at __aes_arm64_encrypt+0x0/0x440
LR is at __aes_arm64_encrypt+0xc/0x440
pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
sp : ffff000009bdc120
x29: ffff000009bdc120 x28: ffff80007c181c00
x27: 0000000000000000 x26: 0000000000000100
x25: ffff0000089a52e8 x24: ffff000009bdfd10
x23: 0000000000000001 x22: ffff0000089a5408
x21: 0000000000000001 x20: ffff80007c181c08
x19: ffff80007c220000 x18: ffff80007c031a00
x17: 00000000002f0000 x16: ffff80007c181d24
x15: ffff0000089b2a68 x14: 00000000000003be
x13: 0000000000000071 x12: 00000000bf5fe8a9
x11: 0000000000000028 x10: 000000000000002c
x9 : ffff80007c181d20 x8 : 000000000000001b
x7 : 0000000046d4609c x6 : ffff0000080c5fd8
x5 : 0000000000000043 x4 : 0000000046d47b87
x3 : 000000000000000a x2 : ffff80007c220000
x1 : ffff80007c220000 x0 : ffff80007c181c80
Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
Stack: (0xffff000009bdc120 to 0xffff000009be0000)
c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
<snipped ...>
ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
Call trace:
Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
0ac0: ffff80007c220000 0001000000000000
0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
[<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
---[ end trace 2c6304a96ec827cb ]---
Ard Biesheuvel (10):
arm64/lib: copy_page: use consistent prefetch stride
arm64/lib: copy_page: avoid x18 register in assembler code
arm64: crypto: avoid register x18 in scalar AES code
arm64: kvm: stop treating register x18 as caller save
arm64: kernel: avoid x18 as an arbitrary temp register
arm64: kbuild: reserve reg x18 from general allocation by the compiler
arm64: kernel: switch to register x18 as a task struct pointer
arm64/kernel: dump entire stack if sp points elsewhere
arm64: mm: add C level handling for stack overflows
arm64: kernel: add support for virtually mapped stacks
arch/arm64/Kconfig | 1 +
arch/arm64/Makefile | 2 +-
arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
arch/arm64/include/asm/asm-uaccess.h | 3 +-
arch/arm64/include/asm/assembler.h | 8 +--
arch/arm64/include/asm/current.h | 6 +-
arch/arm64/include/asm/thread_info.h | 2 +
arch/arm64/kernel/cpu-reset.S | 4 +-
arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
arch/arm64/kernel/head.S | 6 +-
arch/arm64/kernel/process.c | 2 +-
arch/arm64/kernel/traps.c | 9 ++-
arch/arm64/kvm/hyp/entry.S | 12 ++--
arch/arm64/lib/Makefile | 3 +-
arch/arm64/lib/copy_page.S | 47 ++++++++-------
arch/arm64/mm/fault.c | 24 ++++++++
16 files changed, 154 insertions(+), 93 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
This is a fairly quick'n'dirty attempt at enabling virtually mapped
stacks for arm64, after learning from Mark yesterday that progress
had more or less stalled on that front.
Since having actual patches to poke at is infinitely better than having
abstract discussions about how to approach it, I threw some code together
that:
1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
the kernel; flames welcome :-) (#7)
2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
correctly as a platform register (#2, #3, #4, #5, #6)
3. dump the entire task stack if regs->sp points elsewhere (#8)
4. add the code that checks for a stack overflow and dumps the task context
before panic()king (#9, #10)
(#1 is an unrelated cleanup patch for copy_page())
So instead of unconditionally switching between stacks, this code simply uses
tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
we have a couple of general purpose registers to play with.
Tested with the following hunk applied
--- a/arch/arm64/crypto/aes-cipher-core.S
+++ b/arch/arm64/crypto/aes-cipher-core.S
@@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
.align 5
ENTRY(__aes_arm64_encrypt)
+ stp x29, x30, [sp, #-400]!
+ mov x29, sp
+
+ bl __aes_arm64_encrypt
+
do_crypt fround, crypto_ft_tab, crypto_fl_tab
ENDPROC(__aes_arm64_encrypt)
which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
missing: I suppose it should be possible to display something meaningful if x29
still points to a valid location, but I haven't looked into it yet.
BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
Internal error: Oops: 96000047 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
Hardware name: linux,dummy-virt (DT)
task: ffff80007c031a00 task.stack: ffff000009bdc000
PC is at __aes_arm64_encrypt+0x0/0x440
LR is at __aes_arm64_encrypt+0xc/0x440
pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
sp : ffff000009bdc120
x29: ffff000009bdc120 x28: ffff80007c181c00
x27: 0000000000000000 x26: 0000000000000100
x25: ffff0000089a52e8 x24: ffff000009bdfd10
x23: 0000000000000001 x22: ffff0000089a5408
x21: 0000000000000001 x20: ffff80007c181c08
x19: ffff80007c220000 x18: ffff80007c031a00
x17: 00000000002f0000 x16: ffff80007c181d24
x15: ffff0000089b2a68 x14: 00000000000003be
x13: 0000000000000071 x12: 00000000bf5fe8a9
x11: 0000000000000028 x10: 000000000000002c
x9 : ffff80007c181d20 x8 : 000000000000001b
x7 : 0000000046d4609c x6 : ffff0000080c5fd8
x5 : 0000000000000043 x4 : 0000000046d47b87
x3 : 000000000000000a x2 : ffff80007c220000
x1 : ffff80007c220000 x0 : ffff80007c181c80
Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
Stack: (0xffff000009bdc120 to 0xffff000009be0000)
c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
<snipped ...>
ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
Call trace:
Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
0ac0: ffff80007c220000 0001000000000000
0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
[<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
---[ end trace 2c6304a96ec827cb ]---
Ard Biesheuvel (10):
arm64/lib: copy_page: use consistent prefetch stride
arm64/lib: copy_page: avoid x18 register in assembler code
arm64: crypto: avoid register x18 in scalar AES code
arm64: kvm: stop treating register x18 as caller save
arm64: kernel: avoid x18 as an arbitrary temp register
arm64: kbuild: reserve reg x18 from general allocation by the compiler
arm64: kernel: switch to register x18 as a task struct pointer
arm64/kernel: dump entire stack if sp points elsewhere
arm64: mm: add C level handling for stack overflows
arm64: kernel: add support for virtually mapped stacks
arch/arm64/Kconfig | 1 +
arch/arm64/Makefile | 2 +-
arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
arch/arm64/include/asm/asm-uaccess.h | 3 +-
arch/arm64/include/asm/assembler.h | 8 +--
arch/arm64/include/asm/current.h | 6 +-
arch/arm64/include/asm/thread_info.h | 2 +
arch/arm64/kernel/cpu-reset.S | 4 +-
arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
arch/arm64/kernel/head.S | 6 +-
arch/arm64/kernel/process.c | 2 +-
arch/arm64/kernel/traps.c | 9 ++-
arch/arm64/kvm/hyp/entry.S | 12 ++--
arch/arm64/lib/Makefile | 3 +-
arch/arm64/lib/copy_page.S | 47 ++++++++-------
arch/arm64/mm/fault.c | 24 ++++++++
16 files changed, 154 insertions(+), 93 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 01/10] arm64/lib: copy_page: use consistent prefetch stride
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
The optional prefetch instructions in the copy_page() routine are
inconsistent: at the start of the function, two cachelines are
prefetched beyond the one being loaded in the first iteration, but
in the loop, the prefetch is one more line ahead. This appears to
be unintentional, so let's fix it.
While at it, fix the comment style and white space.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/lib/copy_page.S | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S
index c3cd65e31814..076c43715e64 100644
--- a/arch/arm64/lib/copy_page.S
+++ b/arch/arm64/lib/copy_page.S
@@ -30,9 +30,10 @@
*/
ENTRY(copy_page)
alternative_if ARM64_HAS_NO_HW_PREFETCH
- # Prefetch two cache lines ahead.
- prfm pldl1strm, [x1, #128]
- prfm pldl1strm, [x1, #256]
+ // Prefetch three cache lines ahead.
+ prfm pldl1strm, [x1, #128]
+ prfm pldl1strm, [x1, #256]
+ prfm pldl1strm, [x1, #384]
alternative_else_nop_endif
ldp x2, x3, [x1]
@@ -50,7 +51,7 @@ alternative_else_nop_endif
subs x18, x18, #128
alternative_if ARM64_HAS_NO_HW_PREFETCH
- prfm pldl1strm, [x1, #384]
+ prfm pldl1strm, [x1, #384]
alternative_else_nop_endif
stnp x2, x3, [x0]
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 01/10] arm64/lib: copy_page: use consistent prefetch stride
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
The optional prefetch instructions in the copy_page() routine are
inconsistent: at the start of the function, two cachelines are
prefetched beyond the one being loaded in the first iteration, but
in the loop, the prefetch is one more line ahead. This appears to
be unintentional, so let's fix it.
While at it, fix the comment style and white space.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/lib/copy_page.S | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S
index c3cd65e31814..076c43715e64 100644
--- a/arch/arm64/lib/copy_page.S
+++ b/arch/arm64/lib/copy_page.S
@@ -30,9 +30,10 @@
*/
ENTRY(copy_page)
alternative_if ARM64_HAS_NO_HW_PREFETCH
- # Prefetch two cache lines ahead.
- prfm pldl1strm, [x1, #128]
- prfm pldl1strm, [x1, #256]
+ // Prefetch three cache lines ahead.
+ prfm pldl1strm, [x1, #128]
+ prfm pldl1strm, [x1, #256]
+ prfm pldl1strm, [x1, #384]
alternative_else_nop_endif
ldp x2, x3, [x1]
@@ -50,7 +51,7 @@ alternative_else_nop_endif
subs x18, x18, #128
alternative_if ARM64_HAS_NO_HW_PREFETCH
- prfm pldl1strm, [x1, #384]
+ prfm pldl1strm, [x1, #384]
alternative_else_nop_endif
stnp x2, x3, [x0]
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 02/10] arm64/lib: copy_page: avoid x18 register in assembler code
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
Register x18 will no longer be used as a caller save register in the
future, so stop using it in the copy_page() code.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/lib/copy_page.S | 38 ++++++++++----------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S
index 076c43715e64..ba84a992b362 100644
--- a/arch/arm64/lib/copy_page.S
+++ b/arch/arm64/lib/copy_page.S
@@ -45,45 +45,45 @@ alternative_else_nop_endif
ldp x14, x15, [x1, #96]
ldp x16, x17, [x1, #112]
- mov x18, #(PAGE_SIZE - 128)
+ add x0, x0, #256
add x1, x1, #128
1:
- subs x18, x18, #128
+ tst x0, #(PAGE_SIZE - 1)
alternative_if ARM64_HAS_NO_HW_PREFETCH
prfm pldl1strm, [x1, #384]
alternative_else_nop_endif
- stnp x2, x3, [x0]
+ stnp x2, x3, [x0, #-256]
ldp x2, x3, [x1]
- stnp x4, x5, [x0, #16]
+ stnp x4, x5, [x0, #-240]
ldp x4, x5, [x1, #16]
- stnp x6, x7, [x0, #32]
+ stnp x6, x7, [x0, #-224]
ldp x6, x7, [x1, #32]
- stnp x8, x9, [x0, #48]
+ stnp x8, x9, [x0, #-208]
ldp x8, x9, [x1, #48]
- stnp x10, x11, [x0, #64]
+ stnp x10, x11, [x0, #-192]
ldp x10, x11, [x1, #64]
- stnp x12, x13, [x0, #80]
+ stnp x12, x13, [x0, #-176]
ldp x12, x13, [x1, #80]
- stnp x14, x15, [x0, #96]
+ stnp x14, x15, [x0, #-160]
ldp x14, x15, [x1, #96]
- stnp x16, x17, [x0, #112]
+ stnp x16, x17, [x0, #-144]
ldp x16, x17, [x1, #112]
add x0, x0, #128
add x1, x1, #128
- b.gt 1b
+ b.ne 1b
- stnp x2, x3, [x0]
- stnp x4, x5, [x0, #16]
- stnp x6, x7, [x0, #32]
- stnp x8, x9, [x0, #48]
- stnp x10, x11, [x0, #64]
- stnp x12, x13, [x0, #80]
- stnp x14, x15, [x0, #96]
- stnp x16, x17, [x0, #112]
+ stnp x2, x3, [x0, #-256]
+ stnp x4, x5, [x0, #-240]
+ stnp x6, x7, [x0, #-224]
+ stnp x8, x9, [x0, #-208]
+ stnp x10, x11, [x0, #-192]
+ stnp x12, x13, [x0, #-176]
+ stnp x14, x15, [x0, #-160]
+ stnp x16, x17, [x0, #-144]
ret
ENDPROC(copy_page)
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 02/10] arm64/lib: copy_page: avoid x18 register in assembler code
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
Register x18 will no longer be used as a caller save register in the
future, so stop using it in the copy_page() code.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/lib/copy_page.S | 38 ++++++++++----------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S
index 076c43715e64..ba84a992b362 100644
--- a/arch/arm64/lib/copy_page.S
+++ b/arch/arm64/lib/copy_page.S
@@ -45,45 +45,45 @@ alternative_else_nop_endif
ldp x14, x15, [x1, #96]
ldp x16, x17, [x1, #112]
- mov x18, #(PAGE_SIZE - 128)
+ add x0, x0, #256
add x1, x1, #128
1:
- subs x18, x18, #128
+ tst x0, #(PAGE_SIZE - 1)
alternative_if ARM64_HAS_NO_HW_PREFETCH
prfm pldl1strm, [x1, #384]
alternative_else_nop_endif
- stnp x2, x3, [x0]
+ stnp x2, x3, [x0, #-256]
ldp x2, x3, [x1]
- stnp x4, x5, [x0, #16]
+ stnp x4, x5, [x0, #-240]
ldp x4, x5, [x1, #16]
- stnp x6, x7, [x0, #32]
+ stnp x6, x7, [x0, #-224]
ldp x6, x7, [x1, #32]
- stnp x8, x9, [x0, #48]
+ stnp x8, x9, [x0, #-208]
ldp x8, x9, [x1, #48]
- stnp x10, x11, [x0, #64]
+ stnp x10, x11, [x0, #-192]
ldp x10, x11, [x1, #64]
- stnp x12, x13, [x0, #80]
+ stnp x12, x13, [x0, #-176]
ldp x12, x13, [x1, #80]
- stnp x14, x15, [x0, #96]
+ stnp x14, x15, [x0, #-160]
ldp x14, x15, [x1, #96]
- stnp x16, x17, [x0, #112]
+ stnp x16, x17, [x0, #-144]
ldp x16, x17, [x1, #112]
add x0, x0, #128
add x1, x1, #128
- b.gt 1b
+ b.ne 1b
- stnp x2, x3, [x0]
- stnp x4, x5, [x0, #16]
- stnp x6, x7, [x0, #32]
- stnp x8, x9, [x0, #48]
- stnp x10, x11, [x0, #64]
- stnp x12, x13, [x0, #80]
- stnp x14, x15, [x0, #96]
- stnp x16, x17, [x0, #112]
+ stnp x2, x3, [x0, #-256]
+ stnp x4, x5, [x0, #-240]
+ stnp x6, x7, [x0, #-224]
+ stnp x8, x9, [x0, #-208]
+ stnp x10, x11, [x0, #-192]
+ stnp x12, x13, [x0, #-176]
+ stnp x14, x15, [x0, #-160]
+ stnp x16, x17, [x0, #-144]
ret
ENDPROC(copy_page)
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 03/10] arm64: crypto: avoid register x18 in scalar AES code
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
Register x18 is the platform register, and is not unconditionally
classified as a caller save register by the AAPCS64 ABI. So avoid
using it in our AES assembler code, to allow the kernel to use it
as a task_struct pointer.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++++----------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/crypto/aes-cipher-core.S b/arch/arm64/crypto/aes-cipher-core.S
index f2f9cc519309..62c91b9fcd56 100644
--- a/arch/arm64/crypto/aes-cipher-core.S
+++ b/arch/arm64/crypto/aes-cipher-core.S
@@ -17,8 +17,7 @@
out .req x1
in .req x2
rounds .req x3
- tt .req x4
- lt .req x2
+ tt .req x2
.macro __pair, enc, reg0, reg1, in0, in1e, in1d, shift
ubfx \reg0, \in0, #\shift, #8
@@ -34,17 +33,17 @@
.macro __hround, out0, out1, in0, in1, in2, in3, t0, t1, enc
ldp \out0, \out1, [rk], #8
- __pair \enc, w13, w14, \in0, \in1, \in3, 0
- __pair \enc, w15, w16, \in1, \in2, \in0, 8
- __pair \enc, w17, w18, \in2, \in3, \in1, 16
+ __pair \enc, w12, w13, \in0, \in1, \in3, 0
+ __pair \enc, w14, w15, \in1, \in2, \in0, 8
+ __pair \enc, w16, w17, \in2, \in3, \in1, 16
__pair \enc, \t0, \t1, \in3, \in0, \in2, 24
- eor \out0, \out0, w13
- eor \out1, \out1, w14
- eor \out0, \out0, w15, ror #24
- eor \out1, \out1, w16, ror #24
- eor \out0, \out0, w17, ror #16
- eor \out1, \out1, w18, ror #16
+ eor \out0, \out0, w12
+ eor \out1, \out1, w13
+ eor \out0, \out0, w14, ror #24
+ eor \out1, \out1, w15, ror #24
+ eor \out0, \out0, w16, ror #16
+ eor \out1, \out1, w17, ror #16
eor \out0, \out0, \t0, ror #8
eor \out1, \out1, \t1, ror #8
.endm
@@ -60,42 +59,44 @@
.endm
.macro do_crypt, round, ttab, ltab
- ldp w5, w6, [in]
- ldp w7, w8, [in, #8]
- ldp w9, w10, [rk], #16
- ldp w11, w12, [rk, #-8]
+ ldp w4, w5, [in]
+ ldp w6, w7, [in, #8]
+ ldp w8, w9, [rk], #16
+ ldp w10, w11, [rk, #-8]
+CPU_BE( rev w4, w4 )
CPU_BE( rev w5, w5 )
CPU_BE( rev w6, w6 )
CPU_BE( rev w7, w7 )
-CPU_BE( rev w8, w8 )
+ eor w4, w4, w8
eor w5, w5, w9
eor w6, w6, w10
eor w7, w7, w11
- eor w8, w8, w12
adr_l tt, \ttab
- adr_l lt, \ltab
tbnz rounds, #1, 1f
-0: \round w9, w10, w11, w12, w5, w6, w7, w8
- \round w5, w6, w7, w8, w9, w10, w11, w12
+0: \round w8, w9, w10, w11, w4, w5, w6, w7
+ \round w4, w5, w6, w7, w8, w9, w10, w11
1: subs rounds, rounds, #4
- \round w9, w10, w11, w12, w5, w6, w7, w8
- csel tt, tt, lt, hi
- \round w5, w6, w7, w8, w9, w10, w11, w12
- b.hi 0b
+ \round w8, w9, w10, w11, w4, w5, w6, w7
+ b.ls 2f
+ \round w4, w5, w6, w7, w8, w9, w10, w11
+ b 0b
+2: adr_l tt, \ltab
+ \round w4, w5, w6, w7, w8, w9, w10, w11
+
+CPU_BE( rev w4, w4 )
CPU_BE( rev w5, w5 )
CPU_BE( rev w6, w6 )
CPU_BE( rev w7, w7 )
-CPU_BE( rev w8, w8 )
- stp w5, w6, [out]
- stp w7, w8, [out, #8]
+ stp w4, w5, [out]
+ stp w6, w7, [out, #8]
ret
.endm
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 03/10] arm64: crypto: avoid register x18 in scalar AES code
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
Register x18 is the platform register, and is not unconditionally
classified as a caller save register by the AAPCS64 ABI. So avoid
using it in our AES assembler code, to allow the kernel to use it
as a task_struct pointer.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++++----------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/crypto/aes-cipher-core.S b/arch/arm64/crypto/aes-cipher-core.S
index f2f9cc519309..62c91b9fcd56 100644
--- a/arch/arm64/crypto/aes-cipher-core.S
+++ b/arch/arm64/crypto/aes-cipher-core.S
@@ -17,8 +17,7 @@
out .req x1
in .req x2
rounds .req x3
- tt .req x4
- lt .req x2
+ tt .req x2
.macro __pair, enc, reg0, reg1, in0, in1e, in1d, shift
ubfx \reg0, \in0, #\shift, #8
@@ -34,17 +33,17 @@
.macro __hround, out0, out1, in0, in1, in2, in3, t0, t1, enc
ldp \out0, \out1, [rk], #8
- __pair \enc, w13, w14, \in0, \in1, \in3, 0
- __pair \enc, w15, w16, \in1, \in2, \in0, 8
- __pair \enc, w17, w18, \in2, \in3, \in1, 16
+ __pair \enc, w12, w13, \in0, \in1, \in3, 0
+ __pair \enc, w14, w15, \in1, \in2, \in0, 8
+ __pair \enc, w16, w17, \in2, \in3, \in1, 16
__pair \enc, \t0, \t1, \in3, \in0, \in2, 24
- eor \out0, \out0, w13
- eor \out1, \out1, w14
- eor \out0, \out0, w15, ror #24
- eor \out1, \out1, w16, ror #24
- eor \out0, \out0, w17, ror #16
- eor \out1, \out1, w18, ror #16
+ eor \out0, \out0, w12
+ eor \out1, \out1, w13
+ eor \out0, \out0, w14, ror #24
+ eor \out1, \out1, w15, ror #24
+ eor \out0, \out0, w16, ror #16
+ eor \out1, \out1, w17, ror #16
eor \out0, \out0, \t0, ror #8
eor \out1, \out1, \t1, ror #8
.endm
@@ -60,42 +59,44 @@
.endm
.macro do_crypt, round, ttab, ltab
- ldp w5, w6, [in]
- ldp w7, w8, [in, #8]
- ldp w9, w10, [rk], #16
- ldp w11, w12, [rk, #-8]
+ ldp w4, w5, [in]
+ ldp w6, w7, [in, #8]
+ ldp w8, w9, [rk], #16
+ ldp w10, w11, [rk, #-8]
+CPU_BE( rev w4, w4 )
CPU_BE( rev w5, w5 )
CPU_BE( rev w6, w6 )
CPU_BE( rev w7, w7 )
-CPU_BE( rev w8, w8 )
+ eor w4, w4, w8
eor w5, w5, w9
eor w6, w6, w10
eor w7, w7, w11
- eor w8, w8, w12
adr_l tt, \ttab
- adr_l lt, \ltab
tbnz rounds, #1, 1f
-0: \round w9, w10, w11, w12, w5, w6, w7, w8
- \round w5, w6, w7, w8, w9, w10, w11, w12
+0: \round w8, w9, w10, w11, w4, w5, w6, w7
+ \round w4, w5, w6, w7, w8, w9, w10, w11
1: subs rounds, rounds, #4
- \round w9, w10, w11, w12, w5, w6, w7, w8
- csel tt, tt, lt, hi
- \round w5, w6, w7, w8, w9, w10, w11, w12
- b.hi 0b
+ \round w8, w9, w10, w11, w4, w5, w6, w7
+ b.ls 2f
+ \round w4, w5, w6, w7, w8, w9, w10, w11
+ b 0b
+2: adr_l tt, \ltab
+ \round w4, w5, w6, w7, w8, w9, w10, w11
+
+CPU_BE( rev w4, w4 )
CPU_BE( rev w5, w5 )
CPU_BE( rev w6, w6 )
CPU_BE( rev w7, w7 )
-CPU_BE( rev w8, w8 )
- stp w5, w6, [out]
- stp w7, w8, [out, #8]
+ stp w4, w5, [out]
+ stp w6, w7, [out, #8]
ret
.endm
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 04/10] arm64: kvm: stop treating register x18 as caller save
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In preparation of using x18 as a task struct pointer register when
running in the kernel, stop treating it as caller save in the KVM
guest entry/exit code. Currently, the code assumes there is no need
to preserve it for the host, given that it would have been assumed
clobbered anyway by the function call to __guest_enter(). Instead,
preserve its value and restore it upon return.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kvm/hyp/entry.S | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 12ee62d6d410..75d31a161d3e 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -32,6 +32,7 @@
.pushsection .hyp.text, "ax"
.macro save_callee_saved_regs ctxt
+ str x18, [\ctxt, #CPU_XREG_OFFSET(18)]
stp x19, x20, [\ctxt, #CPU_XREG_OFFSET(19)]
stp x21, x22, [\ctxt, #CPU_XREG_OFFSET(21)]
stp x23, x24, [\ctxt, #CPU_XREG_OFFSET(23)]
@@ -47,6 +48,7 @@
ldp x25, x26, [\ctxt, #CPU_XREG_OFFSET(25)]
ldp x27, x28, [\ctxt, #CPU_XREG_OFFSET(27)]
ldp x29, lr, [\ctxt, #CPU_XREG_OFFSET(29)]
+ ldr x18, [\ctxt, #CPU_XREG_OFFSET(18)]
.endm
/*
@@ -78,12 +80,9 @@ ENTRY(__guest_enter)
ldp x14, x15, [x18, #CPU_XREG_OFFSET(14)]
ldp x16, x17, [x18, #CPU_XREG_OFFSET(16)]
- // Restore guest regs x19-x29, lr
+ // Restore guest regs x18-x29, lr
restore_callee_saved_regs x18
- // Restore guest reg x18
- ldr x18, [x18, #CPU_XREG_OFFSET(18)]
-
// Do not touch any register after this!
eret
ENDPROC(__guest_enter)
@@ -104,7 +103,7 @@ ENTRY(__guest_exit)
// Retrieve the guest regs x0-x1 from the stack
ldp x2, x3, [sp], #16 // x0, x1
- // Store the guest regs x0-x1 and x4-x18
+ // Store the guest regs x0-x1 and x4-x17
stp x2, x3, [x1, #CPU_XREG_OFFSET(0)]
stp x4, x5, [x1, #CPU_XREG_OFFSET(4)]
stp x6, x7, [x1, #CPU_XREG_OFFSET(6)]
@@ -113,9 +112,8 @@ ENTRY(__guest_exit)
stp x12, x13, [x1, #CPU_XREG_OFFSET(12)]
stp x14, x15, [x1, #CPU_XREG_OFFSET(14)]
stp x16, x17, [x1, #CPU_XREG_OFFSET(16)]
- str x18, [x1, #CPU_XREG_OFFSET(18)]
- // Store the guest regs x19-x29, lr
+ // Store the guest regs x18-x29, lr
save_callee_saved_regs x1
// Restore the host_ctxt from the stack
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 04/10] arm64: kvm: stop treating register x18 as caller save
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
In preparation of using x18 as a task struct pointer register when
running in the kernel, stop treating it as caller save in the KVM
guest entry/exit code. Currently, the code assumes there is no need
to preserve it for the host, given that it would have been assumed
clobbered anyway by the function call to __guest_enter(). Instead,
preserve its value and restore it upon return.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kvm/hyp/entry.S | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 12ee62d6d410..75d31a161d3e 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -32,6 +32,7 @@
.pushsection .hyp.text, "ax"
.macro save_callee_saved_regs ctxt
+ str x18, [\ctxt, #CPU_XREG_OFFSET(18)]
stp x19, x20, [\ctxt, #CPU_XREG_OFFSET(19)]
stp x21, x22, [\ctxt, #CPU_XREG_OFFSET(21)]
stp x23, x24, [\ctxt, #CPU_XREG_OFFSET(23)]
@@ -47,6 +48,7 @@
ldp x25, x26, [\ctxt, #CPU_XREG_OFFSET(25)]
ldp x27, x28, [\ctxt, #CPU_XREG_OFFSET(27)]
ldp x29, lr, [\ctxt, #CPU_XREG_OFFSET(29)]
+ ldr x18, [\ctxt, #CPU_XREG_OFFSET(18)]
.endm
/*
@@ -78,12 +80,9 @@ ENTRY(__guest_enter)
ldp x14, x15, [x18, #CPU_XREG_OFFSET(14)]
ldp x16, x17, [x18, #CPU_XREG_OFFSET(16)]
- // Restore guest regs x19-x29, lr
+ // Restore guest regs x18-x29, lr
restore_callee_saved_regs x18
- // Restore guest reg x18
- ldr x18, [x18, #CPU_XREG_OFFSET(18)]
-
// Do not touch any register after this!
eret
ENDPROC(__guest_enter)
@@ -104,7 +103,7 @@ ENTRY(__guest_exit)
// Retrieve the guest regs x0-x1 from the stack
ldp x2, x3, [sp], #16 // x0, x1
- // Store the guest regs x0-x1 and x4-x18
+ // Store the guest regs x0-x1 and x4-x17
stp x2, x3, [x1, #CPU_XREG_OFFSET(0)]
stp x4, x5, [x1, #CPU_XREG_OFFSET(4)]
stp x6, x7, [x1, #CPU_XREG_OFFSET(6)]
@@ -113,9 +112,8 @@ ENTRY(__guest_exit)
stp x12, x13, [x1, #CPU_XREG_OFFSET(12)]
stp x14, x15, [x1, #CPU_XREG_OFFSET(14)]
stp x16, x17, [x1, #CPU_XREG_OFFSET(16)]
- str x18, [x1, #CPU_XREG_OFFSET(18)]
- // Store the guest regs x19-x29, lr
+ // Store the guest regs x18-x29, lr
save_callee_saved_regs x1
// Restore the host_ctxt from the stack
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 05/10] arm64: kernel: avoid x18 as an arbitrary temp register
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
The code in __cpu_soft_restart() uses x18 as an arbitrary temp register,
which will shortly be disallowed. So use x8 instead.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/cpu-reset.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
index 65f42d257414..56e637e558c5 100644
--- a/arch/arm64/kernel/cpu-reset.S
+++ b/arch/arm64/kernel/cpu-reset.S
@@ -44,11 +44,11 @@ ENTRY(__cpu_soft_restart)
mov x0, #HVC_SOFT_RESTART
hvc #0 // no return
-1: mov x18, x1 // entry
+1: mov x8, x1 // entry
mov x0, x2 // arg0
mov x1, x3 // arg1
mov x2, x4 // arg2
- br x18
+ br x8
ENDPROC(__cpu_soft_restart)
.popsection
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 05/10] arm64: kernel: avoid x18 as an arbitrary temp register
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
The code in __cpu_soft_restart() uses x18 as an arbitrary temp register,
which will shortly be disallowed. So use x8 instead.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/cpu-reset.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
index 65f42d257414..56e637e558c5 100644
--- a/arch/arm64/kernel/cpu-reset.S
+++ b/arch/arm64/kernel/cpu-reset.S
@@ -44,11 +44,11 @@ ENTRY(__cpu_soft_restart)
mov x0, #HVC_SOFT_RESTART
hvc #0 // no return
-1: mov x18, x1 // entry
+1: mov x8, x1 // entry
mov x0, x2 // arg0
mov x1, x3 // arg1
mov x2, x4 // arg2
- br x18
+ br x8
ENDPROC(__cpu_soft_restart)
.popsection
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 06/10] arm64: kbuild: reserve reg x18 from general allocation by the compiler
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
Before we can start using register x18 for a special purpose (as permitted
by the AAPCS64 ABI), we need to tell the compiler that it is off limits
for general allocation. So tag it as 'fixed', and remove the mention from
the LL/SC compiler flag override.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/Makefile | 2 +-
arch/arm64/lib/Makefile | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 9b41f1e3b1a0..68164338185a 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -46,7 +46,7 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
endif
KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst)
-KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
+KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -ffixed-x18
KBUILD_CFLAGS += $(call cc-option, -mpc-relative-literal-loads)
KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index c86b7909ef31..b52f34fd5ba5 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -15,5 +15,4 @@ CFLAGS_atomic_ll_sc.o := -fcall-used-x0 -ffixed-x1 -ffixed-x2 \
-ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 \
-ffixed-x7 -fcall-saved-x8 -fcall-saved-x9 \
-fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12 \
- -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15 \
- -fcall-saved-x18
+ -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 06/10] arm64: kbuild: reserve reg x18 from general allocation by the compiler
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
Before we can start using register x18 for a special purpose (as permitted
by the AAPCS64 ABI), we need to tell the compiler that it is off limits
for general allocation. So tag it as 'fixed', and remove the mention from
the LL/SC compiler flag override.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/Makefile | 2 +-
arch/arm64/lib/Makefile | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 9b41f1e3b1a0..68164338185a 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -46,7 +46,7 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
endif
KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst)
-KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
+KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -ffixed-x18
KBUILD_CFLAGS += $(call cc-option, -mpc-relative-literal-loads)
KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index c86b7909ef31..b52f34fd5ba5 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -15,5 +15,4 @@ CFLAGS_atomic_ll_sc.o := -fcall-used-x0 -ffixed-x1 -ffixed-x2 \
-ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 \
-ffixed-x7 -fcall-saved-x8 -fcall-saved-x9 \
-fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12 \
- -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15 \
- -fcall-saved-x18
+ -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In order to free up sp_el0, which we will need to deal with faulting
stack accesses when using virtually mapped stacks, switch to register
x18 as the task struct register. This is permitted by the AAPCS64 ABI,
and simplifies many references to 'current', given that they no longer
involve a MSR instruction to access SP_EL0.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/asm-uaccess.h | 3 +--
arch/arm64/include/asm/assembler.h | 8 +-------
arch/arm64/include/asm/current.h | 6 ++----
arch/arm64/kernel/entry.S | 14 ++------------
arch/arm64/kernel/head.S | 6 ++----
arch/arm64/kernel/process.c | 2 +-
6 files changed, 9 insertions(+), 30 deletions(-)
diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h
index ecd9788cd298..db5af0dda311 100644
--- a/arch/arm64/include/asm/asm-uaccess.h
+++ b/arch/arm64/include/asm/asm-uaccess.h
@@ -18,8 +18,7 @@
.endm
.macro __uaccess_ttbr0_enable, tmp1
- get_thread_info \tmp1
- ldr \tmp1, [\tmp1, #TSK_TI_TTBR0] // load saved TTBR0_EL1
+ ldr \tmp1, [tsk, #TSK_TI_TTBR0] // load saved TTBR0_EL1
msr ttbr0_el1, \tmp1 // set the non-PAN TTBR0_EL1
isb
.endm
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 1b67c3782d00..de3335c21632 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -123,6 +123,7 @@
* Register aliases.
*/
lr .req x30 // link register
+tsk .req x18 // current task_struct
/*
* Vector entry
@@ -435,13 +436,6 @@ alternative_endif
.endm
/*
- * Return the current thread_info.
- */
- .macro get_thread_info, rd
- mrs \rd, sp_el0
- .endm
-
-/*
* Errata workaround prior to TTBR0_EL1 update
*
* val: TTBR value with new BADDR, preserved
diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
index f6580d4afb0e..b4e3acff699c 100644
--- a/arch/arm64/include/asm/current.h
+++ b/arch/arm64/include/asm/current.h
@@ -13,11 +13,9 @@ struct task_struct;
*/
static __always_inline struct task_struct *get_current(void)
{
- unsigned long sp_el0;
+ register unsigned long tsk asm ("x18");
- asm ("mrs %0, sp_el0" : "=r" (sp_el0));
-
- return (struct task_struct *)sp_el0;
+ return (struct task_struct *)tsk;
}
#define current get_current()
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index b738880350f9..2ba3185b1c78 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -99,7 +99,6 @@
mov x29, xzr // fp pointed to user-space
.else
add x21, sp, #S_FRAME_SIZE
- get_thread_info tsk
/* Save the task's original addr_limit and set USER_DS (TASK_SIZE_64) */
ldr x20, [tsk, #TSK_TI_ADDR_LIMIT]
str x20, [sp, #S_ORIG_ADDR_LIMIT]
@@ -147,13 +146,6 @@ alternative_else_nop_endif
.endif
/*
- * Set sp_el0 to current thread_info.
- */
- .if \el == 0
- msr sp_el0, tsk
- .endif
-
- /*
* Registers that may be useful after this macro is invoked:
*
* x21 - aborted SP
@@ -293,7 +285,6 @@ alternative_else_nop_endif
sc_nr .req x25 // number of system calls
scno .req x26 // syscall number
stbl .req x27 // syscall table pointer
-tsk .req x28 // current thread_info
/*
* Interrupt handling.
@@ -734,7 +725,7 @@ ENTRY(cpu_switch_to)
ldp x29, x9, [x8], #16
ldr lr, [x8]
mov sp, x9
- msr sp_el0, x1
+ mov tsk, x1
ret
ENDPROC(cpu_switch_to)
@@ -788,8 +779,7 @@ ENTRY(ret_from_fork)
cbz x19, 1f // not a kernel thread
mov x0, x20
blr x19
-1: get_thread_info tsk
- b ret_to_user
+1: b ret_to_user
ENDPROC(ret_from_fork)
/*
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 973df7de7bf8..9b2bb2b08b4f 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -323,8 +323,7 @@ ENDPROC(__create_page_tables)
__primary_switched:
adrp x4, init_thread_union
add sp, x4, #THREAD_SIZE
- adr_l x5, init_task
- msr sp_el0, x5 // Save thread_info
+ adr_l tsk, init_task // Save thread_info
adr_l x8, vectors // load VBAR_EL1 with virtual
msr vbar_el1, x8 // vector table address
@@ -614,8 +613,7 @@ __secondary_switched:
adr_l x0, secondary_data
ldr x1, [x0, #CPU_BOOT_STACK] // get secondary_data.stack
mov sp, x1
- ldr x2, [x0, #CPU_BOOT_TASK]
- msr sp_el0, x2
+ ldr tsk, [x0, #CPU_BOOT_TASK]
mov x29, #0
b secondary_start_kernel
ENDPROC(__secondary_switched)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 659ae8094ed5..b0c9caa9da11 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -329,7 +329,7 @@ void uao_thread_switch(struct task_struct *next)
}
/*
- * We store our current task in sp_el0, which is clobbered by userspace. Keep a
+ * We store our current task in x18, which is clobbered by userspace. Keep a
* shadow copy so that we can restore this upon entry from userspace.
*
* This is *only* for exception entry from EL0, and is not valid until we
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
In order to free up sp_el0, which we will need to deal with faulting
stack accesses when using virtually mapped stacks, switch to register
x18 as the task struct register. This is permitted by the AAPCS64 ABI,
and simplifies many references to 'current', given that they no longer
involve a MSR instruction to access SP_EL0.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/asm-uaccess.h | 3 +--
arch/arm64/include/asm/assembler.h | 8 +-------
arch/arm64/include/asm/current.h | 6 ++----
arch/arm64/kernel/entry.S | 14 ++------------
arch/arm64/kernel/head.S | 6 ++----
arch/arm64/kernel/process.c | 2 +-
6 files changed, 9 insertions(+), 30 deletions(-)
diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h
index ecd9788cd298..db5af0dda311 100644
--- a/arch/arm64/include/asm/asm-uaccess.h
+++ b/arch/arm64/include/asm/asm-uaccess.h
@@ -18,8 +18,7 @@
.endm
.macro __uaccess_ttbr0_enable, tmp1
- get_thread_info \tmp1
- ldr \tmp1, [\tmp1, #TSK_TI_TTBR0] // load saved TTBR0_EL1
+ ldr \tmp1, [tsk, #TSK_TI_TTBR0] // load saved TTBR0_EL1
msr ttbr0_el1, \tmp1 // set the non-PAN TTBR0_EL1
isb
.endm
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 1b67c3782d00..de3335c21632 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -123,6 +123,7 @@
* Register aliases.
*/
lr .req x30 // link register
+tsk .req x18 // current task_struct
/*
* Vector entry
@@ -435,13 +436,6 @@ alternative_endif
.endm
/*
- * Return the current thread_info.
- */
- .macro get_thread_info, rd
- mrs \rd, sp_el0
- .endm
-
-/*
* Errata workaround prior to TTBR0_EL1 update
*
* val: TTBR value with new BADDR, preserved
diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
index f6580d4afb0e..b4e3acff699c 100644
--- a/arch/arm64/include/asm/current.h
+++ b/arch/arm64/include/asm/current.h
@@ -13,11 +13,9 @@ struct task_struct;
*/
static __always_inline struct task_struct *get_current(void)
{
- unsigned long sp_el0;
+ register unsigned long tsk asm ("x18");
- asm ("mrs %0, sp_el0" : "=r" (sp_el0));
-
- return (struct task_struct *)sp_el0;
+ return (struct task_struct *)tsk;
}
#define current get_current()
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index b738880350f9..2ba3185b1c78 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -99,7 +99,6 @@
mov x29, xzr // fp pointed to user-space
.else
add x21, sp, #S_FRAME_SIZE
- get_thread_info tsk
/* Save the task's original addr_limit and set USER_DS (TASK_SIZE_64) */
ldr x20, [tsk, #TSK_TI_ADDR_LIMIT]
str x20, [sp, #S_ORIG_ADDR_LIMIT]
@@ -147,13 +146,6 @@ alternative_else_nop_endif
.endif
/*
- * Set sp_el0 to current thread_info.
- */
- .if \el == 0
- msr sp_el0, tsk
- .endif
-
- /*
* Registers that may be useful after this macro is invoked:
*
* x21 - aborted SP
@@ -293,7 +285,6 @@ alternative_else_nop_endif
sc_nr .req x25 // number of system calls
scno .req x26 // syscall number
stbl .req x27 // syscall table pointer
-tsk .req x28 // current thread_info
/*
* Interrupt handling.
@@ -734,7 +725,7 @@ ENTRY(cpu_switch_to)
ldp x29, x9, [x8], #16
ldr lr, [x8]
mov sp, x9
- msr sp_el0, x1
+ mov tsk, x1
ret
ENDPROC(cpu_switch_to)
@@ -788,8 +779,7 @@ ENTRY(ret_from_fork)
cbz x19, 1f // not a kernel thread
mov x0, x20
blr x19
-1: get_thread_info tsk
- b ret_to_user
+1: b ret_to_user
ENDPROC(ret_from_fork)
/*
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 973df7de7bf8..9b2bb2b08b4f 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -323,8 +323,7 @@ ENDPROC(__create_page_tables)
__primary_switched:
adrp x4, init_thread_union
add sp, x4, #THREAD_SIZE
- adr_l x5, init_task
- msr sp_el0, x5 // Save thread_info
+ adr_l tsk, init_task // Save thread_info
adr_l x8, vectors // load VBAR_EL1 with virtual
msr vbar_el1, x8 // vector table address
@@ -614,8 +613,7 @@ __secondary_switched:
adr_l x0, secondary_data
ldr x1, [x0, #CPU_BOOT_STACK] // get secondary_data.stack
mov sp, x1
- ldr x2, [x0, #CPU_BOOT_TASK]
- msr sp_el0, x2
+ ldr tsk, [x0, #CPU_BOOT_TASK]
mov x29, #0
b secondary_start_kernel
ENDPROC(__secondary_switched)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 659ae8094ed5..b0c9caa9da11 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -329,7 +329,7 @@ void uao_thread_switch(struct task_struct *next)
}
/*
- * We store our current task in sp_el0, which is clobbered by userspace. Keep a
+ * We store our current task in x18, which is clobbered by userspace. Keep a
* shadow copy so that we can restore this upon entry from userspace.
*
* This is *only* for exception entry from EL0, and is not valid until we
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 08/10] arm64/kernel: dump entire stack if sp points elsewhere
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
Before adding handling for out of bounds stack accesses, update the
stack dumping logic to disregard regs->sp if it does not point into
the task stack anymore, and simply dump the entire stack instead.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/traps.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c7c7088097be..017c92b2d707 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -257,7 +257,14 @@ static int __die(const char *str, int err, struct pt_regs *regs)
end_of_stack(tsk));
if (!user_mode(regs)) {
- dump_mem(KERN_EMERG, "Stack: ", regs->sp,
+ u64 task_sp = regs->sp;
+
+ /* dump the entire stack if sp no longer points into it */
+ if (task_sp < (u64)task_stack_page(tsk) ||
+ task_sp > (u64)task_stack_page(tsk) + THREAD_SIZE)
+ task_sp = (u64)task_stack_page(tsk);
+
+ dump_mem(KERN_EMERG, "Stack: ", task_sp,
THREAD_SIZE + (unsigned long)task_stack_page(tsk));
dump_backtrace(regs, tsk);
dump_instr(KERN_EMERG, regs);
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 08/10] arm64/kernel: dump entire stack if sp points elsewhere
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
Before adding handling for out of bounds stack accesses, update the
stack dumping logic to disregard regs->sp if it does not point into
the task stack anymore, and simply dump the entire stack instead.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/traps.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c7c7088097be..017c92b2d707 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -257,7 +257,14 @@ static int __die(const char *str, int err, struct pt_regs *regs)
end_of_stack(tsk));
if (!user_mode(regs)) {
- dump_mem(KERN_EMERG, "Stack: ", regs->sp,
+ u64 task_sp = regs->sp;
+
+ /* dump the entire stack if sp no longer points into it */
+ if (task_sp < (u64)task_stack_page(tsk) ||
+ task_sp > (u64)task_stack_page(tsk) + THREAD_SIZE)
+ task_sp = (u64)task_stack_page(tsk);
+
+ dump_mem(KERN_EMERG, "Stack: ", task_sp,
THREAD_SIZE + (unsigned long)task_stack_page(tsk));
dump_backtrace(regs, tsk);
dump_instr(KERN_EMERG, regs);
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 09/10] arm64: mm: add C level handling for stack overflows
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
Add the code to __do_kernel_fault() to force a panic when the faulting
address of a data abort points into the guard page below the current
task's stack.
Currently, we won't be able to make it all the way here under such a
condition, but that will be addressed in a subsequent patch.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/mm/fault.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c7861c9864e6..b3317e5ff5dd 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -229,6 +229,21 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
return;
/*
+ * If we faulted on the guard page below this task's stack,
+ * we evidently overflowed
+ */
+ if (addr >= (u64)current->stack - PAGE_SIZE &&
+ addr < (u64)current->stack) {
+ printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",
+ (void *)addr, current->stack,
+ (char *)current->stack + THREAD_SIZE - 1);
+ die("Oops", regs, esr);
+
+ /* Be absolutely certain we don't return. */
+ panic("Kernel stack overflow");
+ }
+
+ /*
* No handler, we'll have to terminate things with extreme prejudice.
*/
bust_spinlocks(1);
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 09/10] arm64: mm: add C level handling for stack overflows
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
Add the code to __do_kernel_fault() to force a panic when the faulting
address of a data abort points into the guard page below the current
task's stack.
Currently, we won't be able to make it all the way here under such a
condition, but that will be addressed in a subsequent patch.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/mm/fault.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c7861c9864e6..b3317e5ff5dd 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -229,6 +229,21 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
return;
/*
+ * If we faulted on the guard page below this task's stack,
+ * we evidently overflowed
+ */
+ if (addr >= (u64)current->stack - PAGE_SIZE &&
+ addr < (u64)current->stack) {
+ printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",
+ (void *)addr, current->stack,
+ (char *)current->stack + THREAD_SIZE - 1);
+ die("Oops", regs, esr);
+
+ /* Be absolutely certain we don't return. */
+ panic("Kernel stack overflow");
+ }
+
+ /*
* No handler, we'll have to terminate things with extreme prejudice.
*/
bust_spinlocks(1);
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 14:44 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel
Add code that checks whether an exception taken from EL1 was caused
by a faulting stack access before proceeding to save the interrupted
context to the stack.
This involves checking whether the faulting address coincides with the
guard page below the task stack of 'current'. This uses tpidrro_el0 and
sp_el0 as scratch registers, so we can free up a couple of general
purpose registers for use in the code that performs this check. If it
turns out we are dealing with a stack overflow, switch to a special
per-CPU overflow stack so we can at least call panic().
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/thread_info.h | 2 +
arch/arm64/kernel/entry.S | 49 ++++++++++++++++++++
arch/arm64/mm/fault.c | 9 ++++
4 files changed, 61 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b52db8bb1270..50caf63099c8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -73,6 +73,7 @@ config ARM64
select HAVE_ARCH_SECCOMP_FILTER
select HAVE_ARCH_TRACEHOOK
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
+ select HAVE_ARCH_VMAP_STACK
select HAVE_ARM_SMCCC
select HAVE_EBPF_JIT
select HAVE_C_RECORDMCOUNT
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93cf865..1c3e0a3bf87a 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -32,6 +32,8 @@
#define THREAD_SIZE 16384
#define THREAD_START_SP (THREAD_SIZE - 16)
+#define OVERFLOW_STACK_SIZE 1024
+
#ifndef __ASSEMBLY__
struct task_struct;
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 2ba3185b1c78..4c3e82d6e2f2 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
*/
.align 6
el1_sync:
+#ifdef CONFIG_VMAP_STACK
+ /*
+ * When taking an exception from EL1, we need to check whether it is
+ * caused by a faulting out-of-bounds access to the virtually mapped
+ * stack before we can attempt to preserve the interrupted context.
+ */
+ msr tpidrro_el0, x0 // stash x0
+ mrs x0, far_el1 // get faulting address
+ tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
+
+.Lcheck_stack_resume:
+ mrs x0, tpidrro_el0 // restore x0
+#endif
+
kernel_entry 1
mrs x1, esr_el1 // read the syndrome register
lsr x24, x1, #ESR_ELx_EC_SHIFT // exception class
@@ -411,6 +425,41 @@ el1_sync:
b.ge el1_dbg
b el1_inv
+#ifdef CONFIG_VMAP_STACK
+.Lcheck_stack_ovf:
+ /*
+ * Check if the faulting address is above PAGE_OFFSET, which rules out
+ * the vmapped stacks living in the VMALLOC region.
+ */
+ tbnz x0, #(VA_BITS - 2), .Lcheck_stack_resume
+
+ /*
+ * Check whether the faulting address hit a guard page below our
+ * virtually mapped stack. This is a strong hint that we may be
+ * dealing with a stack overflow.
+ */
+ msr sp_el0, x1 // stash x1
+ ldr x1, [tsk, #TSK_STACK] // get task's stack base
+ sub x1, x1, x0 // subtract FAR from stack base
+ tst x1, #~(PAGE_SIZE - 1) // disregard bits within page
+ mrs x1, sp_el0 // restore x1
+ b.ne .Lcheck_stack_resume // proceed if no stack overflow
+
+ /*
+ * We are not going to recover from a stack overflow in kernel mode,
+ * but we would like to report this condition to the user, which means
+ * we need another stack.
+ */
+ mov x0, sp
+ msr sp_el0, x0 // stash the faulting sp
+
+ adr_l x0, overflow_stack + OVERFLOW_STACK_SIZE
+ sub sp, x0, #S_FRAME_SIZE
+ mrs x0, tpidr_el1
+ add sp, sp, x0
+ b .Lcheck_stack_resume
+#endif
+
el1_ia:
/*
* Fall through to the Data abort case
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b3317e5ff5dd..9ecd47572656 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -45,6 +45,11 @@
#include <acpi/ghes.h>
+#ifdef CONFIG_VMAP_STACK
+DEFINE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack)
+ __aligned(16);
+#endif
+
struct fault_info {
int (*fn)(unsigned long addr, unsigned int esr,
struct pt_regs *regs);
@@ -234,6 +239,10 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
*/
if (addr >= (u64)current->stack - PAGE_SIZE &&
addr < (u64)current->stack) {
+
+ /* fix up regs->sp, we stashed the faulting value in sp_el0 */
+ regs->sp = read_sysreg(sp_el0);
+
printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",
(void *)addr, current->stack,
(char *)current->stack + THREAD_SIZE - 1);
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [kernel-hardening] [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
@ 2017-07-12 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 14:44 UTC (permalink / raw)
To: linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Ard Biesheuvel
Add code that checks whether an exception taken from EL1 was caused
by a faulting stack access before proceeding to save the interrupted
context to the stack.
This involves checking whether the faulting address coincides with the
guard page below the task stack of 'current'. This uses tpidrro_el0 and
sp_el0 as scratch registers, so we can free up a couple of general
purpose registers for use in the code that performs this check. If it
turns out we are dealing with a stack overflow, switch to a special
per-CPU overflow stack so we can at least call panic().
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/thread_info.h | 2 +
arch/arm64/kernel/entry.S | 49 ++++++++++++++++++++
arch/arm64/mm/fault.c | 9 ++++
4 files changed, 61 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b52db8bb1270..50caf63099c8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -73,6 +73,7 @@ config ARM64
select HAVE_ARCH_SECCOMP_FILTER
select HAVE_ARCH_TRACEHOOK
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
+ select HAVE_ARCH_VMAP_STACK
select HAVE_ARM_SMCCC
select HAVE_EBPF_JIT
select HAVE_C_RECORDMCOUNT
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93cf865..1c3e0a3bf87a 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -32,6 +32,8 @@
#define THREAD_SIZE 16384
#define THREAD_START_SP (THREAD_SIZE - 16)
+#define OVERFLOW_STACK_SIZE 1024
+
#ifndef __ASSEMBLY__
struct task_struct;
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 2ba3185b1c78..4c3e82d6e2f2 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
*/
.align 6
el1_sync:
+#ifdef CONFIG_VMAP_STACK
+ /*
+ * When taking an exception from EL1, we need to check whether it is
+ * caused by a faulting out-of-bounds access to the virtually mapped
+ * stack before we can attempt to preserve the interrupted context.
+ */
+ msr tpidrro_el0, x0 // stash x0
+ mrs x0, far_el1 // get faulting address
+ tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
+
+.Lcheck_stack_resume:
+ mrs x0, tpidrro_el0 // restore x0
+#endif
+
kernel_entry 1
mrs x1, esr_el1 // read the syndrome register
lsr x24, x1, #ESR_ELx_EC_SHIFT // exception class
@@ -411,6 +425,41 @@ el1_sync:
b.ge el1_dbg
b el1_inv
+#ifdef CONFIG_VMAP_STACK
+.Lcheck_stack_ovf:
+ /*
+ * Check if the faulting address is above PAGE_OFFSET, which rules out
+ * the vmapped stacks living in the VMALLOC region.
+ */
+ tbnz x0, #(VA_BITS - 2), .Lcheck_stack_resume
+
+ /*
+ * Check whether the faulting address hit a guard page below our
+ * virtually mapped stack. This is a strong hint that we may be
+ * dealing with a stack overflow.
+ */
+ msr sp_el0, x1 // stash x1
+ ldr x1, [tsk, #TSK_STACK] // get task's stack base
+ sub x1, x1, x0 // subtract FAR from stack base
+ tst x1, #~(PAGE_SIZE - 1) // disregard bits within page
+ mrs x1, sp_el0 // restore x1
+ b.ne .Lcheck_stack_resume // proceed if no stack overflow
+
+ /*
+ * We are not going to recover from a stack overflow in kernel mode,
+ * but we would like to report this condition to the user, which means
+ * we need another stack.
+ */
+ mov x0, sp
+ msr sp_el0, x0 // stash the faulting sp
+
+ adr_l x0, overflow_stack + OVERFLOW_STACK_SIZE
+ sub sp, x0, #S_FRAME_SIZE
+ mrs x0, tpidr_el1
+ add sp, sp, x0
+ b .Lcheck_stack_resume
+#endif
+
el1_ia:
/*
* Fall through to the Data abort case
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b3317e5ff5dd..9ecd47572656 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -45,6 +45,11 @@
#include <acpi/ghes.h>
+#ifdef CONFIG_VMAP_STACK
+DEFINE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack)
+ __aligned(16);
+#endif
+
struct fault_info {
int (*fn)(unsigned long addr, unsigned int esr,
struct pt_regs *regs);
@@ -234,6 +239,10 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
*/
if (addr >= (u64)current->stack - PAGE_SIZE &&
addr < (u64)current->stack) {
+
+ /* fix up regs->sp, we stashed the faulting value in sp_el0 */
+ regs->sp = read_sysreg(sp_el0);
+
printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",
(void *)addr, current->stack,
(char *)current->stack + THREAD_SIZE - 1);
--
2.9.3
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 20:12 ` Laura Abbott
-1 siblings, 0 replies; 44+ messages in thread
From: Laura Abbott @ 2017-07-12 20:12 UTC (permalink / raw)
To: linux-arm-kernel
On 07/12/2017 07:44 AM, Ard Biesheuvel wrote:
> This is a fairly quick'n'dirty attempt at enabling virtually mapped
> stacks for arm64, after learning from Mark yesterday that progress
> had more or less stalled on that front.
>
> Since having actual patches to poke at is infinitely better than having
> abstract discussions about how to approach it, I threw some code together
> that:
> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
> the kernel; flames welcome :-) (#7)
> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
> correctly as a platform register (#2, #3, #4, #5, #6)
> 3. dump the entire task stack if regs->sp points elsewhere (#8)
> 4. add the code that checks for a stack overflow and dumps the task context
> before panic()king (#9, #10)
>
> (#1 is an unrelated cleanup patch for copy_page())
>
> So instead of unconditionally switching between stacks, this code simply uses
> tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
> we have a couple of general purpose registers to play with.
>
> Tested with the following hunk applied
>
> --- a/arch/arm64/crypto/aes-cipher-core.S
> +++ b/arch/arm64/crypto/aes-cipher-core.S
> @@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
>
> .align 5
> ENTRY(__aes_arm64_encrypt)
> + stp x29, x30, [sp, #-400]!
> + mov x29, sp
> +
> + bl __aes_arm64_encrypt
> +
> do_crypt fround, crypto_ft_tab, crypto_fl_tab
> ENDPROC(__aes_arm64_encrypt)
>
> which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
> and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
> missing: I suppose it should be possible to display something meaningful if x29
> still points to a valid location, but I haven't looked into it yet.
>
> BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
> Internal error: Oops: 96000047 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
> Hardware name: linux,dummy-virt (DT)
> task: ffff80007c031a00 task.stack: ffff000009bdc000
> PC is at __aes_arm64_encrypt+0x0/0x440
> LR is at __aes_arm64_encrypt+0xc/0x440
> pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
> sp : ffff000009bdc120
> x29: ffff000009bdc120 x28: ffff80007c181c00
> x27: 0000000000000000 x26: 0000000000000100
> x25: ffff0000089a52e8 x24: ffff000009bdfd10
> x23: 0000000000000001 x22: ffff0000089a5408
> x21: 0000000000000001 x20: ffff80007c181c08
> x19: ffff80007c220000 x18: ffff80007c031a00
> x17: 00000000002f0000 x16: ffff80007c181d24
> x15: ffff0000089b2a68 x14: 00000000000003be
> x13: 0000000000000071 x12: 00000000bf5fe8a9
> x11: 0000000000000028 x10: 000000000000002c
> x9 : ffff80007c181d20 x8 : 000000000000001b
> x7 : 0000000046d4609c x6 : ffff0000080c5fd8
> x5 : 0000000000000043 x4 : 0000000046d47b87
> x3 : 000000000000000a x2 : ffff80007c220000
> x1 : ffff80007c220000 x0 : ffff80007c181c80
> Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
> Stack: (0xffff000009bdc120 to 0xffff000009be0000)
> c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
> c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> <snipped ...>
> ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
> ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> Call trace:
> Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
> 0ac0: ffff80007c220000 0001000000000000
> 0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
> 0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
> 0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
> 0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
> 0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
> 0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
> [<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
> Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
> ---[ end trace 2c6304a96ec827cb ]---
>
> Ard Biesheuvel (10):
> arm64/lib: copy_page: use consistent prefetch stride
> arm64/lib: copy_page: avoid x18 register in assembler code
> arm64: crypto: avoid register x18 in scalar AES code
> arm64: kvm: stop treating register x18 as caller save
> arm64: kernel: avoid x18 as an arbitrary temp register
> arm64: kbuild: reserve reg x18 from general allocation by the compiler
> arm64: kernel: switch to register x18 as a task struct pointer
> arm64/kernel: dump entire stack if sp points elsewhere
> arm64: mm: add C level handling for stack overflows
> arm64: kernel: add support for virtually mapped stacks
>
> arch/arm64/Kconfig | 1 +
> arch/arm64/Makefile | 2 +-
> arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
> arch/arm64/include/asm/asm-uaccess.h | 3 +-
> arch/arm64/include/asm/assembler.h | 8 +--
> arch/arm64/include/asm/current.h | 6 +-
> arch/arm64/include/asm/thread_info.h | 2 +
> arch/arm64/kernel/cpu-reset.S | 4 +-
> arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/process.c | 2 +-
> arch/arm64/kernel/traps.c | 9 ++-
> arch/arm64/kvm/hyp/entry.S | 12 ++--
> arch/arm64/lib/Makefile | 3 +-
> arch/arm64/lib/copy_page.S | 47 ++++++++-------
> arch/arm64/mm/fault.c | 24 ++++++++
> 16 files changed, 154 insertions(+), 93 deletions(-)
>
This fails to compile with 64K pages
kernel/fork.c: In function ?free_thread_stack?:
kernel/fork.c:267:41: error: ?THREAD_SIZE_ORDER? undeclared (first use in this function); did you mean ?THREAD_SIZE??
__free_pages(virt_to_page(tsk->stack), THREAD_SIZE_ORDER);
^~~~~~~~~~~~~~~~~
THREAD_SIZE
Because THREAD_SIZE_ORDER isn't defined at all for 64K pages
#ifdef CONFIG_ARM64_4K_PAGES
#define THREAD_SIZE_ORDER 2
#elif defined(CONFIG_ARM64_16K_PAGES)
#define THREAD_SIZE_ORDER 0
#endif
I think this should just be dead code on arm64 but the asymmetric
#ifdef looks fishy to me.
Thanks,
Laura
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
@ 2017-07-12 20:12 ` Laura Abbott
0 siblings, 0 replies; 44+ messages in thread
From: Laura Abbott @ 2017-07-12 20:12 UTC (permalink / raw)
To: Ard Biesheuvel, linux-arm-kernel, kernel-hardening
Cc: mark.rutland, labbott, will.deacon, dave.martin, catalin.marinas,
Andy Lutomirski
On 07/12/2017 07:44 AM, Ard Biesheuvel wrote:
> This is a fairly quick'n'dirty attempt at enabling virtually mapped
> stacks for arm64, after learning from Mark yesterday that progress
> had more or less stalled on that front.
>
> Since having actual patches to poke at is infinitely better than having
> abstract discussions about how to approach it, I threw some code together
> that:
> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
> the kernel; flames welcome :-) (#7)
> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
> correctly as a platform register (#2, #3, #4, #5, #6)
> 3. dump the entire task stack if regs->sp points elsewhere (#8)
> 4. add the code that checks for a stack overflow and dumps the task context
> before panic()king (#9, #10)
>
> (#1 is an unrelated cleanup patch for copy_page())
>
> So instead of unconditionally switching between stacks, this code simply uses
> tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
> we have a couple of general purpose registers to play with.
>
> Tested with the following hunk applied
>
> --- a/arch/arm64/crypto/aes-cipher-core.S
> +++ b/arch/arm64/crypto/aes-cipher-core.S
> @@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
>
> .align 5
> ENTRY(__aes_arm64_encrypt)
> + stp x29, x30, [sp, #-400]!
> + mov x29, sp
> +
> + bl __aes_arm64_encrypt
> +
> do_crypt fround, crypto_ft_tab, crypto_fl_tab
> ENDPROC(__aes_arm64_encrypt)
>
> which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
> and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
> missing: I suppose it should be possible to display something meaningful if x29
> still points to a valid location, but I haven't looked into it yet.
>
> BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
> Internal error: Oops: 96000047 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
> Hardware name: linux,dummy-virt (DT)
> task: ffff80007c031a00 task.stack: ffff000009bdc000
> PC is at __aes_arm64_encrypt+0x0/0x440
> LR is at __aes_arm64_encrypt+0xc/0x440
> pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
> sp : ffff000009bdc120
> x29: ffff000009bdc120 x28: ffff80007c181c00
> x27: 0000000000000000 x26: 0000000000000100
> x25: ffff0000089a52e8 x24: ffff000009bdfd10
> x23: 0000000000000001 x22: ffff0000089a5408
> x21: 0000000000000001 x20: ffff80007c181c08
> x19: ffff80007c220000 x18: ffff80007c031a00
> x17: 00000000002f0000 x16: ffff80007c181d24
> x15: ffff0000089b2a68 x14: 00000000000003be
> x13: 0000000000000071 x12: 00000000bf5fe8a9
> x11: 0000000000000028 x10: 000000000000002c
> x9 : ffff80007c181d20 x8 : 000000000000001b
> x7 : 0000000046d4609c x6 : ffff0000080c5fd8
> x5 : 0000000000000043 x4 : 0000000046d47b87
> x3 : 000000000000000a x2 : ffff80007c220000
> x1 : ffff80007c220000 x0 : ffff80007c181c80
> Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
> Stack: (0xffff000009bdc120 to 0xffff000009be0000)
> c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
> c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> <snipped ...>
> ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
> ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> Call trace:
> Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
> 0ac0: ffff80007c220000 0001000000000000
> 0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
> 0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
> 0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
> 0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
> 0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
> 0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
> [<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
> Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
> ---[ end trace 2c6304a96ec827cb ]---
>
> Ard Biesheuvel (10):
> arm64/lib: copy_page: use consistent prefetch stride
> arm64/lib: copy_page: avoid x18 register in assembler code
> arm64: crypto: avoid register x18 in scalar AES code
> arm64: kvm: stop treating register x18 as caller save
> arm64: kernel: avoid x18 as an arbitrary temp register
> arm64: kbuild: reserve reg x18 from general allocation by the compiler
> arm64: kernel: switch to register x18 as a task struct pointer
> arm64/kernel: dump entire stack if sp points elsewhere
> arm64: mm: add C level handling for stack overflows
> arm64: kernel: add support for virtually mapped stacks
>
> arch/arm64/Kconfig | 1 +
> arch/arm64/Makefile | 2 +-
> arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
> arch/arm64/include/asm/asm-uaccess.h | 3 +-
> arch/arm64/include/asm/assembler.h | 8 +--
> arch/arm64/include/asm/current.h | 6 +-
> arch/arm64/include/asm/thread_info.h | 2 +
> arch/arm64/kernel/cpu-reset.S | 4 +-
> arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/process.c | 2 +-
> arch/arm64/kernel/traps.c | 9 ++-
> arch/arm64/kvm/hyp/entry.S | 12 ++--
> arch/arm64/lib/Makefile | 3 +-
> arch/arm64/lib/copy_page.S | 47 ++++++++-------
> arch/arm64/mm/fault.c | 24 ++++++++
> 16 files changed, 154 insertions(+), 93 deletions(-)
>
This fails to compile with 64K pages
kernel/fork.c: In function ‘free_thread_stack’:
kernel/fork.c:267:41: error: ‘THREAD_SIZE_ORDER’ undeclared (first use in this function); did you mean ‘THREAD_SIZE’?
__free_pages(virt_to_page(tsk->stack), THREAD_SIZE_ORDER);
^~~~~~~~~~~~~~~~~
THREAD_SIZE
Because THREAD_SIZE_ORDER isn't defined at all for 64K pages
#ifdef CONFIG_ARM64_4K_PAGES
#define THREAD_SIZE_ORDER 2
#elif defined(CONFIG_ARM64_16K_PAGES)
#define THREAD_SIZE_ORDER 0
#endif
I think this should just be dead code on arm64 but the asymmetric
#ifdef looks fishy to me.
Thanks,
Laura
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
2017-07-12 20:12 ` [kernel-hardening] " Laura Abbott
@ 2017-07-12 20:49 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 20:49 UTC (permalink / raw)
To: linux-arm-kernel
On 12 July 2017 at 21:12, Laura Abbott <labbott@redhat.com> wrote:
> On 07/12/2017 07:44 AM, Ard Biesheuvel wrote:
>> This is a fairly quick'n'dirty attempt at enabling virtually mapped
>> stacks for arm64, after learning from Mark yesterday that progress
>> had more or less stalled on that front.
>>
>> Since having actual patches to poke at is infinitely better than having
>> abstract discussions about how to approach it, I threw some code together
>> that:
>> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
>> the kernel; flames welcome :-) (#7)
>> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
>> correctly as a platform register (#2, #3, #4, #5, #6)
>> 3. dump the entire task stack if regs->sp points elsewhere (#8)
>> 4. add the code that checks for a stack overflow and dumps the task context
>> before panic()king (#9, #10)
>>
>> (#1 is an unrelated cleanup patch for copy_page())
>>
>> So instead of unconditionally switching between stacks, this code simply uses
>> tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
>> we have a couple of general purpose registers to play with.
>>
>> Tested with the following hunk applied
>>
>> --- a/arch/arm64/crypto/aes-cipher-core.S
>> +++ b/arch/arm64/crypto/aes-cipher-core.S
>> @@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
>>
>> .align 5
>> ENTRY(__aes_arm64_encrypt)
>> + stp x29, x30, [sp, #-400]!
>> + mov x29, sp
>> +
>> + bl __aes_arm64_encrypt
>> +
>> do_crypt fround, crypto_ft_tab, crypto_fl_tab
>> ENDPROC(__aes_arm64_encrypt)
>>
>> which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
>> and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
>> missing: I suppose it should be possible to display something meaningful if x29
>> still points to a valid location, but I haven't looked into it yet.
>>
>> BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
>> Internal error: Oops: 96000047 [#1] PREEMPT SMP
>> Modules linked in:
>> CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
>> Hardware name: linux,dummy-virt (DT)
>> task: ffff80007c031a00 task.stack: ffff000009bdc000
>> PC is at __aes_arm64_encrypt+0x0/0x440
>> LR is at __aes_arm64_encrypt+0xc/0x440
>> pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
>> sp : ffff000009bdc120
>> x29: ffff000009bdc120 x28: ffff80007c181c00
>> x27: 0000000000000000 x26: 0000000000000100
>> x25: ffff0000089a52e8 x24: ffff000009bdfd10
>> x23: 0000000000000001 x22: ffff0000089a5408
>> x21: 0000000000000001 x20: ffff80007c181c08
>> x19: ffff80007c220000 x18: ffff80007c031a00
>> x17: 00000000002f0000 x16: ffff80007c181d24
>> x15: ffff0000089b2a68 x14: 00000000000003be
>> x13: 0000000000000071 x12: 00000000bf5fe8a9
>> x11: 0000000000000028 x10: 000000000000002c
>> x9 : ffff80007c181d20 x8 : 000000000000001b
>> x7 : 0000000046d4609c x6 : ffff0000080c5fd8
>> x5 : 0000000000000043 x4 : 0000000046d47b87
>> x3 : 000000000000000a x2 : ffff80007c220000
>> x1 : ffff80007c220000 x0 : ffff80007c181c80
>> Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
>> Stack: (0xffff000009bdc120 to 0xffff000009be0000)
>> c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
>> c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> <snipped ...>
>> ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
>> ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> Call trace:
>> Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
>> 0ac0: ffff80007c220000 0001000000000000
>> 0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
>> 0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> 0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> 0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> 0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
>> 0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
>> 0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
>> 0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
>> 0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
>> [<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
>> Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
>> ---[ end trace 2c6304a96ec827cb ]---
>>
>> Ard Biesheuvel (10):
>> arm64/lib: copy_page: use consistent prefetch stride
>> arm64/lib: copy_page: avoid x18 register in assembler code
>> arm64: crypto: avoid register x18 in scalar AES code
>> arm64: kvm: stop treating register x18 as caller save
>> arm64: kernel: avoid x18 as an arbitrary temp register
>> arm64: kbuild: reserve reg x18 from general allocation by the compiler
>> arm64: kernel: switch to register x18 as a task struct pointer
>> arm64/kernel: dump entire stack if sp points elsewhere
>> arm64: mm: add C level handling for stack overflows
>> arm64: kernel: add support for virtually mapped stacks
>>
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/Makefile | 2 +-
>> arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
>> arch/arm64/include/asm/asm-uaccess.h | 3 +-
>> arch/arm64/include/asm/assembler.h | 8 +--
>> arch/arm64/include/asm/current.h | 6 +-
>> arch/arm64/include/asm/thread_info.h | 2 +
>> arch/arm64/kernel/cpu-reset.S | 4 +-
>> arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
>> arch/arm64/kernel/head.S | 6 +-
>> arch/arm64/kernel/process.c | 2 +-
>> arch/arm64/kernel/traps.c | 9 ++-
>> arch/arm64/kvm/hyp/entry.S | 12 ++--
>> arch/arm64/lib/Makefile | 3 +-
>> arch/arm64/lib/copy_page.S | 47 ++++++++-------
>> arch/arm64/mm/fault.c | 24 ++++++++
>> 16 files changed, 154 insertions(+), 93 deletions(-)
>>
>
> This fails to compile with 64K pages
>
> kernel/fork.c: In function ?free_thread_stack?:
> kernel/fork.c:267:41: error: ?THREAD_SIZE_ORDER? undeclared (first use in this function); did you mean ?THREAD_SIZE??
> __free_pages(virt_to_page(tsk->stack), THREAD_SIZE_ORDER);
> ^~~~~~~~~~~~~~~~~
> THREAD_SIZE
>
> Because THREAD_SIZE_ORDER isn't defined at all for 64K pages
>
> #ifdef CONFIG_ARM64_4K_PAGES
> #define THREAD_SIZE_ORDER 2
> #elif defined(CONFIG_ARM64_16K_PAGES)
> #define THREAD_SIZE_ORDER 0
> #endif
>
> I think this should just be dead code on arm64 but the asymmetric
> #ifdef looks fishy to me.
>
I guess this is because 16 KB cannot be expressed as PAGE_SIZE <<
THREAD_SIZE_ORDER for sane values of the latter.
For vmap stack, we should just switch to THREAD_SIZE_ORDER == 0 and
hence THREAD_SIZE == PAGE_SIZE
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
@ 2017-07-12 20:49 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-12 20:49 UTC (permalink / raw)
To: Laura Abbott
Cc: linux-arm-kernel, Kernel Hardening, Mark Rutland, Laura Abbott,
Will Deacon, Dave Martin, Catalin Marinas, Andy Lutomirski
On 12 July 2017 at 21:12, Laura Abbott <labbott@redhat.com> wrote:
> On 07/12/2017 07:44 AM, Ard Biesheuvel wrote:
>> This is a fairly quick'n'dirty attempt at enabling virtually mapped
>> stacks for arm64, after learning from Mark yesterday that progress
>> had more or less stalled on that front.
>>
>> Since having actual patches to poke at is infinitely better than having
>> abstract discussions about how to approach it, I threw some code together
>> that:
>> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
>> the kernel; flames welcome :-) (#7)
>> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
>> correctly as a platform register (#2, #3, #4, #5, #6)
>> 3. dump the entire task stack if regs->sp points elsewhere (#8)
>> 4. add the code that checks for a stack overflow and dumps the task context
>> before panic()king (#9, #10)
>>
>> (#1 is an unrelated cleanup patch for copy_page())
>>
>> So instead of unconditionally switching between stacks, this code simply uses
>> tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
>> we have a couple of general purpose registers to play with.
>>
>> Tested with the following hunk applied
>>
>> --- a/arch/arm64/crypto/aes-cipher-core.S
>> +++ b/arch/arm64/crypto/aes-cipher-core.S
>> @@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
>>
>> .align 5
>> ENTRY(__aes_arm64_encrypt)
>> + stp x29, x30, [sp, #-400]!
>> + mov x29, sp
>> +
>> + bl __aes_arm64_encrypt
>> +
>> do_crypt fround, crypto_ft_tab, crypto_fl_tab
>> ENDPROC(__aes_arm64_encrypt)
>>
>> which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
>> and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
>> missing: I suppose it should be possible to display something meaningful if x29
>> still points to a valid location, but I haven't looked into it yet.
>>
>> BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
>> Internal error: Oops: 96000047 [#1] PREEMPT SMP
>> Modules linked in:
>> CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
>> Hardware name: linux,dummy-virt (DT)
>> task: ffff80007c031a00 task.stack: ffff000009bdc000
>> PC is at __aes_arm64_encrypt+0x0/0x440
>> LR is at __aes_arm64_encrypt+0xc/0x440
>> pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
>> sp : ffff000009bdc120
>> x29: ffff000009bdc120 x28: ffff80007c181c00
>> x27: 0000000000000000 x26: 0000000000000100
>> x25: ffff0000089a52e8 x24: ffff000009bdfd10
>> x23: 0000000000000001 x22: ffff0000089a5408
>> x21: 0000000000000001 x20: ffff80007c181c08
>> x19: ffff80007c220000 x18: ffff80007c031a00
>> x17: 00000000002f0000 x16: ffff80007c181d24
>> x15: ffff0000089b2a68 x14: 00000000000003be
>> x13: 0000000000000071 x12: 00000000bf5fe8a9
>> x11: 0000000000000028 x10: 000000000000002c
>> x9 : ffff80007c181d20 x8 : 000000000000001b
>> x7 : 0000000046d4609c x6 : ffff0000080c5fd8
>> x5 : 0000000000000043 x4 : 0000000046d47b87
>> x3 : 000000000000000a x2 : ffff80007c220000
>> x1 : ffff80007c220000 x0 : ffff80007c181c80
>> Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
>> Stack: (0xffff000009bdc120 to 0xffff000009be0000)
>> c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
>> c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> <snipped ...>
>> ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
>> ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> Call trace:
>> Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
>> 0ac0: ffff80007c220000 0001000000000000
>> 0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
>> 0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> 0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> 0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> 0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
>> 0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
>> 0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
>> 0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
>> 0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
>> [<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
>> Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
>> ---[ end trace 2c6304a96ec827cb ]---
>>
>> Ard Biesheuvel (10):
>> arm64/lib: copy_page: use consistent prefetch stride
>> arm64/lib: copy_page: avoid x18 register in assembler code
>> arm64: crypto: avoid register x18 in scalar AES code
>> arm64: kvm: stop treating register x18 as caller save
>> arm64: kernel: avoid x18 as an arbitrary temp register
>> arm64: kbuild: reserve reg x18 from general allocation by the compiler
>> arm64: kernel: switch to register x18 as a task struct pointer
>> arm64/kernel: dump entire stack if sp points elsewhere
>> arm64: mm: add C level handling for stack overflows
>> arm64: kernel: add support for virtually mapped stacks
>>
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/Makefile | 2 +-
>> arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
>> arch/arm64/include/asm/asm-uaccess.h | 3 +-
>> arch/arm64/include/asm/assembler.h | 8 +--
>> arch/arm64/include/asm/current.h | 6 +-
>> arch/arm64/include/asm/thread_info.h | 2 +
>> arch/arm64/kernel/cpu-reset.S | 4 +-
>> arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
>> arch/arm64/kernel/head.S | 6 +-
>> arch/arm64/kernel/process.c | 2 +-
>> arch/arm64/kernel/traps.c | 9 ++-
>> arch/arm64/kvm/hyp/entry.S | 12 ++--
>> arch/arm64/lib/Makefile | 3 +-
>> arch/arm64/lib/copy_page.S | 47 ++++++++-------
>> arch/arm64/mm/fault.c | 24 ++++++++
>> 16 files changed, 154 insertions(+), 93 deletions(-)
>>
>
> This fails to compile with 64K pages
>
> kernel/fork.c: In function ‘free_thread_stack’:
> kernel/fork.c:267:41: error: ‘THREAD_SIZE_ORDER’ undeclared (first use in this function); did you mean ‘THREAD_SIZE’?
> __free_pages(virt_to_page(tsk->stack), THREAD_SIZE_ORDER);
> ^~~~~~~~~~~~~~~~~
> THREAD_SIZE
>
> Because THREAD_SIZE_ORDER isn't defined at all for 64K pages
>
> #ifdef CONFIG_ARM64_4K_PAGES
> #define THREAD_SIZE_ORDER 2
> #elif defined(CONFIG_ARM64_16K_PAGES)
> #define THREAD_SIZE_ORDER 0
> #endif
>
> I think this should just be dead code on arm64 but the asymmetric
> #ifdef looks fishy to me.
>
I guess this is because 16 KB cannot be expressed as PAGE_SIZE <<
THREAD_SIZE_ORDER for sane values of the latter.
For vmap stack, we should just switch to THREAD_SIZE_ORDER == 0 and
hence THREAD_SIZE == PAGE_SIZE
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
2017-07-12 20:49 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 21:32 ` Andy Lutomirski
-1 siblings, 0 replies; 44+ messages in thread
From: Andy Lutomirski @ 2017-07-12 21:32 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 12, 2017 at 1:49 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 12 July 2017 at 21:12, Laura Abbott <labbott@redhat.com> wrote:
>> On 07/12/2017 07:44 AM, Ard Biesheuvel wrote:
>>> This is a fairly quick'n'dirty attempt at enabling virtually mapped
>>> stacks for arm64, after learning from Mark yesterday that progress
>>> had more or less stalled on that front.
>>>
>>> Since having actual patches to poke at is infinitely better than having
>>> abstract discussions about how to approach it, I threw some code together
>>> that:
>>> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
>>> the kernel; flames welcome :-) (#7)
>>> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
>>> correctly as a platform register (#2, #3, #4, #5, #6)
>>> 3. dump the entire task stack if regs->sp points elsewhere (#8)
>>> 4. add the code that checks for a stack overflow and dumps the task context
>>> before panic()king (#9, #10)
>>>
>>> (#1 is an unrelated cleanup patch for copy_page())
>>>
>>> So instead of unconditionally switching between stacks, this code simply uses
>>> tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
>>> we have a couple of general purpose registers to play with.
>>>
>>> Tested with the following hunk applied
>>>
>>> --- a/arch/arm64/crypto/aes-cipher-core.S
>>> +++ b/arch/arm64/crypto/aes-cipher-core.S
>>> @@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
>>>
>>> .align 5
>>> ENTRY(__aes_arm64_encrypt)
>>> + stp x29, x30, [sp, #-400]!
>>> + mov x29, sp
>>> +
>>> + bl __aes_arm64_encrypt
>>> +
>>> do_crypt fround, crypto_ft_tab, crypto_fl_tab
>>> ENDPROC(__aes_arm64_encrypt)
>>>
>>> which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
>>> and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
>>> missing: I suppose it should be possible to display something meaningful if x29
>>> still points to a valid location, but I haven't looked into it yet.
>>>
>>> BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
>>> Internal error: Oops: 96000047 [#1] PREEMPT SMP
>>> Modules linked in:
>>> CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
>>> Hardware name: linux,dummy-virt (DT)
>>> task: ffff80007c031a00 task.stack: ffff000009bdc000
>>> PC is at __aes_arm64_encrypt+0x0/0x440
>>> LR is at __aes_arm64_encrypt+0xc/0x440
>>> pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
>>> sp : ffff000009bdc120
>>> x29: ffff000009bdc120 x28: ffff80007c181c00
>>> x27: 0000000000000000 x26: 0000000000000100
>>> x25: ffff0000089a52e8 x24: ffff000009bdfd10
>>> x23: 0000000000000001 x22: ffff0000089a5408
>>> x21: 0000000000000001 x20: ffff80007c181c08
>>> x19: ffff80007c220000 x18: ffff80007c031a00
>>> x17: 00000000002f0000 x16: ffff80007c181d24
>>> x15: ffff0000089b2a68 x14: 00000000000003be
>>> x13: 0000000000000071 x12: 00000000bf5fe8a9
>>> x11: 0000000000000028 x10: 000000000000002c
>>> x9 : ffff80007c181d20 x8 : 000000000000001b
>>> x7 : 0000000046d4609c x6 : ffff0000080c5fd8
>>> x5 : 0000000000000043 x4 : 0000000046d47b87
>>> x3 : 000000000000000a x2 : ffff80007c220000
>>> x1 : ffff80007c220000 x0 : ffff80007c181c80
>>> Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
>>> Stack: (0xffff000009bdc120 to 0xffff000009be0000)
>>> c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
>>> c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> <snipped ...>
>>> ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
>>> ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> Call trace:
>>> Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
>>> 0ac0: ffff80007c220000 0001000000000000
>>> 0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
>>> 0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> 0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> 0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> 0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
>>> 0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
>>> 0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
>>> 0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
>>> 0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
>>> [<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
>>> Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
>>> ---[ end trace 2c6304a96ec827cb ]---
>>>
>>> Ard Biesheuvel (10):
>>> arm64/lib: copy_page: use consistent prefetch stride
>>> arm64/lib: copy_page: avoid x18 register in assembler code
>>> arm64: crypto: avoid register x18 in scalar AES code
>>> arm64: kvm: stop treating register x18 as caller save
>>> arm64: kernel: avoid x18 as an arbitrary temp register
>>> arm64: kbuild: reserve reg x18 from general allocation by the compiler
>>> arm64: kernel: switch to register x18 as a task struct pointer
>>> arm64/kernel: dump entire stack if sp points elsewhere
>>> arm64: mm: add C level handling for stack overflows
>>> arm64: kernel: add support for virtually mapped stacks
>>>
>>> arch/arm64/Kconfig | 1 +
>>> arch/arm64/Makefile | 2 +-
>>> arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
>>> arch/arm64/include/asm/asm-uaccess.h | 3 +-
>>> arch/arm64/include/asm/assembler.h | 8 +--
>>> arch/arm64/include/asm/current.h | 6 +-
>>> arch/arm64/include/asm/thread_info.h | 2 +
>>> arch/arm64/kernel/cpu-reset.S | 4 +-
>>> arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
>>> arch/arm64/kernel/head.S | 6 +-
>>> arch/arm64/kernel/process.c | 2 +-
>>> arch/arm64/kernel/traps.c | 9 ++-
>>> arch/arm64/kvm/hyp/entry.S | 12 ++--
>>> arch/arm64/lib/Makefile | 3 +-
>>> arch/arm64/lib/copy_page.S | 47 ++++++++-------
>>> arch/arm64/mm/fault.c | 24 ++++++++
>>> 16 files changed, 154 insertions(+), 93 deletions(-)
>>>
>>
>> This fails to compile with 64K pages
>>
>> kernel/fork.c: In function ?free_thread_stack?:
>> kernel/fork.c:267:41: error: ?THREAD_SIZE_ORDER? undeclared (first use in this function); did you mean ?THREAD_SIZE??
>> __free_pages(virt_to_page(tsk->stack), THREAD_SIZE_ORDER);
>> ^~~~~~~~~~~~~~~~~
>> THREAD_SIZE
>>
>> Because THREAD_SIZE_ORDER isn't defined at all for 64K pages
>>
>> #ifdef CONFIG_ARM64_4K_PAGES
>> #define THREAD_SIZE_ORDER 2
>> #elif defined(CONFIG_ARM64_16K_PAGES)
>> #define THREAD_SIZE_ORDER 0
>> #endif
>>
>> I think this should just be dead code on arm64 but the asymmetric
>> #ifdef looks fishy to me.
>>
>
> I guess this is because 16 KB cannot be expressed as PAGE_SIZE <<
> THREAD_SIZE_ORDER for sane values of the latter.
>
> For vmap stack, we should just switch to THREAD_SIZE_ORDER == 0 and
> hence THREAD_SIZE == PAGE_SIZE
Agreed. Using 16k virtually mapped stacks with a 64k page size seems pointless.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
@ 2017-07-12 21:32 ` Andy Lutomirski
0 siblings, 0 replies; 44+ messages in thread
From: Andy Lutomirski @ 2017-07-12 21:32 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Laura Abbott, linux-arm-kernel, Kernel Hardening, Mark Rutland,
Laura Abbott, Will Deacon, Dave Martin, Catalin Marinas,
Andy Lutomirski
On Wed, Jul 12, 2017 at 1:49 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 12 July 2017 at 21:12, Laura Abbott <labbott@redhat.com> wrote:
>> On 07/12/2017 07:44 AM, Ard Biesheuvel wrote:
>>> This is a fairly quick'n'dirty attempt at enabling virtually mapped
>>> stacks for arm64, after learning from Mark yesterday that progress
>>> had more or less stalled on that front.
>>>
>>> Since having actual patches to poke at is infinitely better than having
>>> abstract discussions about how to approach it, I threw some code together
>>> that:
>>> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
>>> the kernel; flames welcome :-) (#7)
>>> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
>>> correctly as a platform register (#2, #3, #4, #5, #6)
>>> 3. dump the entire task stack if regs->sp points elsewhere (#8)
>>> 4. add the code that checks for a stack overflow and dumps the task context
>>> before panic()king (#9, #10)
>>>
>>> (#1 is an unrelated cleanup patch for copy_page())
>>>
>>> So instead of unconditionally switching between stacks, this code simply uses
>>> tpidrro_el0 and sp_el0 as scratch registers in the entry-from-el1 code so that
>>> we have a couple of general purpose registers to play with.
>>>
>>> Tested with the following hunk applied
>>>
>>> --- a/arch/arm64/crypto/aes-cipher-core.S
>>> +++ b/arch/arm64/crypto/aes-cipher-core.S
>>> @@ -102,6 +102,11 @@ CPU_BE( rev w7, w7 )
>>>
>>> .align 5
>>> ENTRY(__aes_arm64_encrypt)
>>> + stp x29, x30, [sp, #-400]!
>>> + mov x29, sp
>>> +
>>> + bl __aes_arm64_encrypt
>>> +
>>> do_crypt fround, crypto_ft_tab, crypto_fl_tab
>>> ENDPROC(__aes_arm64_encrypt)
>>>
>>> which produces the output below at boot (provided that CONFIG_CRYPTO_AES_ARM64=y
>>> and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set). Note that the call stack is
>>> missing: I suppose it should be possible to display something meaningful if x29
>>> still points to a valid location, but I haven't looked into it yet.
>>>
>>> BUG: stack guard page was hit at ffff000009bdbf90 (stack is ffff000009bdc000..ffff000009bdffff)
>>> Internal error: Oops: 96000047 [#1] PREEMPT SMP
>>> Modules linked in:
>>> CPU: 0 PID: 613 Comm: cryptomgr_test Not tainted 4.12.0-rc4-00119-g1fb2159e248e-dirty #520
>>> Hardware name: linux,dummy-virt (DT)
>>> task: ffff80007c031a00 task.stack: ffff000009bdc000
>>> PC is at __aes_arm64_encrypt+0x0/0x440
>>> LR is at __aes_arm64_encrypt+0xc/0x440
>>> pc : [<ffff0000080c5760>] lr : [<ffff0000080c576c>] pstate: 80000145
>>> sp : ffff000009bdc120
>>> x29: ffff000009bdc120 x28: ffff80007c181c00
>>> x27: 0000000000000000 x26: 0000000000000100
>>> x25: ffff0000089a52e8 x24: ffff000009bdfd10
>>> x23: 0000000000000001 x22: ffff0000089a5408
>>> x21: 0000000000000001 x20: ffff80007c181c08
>>> x19: ffff80007c220000 x18: ffff80007c031a00
>>> x17: 00000000002f0000 x16: ffff80007c181d24
>>> x15: ffff0000089b2a68 x14: 00000000000003be
>>> x13: 0000000000000071 x12: 00000000bf5fe8a9
>>> x11: 0000000000000028 x10: 000000000000002c
>>> x9 : ffff80007c181d20 x8 : 000000000000001b
>>> x7 : 0000000046d4609c x6 : ffff0000080c5fd8
>>> x5 : 0000000000000043 x4 : 0000000046d47b87
>>> x3 : 000000000000000a x2 : ffff80007c220000
>>> x1 : ffff80007c220000 x0 : ffff80007c181c80
>>> Process cryptomgr_test (pid: 613, stack limit = 0xffff000009bdc000)
>>> Stack: (0xffff000009bdc120 to 0xffff000009be0000)
>>> c120: ffff000009bdc2b0 ffff0000080c576c 0000000000000000 0000000000000000
>>> c140: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> <snipped ...>
>>> ffc0: 0000000000000000 0000000000000005 0000000000000000 0000000000000000
>>> ffe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> Call trace:
>>> Exception stack(0xffff80007efd0ad0 to 0xffff80007efd0c00)
>>> 0ac0: ffff80007c220000 0001000000000000
>>> 0ae0: ffff80007efd0ca0 ffff0000080c5760 0000000000000000 0000000000000000
>>> 0b00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> 0b20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> 0b40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> 0b60: 0000000000000000 0000000000000000 ffff80007c181c80 ffff80007c220000
>>> 0b80: ffff80007c220000 000000000000000a 0000000046d47b87 0000000000000043
>>> 0ba0: ffff0000080c5fd8 0000000046d4609c 000000000000001b ffff80007c181d20
>>> 0bc0: 000000000000002c 0000000000000028 00000000bf5fe8a9 0000000000000071
>>> 0be0: 00000000000003be ffff0000089b2a68 ffff80007c181d24 00000000002f0000
>>> [<ffff0000080c5760>] __aes_arm64_encrypt+0x0/0x440
>>> Code: 00000000 00000000 00000000 00000000 (a9a77bfd)
>>> ---[ end trace 2c6304a96ec827cb ]---
>>>
>>> Ard Biesheuvel (10):
>>> arm64/lib: copy_page: use consistent prefetch stride
>>> arm64/lib: copy_page: avoid x18 register in assembler code
>>> arm64: crypto: avoid register x18 in scalar AES code
>>> arm64: kvm: stop treating register x18 as caller save
>>> arm64: kernel: avoid x18 as an arbitrary temp register
>>> arm64: kbuild: reserve reg x18 from general allocation by the compiler
>>> arm64: kernel: switch to register x18 as a task struct pointer
>>> arm64/kernel: dump entire stack if sp points elsewhere
>>> arm64: mm: add C level handling for stack overflows
>>> arm64: kernel: add support for virtually mapped stacks
>>>
>>> arch/arm64/Kconfig | 1 +
>>> arch/arm64/Makefile | 2 +-
>>> arch/arm64/crypto/aes-cipher-core.S | 55 ++++++++---------
>>> arch/arm64/include/asm/asm-uaccess.h | 3 +-
>>> arch/arm64/include/asm/assembler.h | 8 +--
>>> arch/arm64/include/asm/current.h | 6 +-
>>> arch/arm64/include/asm/thread_info.h | 2 +
>>> arch/arm64/kernel/cpu-reset.S | 4 +-
>>> arch/arm64/kernel/entry.S | 63 ++++++++++++++++----
>>> arch/arm64/kernel/head.S | 6 +-
>>> arch/arm64/kernel/process.c | 2 +-
>>> arch/arm64/kernel/traps.c | 9 ++-
>>> arch/arm64/kvm/hyp/entry.S | 12 ++--
>>> arch/arm64/lib/Makefile | 3 +-
>>> arch/arm64/lib/copy_page.S | 47 ++++++++-------
>>> arch/arm64/mm/fault.c | 24 ++++++++
>>> 16 files changed, 154 insertions(+), 93 deletions(-)
>>>
>>
>> This fails to compile with 64K pages
>>
>> kernel/fork.c: In function ‘free_thread_stack’:
>> kernel/fork.c:267:41: error: ‘THREAD_SIZE_ORDER’ undeclared (first use in this function); did you mean ‘THREAD_SIZE’?
>> __free_pages(virt_to_page(tsk->stack), THREAD_SIZE_ORDER);
>> ^~~~~~~~~~~~~~~~~
>> THREAD_SIZE
>>
>> Because THREAD_SIZE_ORDER isn't defined at all for 64K pages
>>
>> #ifdef CONFIG_ARM64_4K_PAGES
>> #define THREAD_SIZE_ORDER 2
>> #elif defined(CONFIG_ARM64_16K_PAGES)
>> #define THREAD_SIZE_ORDER 0
>> #endif
>>
>> I think this should just be dead code on arm64 but the asymmetric
>> #ifdef looks fishy to me.
>>
>
> I guess this is because 16 KB cannot be expressed as PAGE_SIZE <<
> THREAD_SIZE_ORDER for sane values of the latter.
>
> For vmap stack, we should just switch to THREAD_SIZE_ORDER == 0 and
> hence THREAD_SIZE == PAGE_SIZE
Agreed. Using 16k virtually mapped stacks with a 64k page size seems pointless.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 22:47 ` Mark Rutland
-1 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2017-07-12 22:47 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On Wed, Jul 12, 2017 at 03:44:13PM +0100, Ard Biesheuvel wrote:
> This is a fairly quick'n'dirty attempt at enabling virtually mapped
> stacks for arm64, after learning from Mark yesterday that progress
> had more or less stalled on that front.
Thanks for putting this together. If nothing else, this work needed a
good kick.
I had some rought comments on this, but in the process of wiring those
up, I ended up writing an alternative [1] by cobblnig together prior
attempts. Apologies for the NIH.
> Since having actual patches to poke at is infinitely better than having
> abstract discussions about how to approach it, I threw some code together
> that:
> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
> the kernel; flames welcome :-) (#7)
> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
> correctly as a platform register (#2, #3, #4, #5, #6)
>From past experience [2], I know that Will is not a fan of reserving a
GPR like this. There are a couple of other ways we can free up SP_EL0,
though, so that isn't the end of the world.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/518434.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/this-cpu-reg
> 3. dump the entire task stack if regs->sp points elsewhere (#8)
This sounds useful, but it's liable to fill a scrollbuffer and/or take a
while to dump (especially with 64K stacks), so we might want a boot-time
option to turn that on/off.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
@ 2017-07-12 22:47 ` Mark Rutland
0 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2017-07-12 22:47 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, kernel-hardening, labbott, will.deacon,
dave.martin, catalin.marinas
Hi Ard,
On Wed, Jul 12, 2017 at 03:44:13PM +0100, Ard Biesheuvel wrote:
> This is a fairly quick'n'dirty attempt at enabling virtually mapped
> stacks for arm64, after learning from Mark yesterday that progress
> had more or less stalled on that front.
Thanks for putting this together. If nothing else, this work needed a
good kick.
I had some rought comments on this, but in the process of wiring those
up, I ended up writing an alternative [1] by cobblnig together prior
attempts. Apologies for the NIH.
> Since having actual patches to poke at is infinitely better than having
> abstract discussions about how to approach it, I threw some code together
> that:
> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
> the kernel; flames welcome :-) (#7)
> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
> correctly as a platform register (#2, #3, #4, #5, #6)
>From past experience [2], I know that Will is not a fan of reserving a
GPR like this. There are a couple of other ways we can free up SP_EL0,
though, so that isn't the end of the world.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/518434.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/this-cpu-reg
> 3. dump the entire task stack if regs->sp points elsewhere (#8)
This sounds useful, but it's liable to fill a scrollbuffer and/or take a
while to dump (especially with 64K stacks), so we might want a boot-time
option to turn that on/off.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-12 22:59 ` Mark Rutland
-1 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2017-07-12 22:59 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 12, 2017 at 03:44:23PM +0100, Ard Biesheuvel wrote:
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 2ba3185b1c78..4c3e82d6e2f2 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
> */
> .align 6
> el1_sync:
> +#ifdef CONFIG_VMAP_STACK
> + /*
> + * When taking an exception from EL1, we need to check whether it is
> + * caused by a faulting out-of-bounds access to the virtually mapped
> + * stack before we can attempt to preserve the interrupted context.
> + */
> + msr tpidrro_el0, x0 // stash x0
> + mrs x0, far_el1 // get faulting address
> + tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
One thing I don't like about this is that we're reading FAR_EL1 even
though we don't know whether it's valid. It could contain junk, and we
could spuriously jump into the slow path. That's liable to make
performance erratic.
Given we have to stash a GPR anyway, we can check whether the SP is
valid in the fast-path, and that should be the only check we require. I
think we can also get rid of the overflow heuristic by checking that the
SP is strictly in the THREAD_SIZE stack area.
[...]
> @@ -234,6 +239,10 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
> */
> if (addr >= (u64)current->stack - PAGE_SIZE &&
> addr < (u64)current->stack) {
> +
> + /* fix up regs->sp, we stashed the faulting value in sp_el0 */
> + regs->sp = read_sysreg(sp_el0);
> +
This also seems a bit "magic", as we're relying on what the entry asm
will have done in a very specific case.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
@ 2017-07-12 22:59 ` Mark Rutland
0 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2017-07-12 22:59 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, kernel-hardening, labbott, will.deacon,
dave.martin, catalin.marinas
On Wed, Jul 12, 2017 at 03:44:23PM +0100, Ard Biesheuvel wrote:
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 2ba3185b1c78..4c3e82d6e2f2 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
> */
> .align 6
> el1_sync:
> +#ifdef CONFIG_VMAP_STACK
> + /*
> + * When taking an exception from EL1, we need to check whether it is
> + * caused by a faulting out-of-bounds access to the virtually mapped
> + * stack before we can attempt to preserve the interrupted context.
> + */
> + msr tpidrro_el0, x0 // stash x0
> + mrs x0, far_el1 // get faulting address
> + tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
One thing I don't like about this is that we're reading FAR_EL1 even
though we don't know whether it's valid. It could contain junk, and we
could spuriously jump into the slow path. That's liable to make
performance erratic.
Given we have to stash a GPR anyway, we can check whether the SP is
valid in the fast-path, and that should be the only check we require. I
think we can also get rid of the overflow heuristic by checking that the
SP is strictly in the THREAD_SIZE stack area.
[...]
> @@ -234,6 +239,10 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
> */
> if (addr >= (u64)current->stack - PAGE_SIZE &&
> addr < (u64)current->stack) {
> +
> + /* fix up regs->sp, we stashed the faulting value in sp_el0 */
> + regs->sp = read_sysreg(sp_el0);
> +
This also seems a bit "magic", as we're relying on what the entry asm
will have done in a very specific case.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
2017-07-12 22:47 ` [kernel-hardening] " Mark Rutland
@ 2017-07-13 6:51 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-13 6:51 UTC (permalink / raw)
To: linux-arm-kernel
On 12 July 2017 at 23:47, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Ard,
>
> On Wed, Jul 12, 2017 at 03:44:13PM +0100, Ard Biesheuvel wrote:
>> This is a fairly quick'n'dirty attempt at enabling virtually mapped
>> stacks for arm64, after learning from Mark yesterday that progress
>> had more or less stalled on that front.
>
> Thanks for putting this together. If nothing else, this work needed a
> good kick.
>
> I had some rought comments on this, but in the process of wiring those
> up, I ended up writing an alternative [1] by cobblnig together prior
> attempts. Apologies for the NIH.
>
No worries. I deliberately refrained from any polishing of the code
because I was expecting debate not acks.
>> Since having actual patches to poke at is infinitely better than having
>> abstract discussions about how to approach it, I threw some code together
>> that:
>> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
>> the kernel; flames welcome :-) (#7)
>> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
>> correctly as a platform register (#2, #3, #4, #5, #6)
>
> From past experience [2], I know that Will is not a fan of reserving a
> GPR like this. There are a couple of other ways we can free up SP_EL0,
> though, so that isn't the end of the world.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/518434.html
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/this-cpu-reg
>
>> 3. dump the entire task stack if regs->sp points elsewhere (#8)
>
> This sounds useful, but it's liable to fill a scrollbuffer and/or take a
> while to dump (especially with 64K stacks), so we might want a boot-time
> option to turn that on/off.
>
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled
@ 2017-07-13 6:51 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-13 6:51 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel, Kernel Hardening, Laura Abbott, Will Deacon,
Dave Martin, Catalin Marinas
On 12 July 2017 at 23:47, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Ard,
>
> On Wed, Jul 12, 2017 at 03:44:13PM +0100, Ard Biesheuvel wrote:
>> This is a fairly quick'n'dirty attempt at enabling virtually mapped
>> stacks for arm64, after learning from Mark yesterday that progress
>> had more or less stalled on that front.
>
> Thanks for putting this together. If nothing else, this work needed a
> good kick.
>
> I had some rought comments on this, but in the process of wiring those
> up, I ended up writing an alternative [1] by cobblnig together prior
> attempts. Apologies for the NIH.
>
No worries. I deliberately refrained from any polishing of the code
because I was expecting debate not acks.
>> Since having actual patches to poke at is infinitely better than having
>> abstract discussions about how to approach it, I threw some code together
>> that:
>> 1. frees up sp_el0 by promoting register x18 to 'current' pointer while in
>> the kernel; flames welcome :-) (#7)
>> 2. preparatory work to allow 1., i.e., to free up x18 and preserve/restore it
>> correctly as a platform register (#2, #3, #4, #5, #6)
>
> From past experience [2], I know that Will is not a fan of reserving a
> GPR like this. There are a couple of other ways we can free up SP_EL0,
> though, so that isn't the end of the world.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/518434.html
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/this-cpu-reg
>
>> 3. dump the entire task stack if regs->sp points elsewhere (#8)
>
> This sounds useful, but it's liable to fill a scrollbuffer and/or take a
> while to dump (especially with 64K stacks), so we might want a boot-time
> option to turn that on/off.
>
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
2017-07-12 22:59 ` [kernel-hardening] " Mark Rutland
@ 2017-07-13 9:12 ` Mark Rutland
-1 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2017-07-13 9:12 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 12, 2017 at 11:59:38PM +0100, Mark Rutland wrote:
> On Wed, Jul 12, 2017 at 03:44:23PM +0100, Ard Biesheuvel wrote:
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index 2ba3185b1c78..4c3e82d6e2f2 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
> > */
> > .align 6
> > el1_sync:
> > +#ifdef CONFIG_VMAP_STACK
> > + /*
> > + * When taking an exception from EL1, we need to check whether it is
> > + * caused by a faulting out-of-bounds access to the virtually mapped
> > + * stack before we can attempt to preserve the interrupted context.
> > + */
> > + msr tpidrro_el0, x0 // stash x0
> > + mrs x0, far_el1 // get faulting address
> > + tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
>
> One thing I don't like about this is that we're reading FAR_EL1 even
> though we don't know whether it's valid. It could contain junk, and we
> could spuriously jump into the slow path. That's liable to make
> performance erratic.
Once I sent this, I realised what I said wasn't quite right.
It's true that FAR_EL1 is UNKNOWN in some cases, but today those are all
fatal (e.g. if we take an UNDEFINED exception due to a bad instruction
at EL1).
So my performance concern was spurious; sorry for the noise.
However, so as to not mis-report those fatal cases, we either need to
inspect the SP or ESR_EL1 to determine that FAR_EL1 is valid and/or this
was a stack-overflow
If we follow the approach of this series, we can move those checks into
the check_stack_ovf slow path.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [kernel-hardening] Re: [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
@ 2017-07-13 9:12 ` Mark Rutland
0 siblings, 0 replies; 44+ messages in thread
From: Mark Rutland @ 2017-07-13 9:12 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, kernel-hardening, labbott, will.deacon,
dave.martin, catalin.marinas
On Wed, Jul 12, 2017 at 11:59:38PM +0100, Mark Rutland wrote:
> On Wed, Jul 12, 2017 at 03:44:23PM +0100, Ard Biesheuvel wrote:
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index 2ba3185b1c78..4c3e82d6e2f2 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
> > */
> > .align 6
> > el1_sync:
> > +#ifdef CONFIG_VMAP_STACK
> > + /*
> > + * When taking an exception from EL1, we need to check whether it is
> > + * caused by a faulting out-of-bounds access to the virtually mapped
> > + * stack before we can attempt to preserve the interrupted context.
> > + */
> > + msr tpidrro_el0, x0 // stash x0
> > + mrs x0, far_el1 // get faulting address
> > + tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
>
> One thing I don't like about this is that we're reading FAR_EL1 even
> though we don't know whether it's valid. It could contain junk, and we
> could spuriously jump into the slow path. That's liable to make
> performance erratic.
Once I sent this, I realised what I said wasn't quite right.
It's true that FAR_EL1 is UNKNOWN in some cases, but today those are all
fatal (e.g. if we take an UNDEFINED exception due to a bad instruction
at EL1).
So my performance concern was spurious; sorry for the noise.
However, so as to not mis-report those fatal cases, we either need to
inspect the SP or ESR_EL1 to determine that FAR_EL1 is valid and/or this
was a stack-overflow
If we follow the approach of this series, we can move those checks into
the check_stack_ovf slow path.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-13 10:35 ` Dave Martin
-1 siblings, 0 replies; 44+ messages in thread
From: Dave Martin @ 2017-07-13 10:35 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 12, 2017 at 03:44:23PM +0100, Ard Biesheuvel wrote:
> Add code that checks whether an exception taken from EL1 was caused
> by a faulting stack access before proceeding to save the interrupted
> context to the stack.
>
> This involves checking whether the faulting address coincides with the
> guard page below the task stack of 'current'. This uses tpidrro_el0 and
> sp_el0 as scratch registers, so we can free up a couple of general
> purpose registers for use in the code that performs this check. If it
> turns out we are dealing with a stack overflow, switch to a special
> per-CPU overflow stack so we can at least call panic().
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/thread_info.h | 2 +
> arch/arm64/kernel/entry.S | 49 ++++++++++++++++++++
> arch/arm64/mm/fault.c | 9 ++++
> 4 files changed, 61 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b52db8bb1270..50caf63099c8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -73,6 +73,7 @@ config ARM64
> select HAVE_ARCH_SECCOMP_FILTER
> select HAVE_ARCH_TRACEHOOK
> select HAVE_ARCH_TRANSPARENT_HUGEPAGE
> + select HAVE_ARCH_VMAP_STACK
> select HAVE_ARM_SMCCC
> select HAVE_EBPF_JIT
> select HAVE_C_RECORDMCOUNT
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 46c3b93cf865..1c3e0a3bf87a 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -32,6 +32,8 @@
> #define THREAD_SIZE 16384
> #define THREAD_START_SP (THREAD_SIZE - 16)
>
> +#define OVERFLOW_STACK_SIZE 1024
> +
How was this number chosen?
Kernel stack overflow is not going to get routinely tested -- after all
it should only get exercised when things go horribly wrong.
So, if the panic code (or compiler-generated stack frames) become
bloated enough, we will silently overrun this limit.
Maybe it doesn't matter if we trash a load of unrelated percpu data when
we panic.
> #ifndef __ASSEMBLY__
>
> struct task_struct;
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 2ba3185b1c78..4c3e82d6e2f2 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
> */
> .align 6
> el1_sync:
> +#ifdef CONFIG_VMAP_STACK
> + /*
> + * When taking an exception from EL1, we need to check whether it is
> + * caused by a faulting out-of-bounds access to the virtually mapped
> + * stack before we can attempt to preserve the interrupted context.
> + */
> + msr tpidrro_el0, x0 // stash x0
> + mrs x0, far_el1 // get faulting address
> + tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
> +
> +.Lcheck_stack_resume:
> + mrs x0, tpidrro_el0 // restore x0
> +#endif
> +
Unless I'm missing something, this leaves tpidrr0_el0 trashed (and
containing kernel data that userspace can subsequently read).
This could be solved by moving the tpidrro_el0 load out of context
switch and into ret_to_user.
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks
@ 2017-07-13 10:35 ` Dave Martin
0 siblings, 0 replies; 44+ messages in thread
From: Dave Martin @ 2017-07-13 10:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, kernel-hardening, mark.rutland,
catalin.marinas, will.deacon, labbott
On Wed, Jul 12, 2017 at 03:44:23PM +0100, Ard Biesheuvel wrote:
> Add code that checks whether an exception taken from EL1 was caused
> by a faulting stack access before proceeding to save the interrupted
> context to the stack.
>
> This involves checking whether the faulting address coincides with the
> guard page below the task stack of 'current'. This uses tpidrro_el0 and
> sp_el0 as scratch registers, so we can free up a couple of general
> purpose registers for use in the code that performs this check. If it
> turns out we are dealing with a stack overflow, switch to a special
> per-CPU overflow stack so we can at least call panic().
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/thread_info.h | 2 +
> arch/arm64/kernel/entry.S | 49 ++++++++++++++++++++
> arch/arm64/mm/fault.c | 9 ++++
> 4 files changed, 61 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b52db8bb1270..50caf63099c8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -73,6 +73,7 @@ config ARM64
> select HAVE_ARCH_SECCOMP_FILTER
> select HAVE_ARCH_TRACEHOOK
> select HAVE_ARCH_TRANSPARENT_HUGEPAGE
> + select HAVE_ARCH_VMAP_STACK
> select HAVE_ARM_SMCCC
> select HAVE_EBPF_JIT
> select HAVE_C_RECORDMCOUNT
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 46c3b93cf865..1c3e0a3bf87a 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -32,6 +32,8 @@
> #define THREAD_SIZE 16384
> #define THREAD_START_SP (THREAD_SIZE - 16)
>
> +#define OVERFLOW_STACK_SIZE 1024
> +
How was this number chosen?
Kernel stack overflow is not going to get routinely tested -- after all
it should only get exercised when things go horribly wrong.
So, if the panic code (or compiler-generated stack frames) become
bloated enough, we will silently overrun this limit.
Maybe it doesn't matter if we trash a load of unrelated percpu data when
we panic.
> #ifndef __ASSEMBLY__
>
> struct task_struct;
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 2ba3185b1c78..4c3e82d6e2f2 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -392,6 +392,20 @@ ENDPROC(el1_error_invalid)
> */
> .align 6
> el1_sync:
> +#ifdef CONFIG_VMAP_STACK
> + /*
> + * When taking an exception from EL1, we need to check whether it is
> + * caused by a faulting out-of-bounds access to the virtually mapped
> + * stack before we can attempt to preserve the interrupted context.
> + */
> + msr tpidrro_el0, x0 // stash x0
> + mrs x0, far_el1 // get faulting address
> + tbnz x0, #63, .Lcheck_stack_ovf // check if not user address
> +
> +.Lcheck_stack_resume:
> + mrs x0, tpidrro_el0 // restore x0
> +#endif
> +
Unless I'm missing something, this leaves tpidrr0_el0 trashed (and
containing kernel data that userspace can subsequently read).
This could be solved by moving the tpidrro_el0 load out of context
switch and into ret_to_user.
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-13 10:41 ` Dave Martin
-1 siblings, 0 replies; 44+ messages in thread
From: Dave Martin @ 2017-07-13 10:41 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 12, 2017 at 03:44:20PM +0100, Ard Biesheuvel wrote:
> In order to free up sp_el0, which we will need to deal with faulting
> stack accesses when using virtually mapped stacks, switch to register
> x18 as the task struct register. This is permitted by the AAPCS64 ABI,
> and simplifies many references to 'current', given that they no longer
> involve a MSR instruction to access SP_EL0.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
[...]
> diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
> index f6580d4afb0e..b4e3acff699c 100644
> --- a/arch/arm64/include/asm/current.h
> +++ b/arch/arm64/include/asm/current.h
> @@ -13,11 +13,9 @@ struct task_struct;
> */
> static __always_inline struct task_struct *get_current(void)
> {
> - unsigned long sp_el0;
> + register unsigned long tsk asm ("x18");
>
> - asm ("mrs %0, sp_el0" : "=r" (sp_el0));
> -
> - return (struct task_struct *)sp_el0;
> + return (struct task_struct *)tsk;
Nit:
You're explicitly returning an uninitialised variable here: the asm
annotation doesn't change the fact that tsk lifetime is that of the
function. Sufficiently aggressive GCC can probably optimise the whole
thing (and any caller) away as undefined behaviour.
The GCC docs say
"The only supported use for [specifying registers for local variables]
is to specify registers for input and output operands when calling
Extended 'asm'".
As an alternative, you could make tsk a global register variable. I
don't know whether it should be volatile or not in that case --
probably not, since it's constant for a given thread.
Alternatively, the following should work:
unsigned long ret;
asm ("mrs %0, x18" : "=r" (ret));
return ret;
(with -ffixed-x18, naturally).
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
@ 2017-07-13 10:41 ` Dave Martin
0 siblings, 0 replies; 44+ messages in thread
From: Dave Martin @ 2017-07-13 10:41 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, kernel-hardening, mark.rutland,
catalin.marinas, will.deacon, labbott
On Wed, Jul 12, 2017 at 03:44:20PM +0100, Ard Biesheuvel wrote:
> In order to free up sp_el0, which we will need to deal with faulting
> stack accesses when using virtually mapped stacks, switch to register
> x18 as the task struct register. This is permitted by the AAPCS64 ABI,
> and simplifies many references to 'current', given that they no longer
> involve a MSR instruction to access SP_EL0.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
[...]
> diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
> index f6580d4afb0e..b4e3acff699c 100644
> --- a/arch/arm64/include/asm/current.h
> +++ b/arch/arm64/include/asm/current.h
> @@ -13,11 +13,9 @@ struct task_struct;
> */
> static __always_inline struct task_struct *get_current(void)
> {
> - unsigned long sp_el0;
> + register unsigned long tsk asm ("x18");
>
> - asm ("mrs %0, sp_el0" : "=r" (sp_el0));
> -
> - return (struct task_struct *)sp_el0;
> + return (struct task_struct *)tsk;
Nit:
You're explicitly returning an uninitialised variable here: the asm
annotation doesn't change the fact that tsk lifetime is that of the
function. Sufficiently aggressive GCC can probably optimise the whole
thing (and any caller) away as undefined behaviour.
The GCC docs say
"The only supported use for [specifying registers for local variables]
is to specify registers for input and output operands when calling
Extended 'asm'".
As an alternative, you could make tsk a global register variable. I
don't know whether it should be volatile or not in that case --
probably not, since it's constant for a given thread.
Alternatively, the following should work:
unsigned long ret;
asm ("mrs %0, x18" : "=r" (ret));
return ret;
(with -ffixed-x18, naturally).
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
2017-07-13 10:41 ` [kernel-hardening] " Dave Martin
@ 2017-07-13 12:27 ` Ard Biesheuvel
-1 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-13 12:27 UTC (permalink / raw)
To: linux-arm-kernel
On 13 July 2017 at 11:41, Dave Martin <Dave.Martin@arm.com> wrote:
> On Wed, Jul 12, 2017 at 03:44:20PM +0100, Ard Biesheuvel wrote:
>> In order to free up sp_el0, which we will need to deal with faulting
>> stack accesses when using virtually mapped stacks, switch to register
>> x18 as the task struct register. This is permitted by the AAPCS64 ABI,
>> and simplifies many references to 'current', given that they no longer
>> involve a MSR instruction to access SP_EL0.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> [...]
>
>> diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
>> index f6580d4afb0e..b4e3acff699c 100644
>> --- a/arch/arm64/include/asm/current.h
>> +++ b/arch/arm64/include/asm/current.h
>> @@ -13,11 +13,9 @@ struct task_struct;
>> */
>> static __always_inline struct task_struct *get_current(void)
>> {
>> - unsigned long sp_el0;
>> + register unsigned long tsk asm ("x18");
>>
>> - asm ("mrs %0, sp_el0" : "=r" (sp_el0));
>> -
>> - return (struct task_struct *)sp_el0;
>> + return (struct task_struct *)tsk;
>
> Nit:
>
> You're explicitly returning an uninitialised variable here: the asm
> annotation doesn't change the fact that tsk lifetime is that of the
> function. Sufficiently aggressive GCC can probably optimise the whole
> thing (and any caller) away as undefined behaviour.
>
> The GCC docs say
>
> "The only supported use for [specifying registers for local variables]
> is to specify registers for input and output operands when calling
> Extended 'asm'".
>
Ah ok, so it needs to live outside of the function, just like
current_stack_pointer.
>
> As an alternative, you could make tsk a global register variable. I
> don't know whether it should be volatile or not in that case --
> probably not, since it's constant for a given thread.
>
> Alternatively, the following should work:
>
> unsigned long ret;
>
> asm ("mrs %0, x18" : "=r" (ret));
>
> return ret;
>
> (with -ffixed-x18, naturally).
>
Indeed (assuming you meant mov not mrs)
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
@ 2017-07-13 12:27 ` Ard Biesheuvel
0 siblings, 0 replies; 44+ messages in thread
From: Ard Biesheuvel @ 2017-07-13 12:27 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arm-kernel, Kernel Hardening, Mark Rutland,
Catalin Marinas, Will Deacon, Laura Abbott
On 13 July 2017 at 11:41, Dave Martin <Dave.Martin@arm.com> wrote:
> On Wed, Jul 12, 2017 at 03:44:20PM +0100, Ard Biesheuvel wrote:
>> In order to free up sp_el0, which we will need to deal with faulting
>> stack accesses when using virtually mapped stacks, switch to register
>> x18 as the task struct register. This is permitted by the AAPCS64 ABI,
>> and simplifies many references to 'current', given that they no longer
>> involve a MSR instruction to access SP_EL0.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> [...]
>
>> diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
>> index f6580d4afb0e..b4e3acff699c 100644
>> --- a/arch/arm64/include/asm/current.h
>> +++ b/arch/arm64/include/asm/current.h
>> @@ -13,11 +13,9 @@ struct task_struct;
>> */
>> static __always_inline struct task_struct *get_current(void)
>> {
>> - unsigned long sp_el0;
>> + register unsigned long tsk asm ("x18");
>>
>> - asm ("mrs %0, sp_el0" : "=r" (sp_el0));
>> -
>> - return (struct task_struct *)sp_el0;
>> + return (struct task_struct *)tsk;
>
> Nit:
>
> You're explicitly returning an uninitialised variable here: the asm
> annotation doesn't change the fact that tsk lifetime is that of the
> function. Sufficiently aggressive GCC can probably optimise the whole
> thing (and any caller) away as undefined behaviour.
>
> The GCC docs say
>
> "The only supported use for [specifying registers for local variables]
> is to specify registers for input and output operands when calling
> Extended 'asm'".
>
Ah ok, so it needs to live outside of the function, just like
current_stack_pointer.
>
> As an alternative, you could make tsk a global register variable. I
> don't know whether it should be volatile or not in that case --
> probably not, since it's constant for a given thread.
>
> Alternatively, the following should work:
>
> unsigned long ret;
>
> asm ("mrs %0, x18" : "=r" (ret));
>
> return ret;
>
> (with -ffixed-x18, naturally).
>
Indeed (assuming you meant mov not mrs)
^ permalink raw reply [flat|nested] 44+ messages in thread
* [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
2017-07-13 12:27 ` [kernel-hardening] " Ard Biesheuvel
@ 2017-07-13 14:11 ` Dave Martin
-1 siblings, 0 replies; 44+ messages in thread
From: Dave Martin @ 2017-07-13 14:11 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 13, 2017 at 01:27:50PM +0100, Ard Biesheuvel wrote:
> On 13 July 2017 at 11:41, Dave Martin <Dave.Martin@arm.com> wrote:
> > On Wed, Jul 12, 2017 at 03:44:20PM +0100, Ard Biesheuvel wrote:
> >> In order to free up sp_el0, which we will need to deal with faulting
> >> stack accesses when using virtually mapped stacks, switch to register
> >> x18 as the task struct register. This is permitted by the AAPCS64 ABI,
> >> and simplifies many references to 'current', given that they no longer
> >> involve a MSR instruction to access SP_EL0.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > [...]
> >
> >> diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
> >> index f6580d4afb0e..b4e3acff699c 100644
> >> --- a/arch/arm64/include/asm/current.h
> >> +++ b/arch/arm64/include/asm/current.h
> >> @@ -13,11 +13,9 @@ struct task_struct;
> >> */
> >> static __always_inline struct task_struct *get_current(void)
> >> {
> >> - unsigned long sp_el0;
> >> + register unsigned long tsk asm ("x18");
> >>
> >> - asm ("mrs %0, sp_el0" : "=r" (sp_el0));
> >> -
> >> - return (struct task_struct *)sp_el0;
> >> + return (struct task_struct *)tsk;
> >
> > Nit:
> >
> > You're explicitly returning an uninitialised variable here: the asm
> > annotation doesn't change the fact that tsk lifetime is that of the
> > function. Sufficiently aggressive GCC can probably optimise the whole
> > thing (and any caller) away as undefined behaviour.
> >
> > The GCC docs say
> >
> > "The only supported use for [specifying registers for local variables]
> > is to specify registers for input and output operands when calling
> > Extended 'asm'".
> >
>
> Ah ok, so it needs to live outside of the function, just like
> current_stack_pointer.
>
> >
> > As an alternative, you could make tsk a global register variable. I
> > don't know whether it should be volatile or not in that case --
> > probably not, since it's constant for a given thread.
> >
> > Alternatively, the following should work:
> >
> > unsigned long ret;
> >
> > asm ("mrs %0, x18" : "=r" (ret));
> >
> > return ret;
> >
> > (with -ffixed-x18, naturally).
> >
>
> Indeed (assuming you meant mov not mrs)
Yes (oops).
Cheers
---Dave
^ permalink raw reply [flat|nested] 44+ messages in thread
* [kernel-hardening] Re: [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer
@ 2017-07-13 14:11 ` Dave Martin
0 siblings, 0 replies; 44+ messages in thread
From: Dave Martin @ 2017-07-13 14:11 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Mark Rutland, Kernel Hardening, Catalin Marinas, Will Deacon,
linux-arm-kernel, Laura Abbott
On Thu, Jul 13, 2017 at 01:27:50PM +0100, Ard Biesheuvel wrote:
> On 13 July 2017 at 11:41, Dave Martin <Dave.Martin@arm.com> wrote:
> > On Wed, Jul 12, 2017 at 03:44:20PM +0100, Ard Biesheuvel wrote:
> >> In order to free up sp_el0, which we will need to deal with faulting
> >> stack accesses when using virtually mapped stacks, switch to register
> >> x18 as the task struct register. This is permitted by the AAPCS64 ABI,
> >> and simplifies many references to 'current', given that they no longer
> >> involve a MSR instruction to access SP_EL0.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > [...]
> >
> >> diff --git a/arch/arm64/include/asm/current.h b/arch/arm64/include/asm/current.h
> >> index f6580d4afb0e..b4e3acff699c 100644
> >> --- a/arch/arm64/include/asm/current.h
> >> +++ b/arch/arm64/include/asm/current.h
> >> @@ -13,11 +13,9 @@ struct task_struct;
> >> */
> >> static __always_inline struct task_struct *get_current(void)
> >> {
> >> - unsigned long sp_el0;
> >> + register unsigned long tsk asm ("x18");
> >>
> >> - asm ("mrs %0, sp_el0" : "=r" (sp_el0));
> >> -
> >> - return (struct task_struct *)sp_el0;
> >> + return (struct task_struct *)tsk;
> >
> > Nit:
> >
> > You're explicitly returning an uninitialised variable here: the asm
> > annotation doesn't change the fact that tsk lifetime is that of the
> > function. Sufficiently aggressive GCC can probably optimise the whole
> > thing (and any caller) away as undefined behaviour.
> >
> > The GCC docs say
> >
> > "The only supported use for [specifying registers for local variables]
> > is to specify registers for input and output operands when calling
> > Extended 'asm'".
> >
>
> Ah ok, so it needs to live outside of the function, just like
> current_stack_pointer.
>
> >
> > As an alternative, you could make tsk a global register variable. I
> > don't know whether it should be volatile or not in that case --
> > probably not, since it's constant for a given thread.
> >
> > Alternatively, the following should work:
> >
> > unsigned long ret;
> >
> > asm ("mrs %0, x18" : "=r" (ret));
> >
> > return ret;
> >
> > (with -ffixed-x18, naturally).
> >
>
> Indeed (assuming you meant mov not mrs)
Yes (oops).
Cheers
---Dave
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2017-07-13 14:11 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-12 14:44 [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 01/10] arm64/lib: copy_page: use consistent prefetch stride Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 02/10] arm64/lib: copy_page: avoid x18 register in assembler code Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 03/10] arm64: crypto: avoid register x18 in scalar AES code Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 04/10] arm64: kvm: stop treating register x18 as caller save Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 05/10] arm64: kernel: avoid x18 as an arbitrary temp register Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 06/10] arm64: kbuild: reserve reg x18 from general allocation by the compiler Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 07/10] arm64: kernel: switch to register x18 as a task struct pointer Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-13 10:41 ` Dave Martin
2017-07-13 10:41 ` [kernel-hardening] " Dave Martin
2017-07-13 12:27 ` Ard Biesheuvel
2017-07-13 12:27 ` [kernel-hardening] " Ard Biesheuvel
2017-07-13 14:11 ` Dave Martin
2017-07-13 14:11 ` [kernel-hardening] " Dave Martin
2017-07-12 14:44 ` [RFC PATCH 08/10] arm64/kernel: dump entire stack if sp points elsewhere Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 09/10] arm64: mm: add C level handling for stack overflows Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 14:44 ` [RFC PATCH 10/10] arm64: kernel: add support for virtually mapped stacks Ard Biesheuvel
2017-07-12 14:44 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 22:59 ` Mark Rutland
2017-07-12 22:59 ` [kernel-hardening] " Mark Rutland
2017-07-13 9:12 ` Mark Rutland
2017-07-13 9:12 ` Mark Rutland
2017-07-13 10:35 ` Dave Martin
2017-07-13 10:35 ` [kernel-hardening] " Dave Martin
2017-07-12 20:12 ` [RFC PATCH 00/10] arm64: allow virtually mapped stacks to be enabled Laura Abbott
2017-07-12 20:12 ` [kernel-hardening] " Laura Abbott
2017-07-12 20:49 ` Ard Biesheuvel
2017-07-12 20:49 ` [kernel-hardening] " Ard Biesheuvel
2017-07-12 21:32 ` Andy Lutomirski
2017-07-12 21:32 ` [kernel-hardening] " Andy Lutomirski
2017-07-12 22:47 ` Mark Rutland
2017-07-12 22:47 ` [kernel-hardening] " Mark Rutland
2017-07-13 6:51 ` Ard Biesheuvel
2017-07-13 6:51 ` [kernel-hardening] " Ard Biesheuvel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.