Page protections for arm64
v2 cover Page protections for arm64
Hi,
This is v2 of adding page protections for arm64. This implements CONFIG_DEBUG_SET_MODULE_RONX for modules and adds similar protections for direct mapped memory. The module portion (1/4) should be ready for merging assuming no more issues are found (I know I'm off cycle here).
v2:
- Added a memory barrier after flush_tlb_kernel_range per request of Steve Capper.
- Dropped the ENTRY per Will in handle_arch_irq
- Added the Kconfig to align sections up to SECTION_SIZE. Added a patch to fix the related relocation errors from this option.
Laura Abbott (4):
arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
arm64: Treat handle_arch_irq as a function pointer
arm64: Switch to ldr for loading the stub vectors
arm64: add better page protections to arm64
arch/arm64/Kconfig.debug | 34 +++++++ arch/arm64/include/asm/cacheflush.h | 4 + arch/arm64/kernel/entry.S | 8 +- arch/arm64/kernel/head.S | 2 +- arch/arm64/kernel/vmlinux.lds.S | 17 ++++ arch/arm64/mm/Makefile | 2 +- arch/arm64/mm/init.c | 1 + arch/arm64/mm/mm.h | 2 + arch/arm64/mm/mmu.c | 173 ++++++++++++++++++++++++++++++++---- arch/arm64/mm/pageattr.c | 121 +++++++++++++++++++++++++ 10 files changed, 344 insertions(+), 20 deletions(-) create mode 100644 arch/arm64/mm/pageattr.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 1/4 arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
In a similar fashion to other architecture, add the infrastructure and Kconfig to enable DEBUG_SET_MODULE_RONX support. When enabled, module ranges will be marked read-only/no-execute as appropriate.
--- arch/arm64/Kconfig.debug | 11 ++++ arch/arm64/include/asm/cacheflush.h | 4 ++ arch/arm64/mm/Makefile | 2 +- arch/arm64/mm/pageattr.c | 121 ++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/mm/pageattr.c
arch/arm64/Kconfig.debug +11 -0
@@ -37,4 +37,15 @@ config PID_IN_CONTEXTIDR instructions during context switch. Say Y here only if you are planning to use hardware trace tools with this kernel. +config DEBUG_SET_MODULE_RONX + bool "Set loadable kernel module data as NX and text as RO" + depends on MODULES + help + This option helps catch unintended modifications to loadable + kernel module's text and read-only data. It also prevents execution + of module data. Such protection may interfere with run-time code + patching and dynamic kernel tracing - and they might also protect + against certain classes of kernel exploits. + If in doubt, say "N". + endmenu
arch/arm64/include/asm/cacheflush.h +4 -0
@@ -157,4 +157,8 @@ static inline void flush_cache_vunmap(unsigned long start, unsigned long end) { } +int set_memory_ro (unsigned long addr,int numpages); +int set_memory_rw (unsigned long addr,int numpages); +int set_memory_x (unsigned long addr,int numpages); +int set_memory_nx (unsigned long addr,int numpages);#endif
arch/arm64/mm/Makefile +1 -1
@@ -1,5 +1,5 @@ obj-y := dma-mapping.o extable.o fault.o init.o \ cache.o copypage.o flush.o \ ioremap.o mmap.o pgd.o mmu.o \ - context.o tlb.o proc.o + context.o tlb.o proc.o pageattr.o obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
arch/arm64/mm/pageattr.c new file +121 -0
121 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 1/4 arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
Hi Laura,
This is looking better, but comments inline.
Quoted 14 lines · Laura Abbott
In a similar fashion to other architecture, add the infrastructure and
Kconfigto enableDEBUG_SET_MODULE_RONXsupport. When enabled, module ranges will be marked read-only/no-execute as appropriate.Signed-off-by Laura Abbott <[email protected]>--- arch/arm64/Kconfig.debug | 11 ++++ arch/arm64/include/asm/cacheflush.h | 4 ++ arch/arm64/mm/Makefile | 2 +- arch/arm64/mm/pageattr.c | 121 ++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/mm/pageattr.c
[...]
Quoted 27 lines
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
arch/arm64/mm/pageattr.c new file +20
-0@@ -0,0 +1,121 @@ +/* +* Copyright (c) 2014, The Linux Foundation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 and +* only version 2 as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ +#include <linux/kernel.h> +#include <linux/mm.h> +#include <linux/sched.h> + +#include <asm/pgtable.h> +#include <asm/tlbflush.h> + +static pte_t clear_pte_bit (pte_t pte,pgprot_t prot)
arch/arm64/mm/pageattr.c line 30
Quoted 10 lines
arch/arm64/mm/pageattr.c new file +10
-0+{ +pte_val (pte) &= ~pgprot_val (prot); +return pte; +} + +static pte_t set_pte_bit (pte_t pte,pgprot_t prot) +{ +pte_val (pte) |=pgprot_val (prot); +return pte; +}
We could actually re-use these for building our pte_mk* functions in pgtable.h. Care to move them there?
Quoted 32 lines
arch/arm64/mm/pageattr.c new file +32
-0+static int __change_memory (pte_t *ptep,pgtable_t token,unsigned long addr, +pgprot_t prot,bool set) +{ +pte_t pte; + +if (set) + pte =set_pte_bit (*ptep, prot); +else + pte =clear_pte_bit (*ptep, prot); +set_pte (ptep, pte); +return 0 ; +} + +static int set_page_range (pte_t *ptep,pgtable_t token,unsigned long addr, +void *data) +{ +pgprot_t prot = (pgprot_t )data; + +return __change_memory (ptep, token, addr, prot, true); +} + +static int clear_page_range (pte_t *ptep,pgtable_t token,unsigned long addr, +void *data) +{ +pgprot_t prot = (pgprot_t )data; + +return __change_memory (ptep, token, addr, prot, false); +} + +static int change_memory_common (unsigned long addr,int numpages, +pgprot_t prot,bool set) +{
arch/arm64/mm/pageattr.c line 73
Quoted 10 lines
arch/arm64/mm/pageattr.c new file +10
-0+unsigned long start = addr; +unsigned long size =PAGE_SIZE *numpages; +unsigned long end = start + size; +int ret; + +if (start <MODULES_VADDR || start >=MODULES_END ) +return -EINVAL ; + +if (end <MODULES_VADDR || end >=MODULES_END ) +return -EINVAL ;
Can you use is_module_address here, or do you need to change the page attributes for areas where no modules are currently loaded too?
arch/arm64/mm/pageattr.c line 84
Quoted 10 lines
arch/arm64/mm/pageattr.c new file +10
-0+if (set) + ret =apply_to_page_range (&init_mm, start, size, + set_page_range, (void *)prot); +else + ret =apply_to_page_range (&init_mm, start, size, + clear_page_range, (void *)prot); + +flush_tlb_kernel_range (start, end); +isb (); +return ret;
We already have an isb in flush_tlb_kernel_range.
Quoted 13 lines
arch/arm64/mm/pageattr.c new file +13
-0+static int change_memory_set_bit (unsigned long addr,int numpages, +pgprot_t prot) +{ +return change_memory_common (addr, numpages, prot, true); +} + +static int change_memory_clear_bit (unsigned long addr,int numpages, +pgprot_t prot) +{ +return change_memory_common (addr, numpages, prot, false); +} + +int set_memory_ro (unsigned long addr,int numpages)
arch/arm64/mm/pageattr.c line 109
Quoted 10 lines
arch/arm64/mm/pageattr.c new file +10
-0+{ +return change_memory_set_bit (addr, numpages,__pgprot (PTE_RDONLY )); +} +EXPORT_SYMBOL_GPL (set_memory_ro); + +int set_memory_rw (unsigned long addr,int numpages) +{ +return change_memory_clear_bit (addr, numpages,__pgprot (PTE_RDONLY )); +} +EXPORT_SYMBOL_GPL (set_memory_rw);
I'm slightly worried about the interaction with this and PTE_WRITE (see linux-next). If the kernel pages are marked as PTE_DIRTY | PTE_WRITE, then setting read-only is a weird contradiction. Can you take PTE_WRITE into account for these two please?
Will
PATCH v2 1/4 arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
On 3 June 2014 16:22, Will Deacon <[email protected]> wrote:
[ ... ]
Quoted 2 lines
We already have an isb in
flush_tlb_kernel_range.
Hi Will,
The following thread discusses the removal of the isb() from flush_tlb_kernel_range:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252829.html
Also, I asked for it to be added to this series in this email thread but you thought it was benign:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252869.html
So we could probably do without the isb(.).
Cheers,
--
Steve
PATCH v2 1/4 arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
Quoted 12 lines · Steve Capper
On 3 June 2014 16:22, Will Deacon <[email protected]> wrote:
[ ... ]
Quoted 2 lines
We already have an isb in
flush_tlb_kernel_range.Hi Will,
The following thread discusses the removal of theisb()fromflush_tlb_kernel_range:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252829.html
Quoted 4 lines
Also, I asked for it to be added to this series in this email thread but you thought it was benign:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252869.html
So we could probably do without the isb(.).
Bah, I'd completely forgotten all of that! Anyway, I'd say it's up to the patch removing the existing isb from flush_tlb_kernel_range to fix the callers, not for other people to anticipate that change.
Even with that change, the isb isn't needed, as mentioned in the second link above.
Will
PATCH v2 1/4 arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
Quoted 13 lines · Will Deacon
Quoted 12 lines · Steve Capper
On 3 June 2014 16:22, Will Deacon <[email protected]> wrote:
[ ... ]
Quoted 2 lines
We already have an isb in
flush_tlb_kernel_range.Hi Will,
The following thread discusses the removal of theisb()fromflush_tlb_kernel_range:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252829.html
Quoted 8 lines
Quoted 4 lines
Also, I asked for it to be added to this series in this email thread but you thought it was benign:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252869.html
So we could probably do without the isb(.).Bah, I'd completely forgotten all of that! Anyway, I'd say it's up to the patch removing the existing isb from
flush_tlb_kernel_rangeto fix the callers, not for other people to anticipate that change.
Agreed.... but Catalin has applied my patch before this version hit the list:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/254828.html
Quoted 3 lines
Even with that change, the isb isn't needed, as mentioned in the second link above.
Agreed :-).
PATCH v2 1/4 arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
In reply to Will DeaconQuoted 54 lines · Will Deacon
Hi Laura,
This is looking better, but comments inline.
Quoted 14 lines · Laura Abbott
In a similar fashion to other architecture, add the infrastructure and
Kconfigto enableDEBUG_SET_MODULE_RONXsupport. When enabled, module ranges will be marked read-only/no-execute as appropriate.Signed-off-by Laura Abbott <[email protected]>--- arch/arm64/Kconfig.debug | 11 ++++ arch/arm64/include/asm/cacheflush.h | 4 ++ arch/arm64/mm/Makefile | 2 +- arch/arm64/mm/pageattr.c | 121 ++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/mm/pageattr.c[...]
Quoted 32 lines
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
arch/arm64/mm/pageattr.c new file +25
-0@@ -0,0 +1,121 @@ +/* +* Copyright (c) 2014, The Linux Foundation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 and +* only version 2 as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ +#include <linux/kernel.h> +#include <linux/mm.h> +#include <linux/sched.h> + +#include <asm/pgtable.h> +#include <asm/tlbflush.h> + +static pte_t clear_pte_bit (pte_t pte,pgprot_t prot) +{ +pte_val (pte) &= ~pgprot_val (prot); +return pte; +} +
Quoted 9 lines
arch/arm64/mm/pageattr.c line 30
Quoted 5 lines
arch/arm64/mm/pageattr.c new file +5
-0+static pte_t set_pte_bit (pte_t pte,pgprot_t prot) +{ +pte_val (pte) |=pgprot_val (prot); +return pte; +}We could actually re-use these for building our pte_mk* functions in
pgtable.h. Care to move them there?
Fine.
Quoted 37 lines
Quoted 37 lines
arch/arm64/mm/pageattr.c new file +37
-0+static int __change_memory (pte_t *ptep,pgtable_t token,unsigned long addr, +pgprot_t prot,bool set) +{ +pte_t pte; + +if (set) + pte =set_pte_bit (*ptep, prot); +else + pte =clear_pte_bit (*ptep, prot); +set_pte (ptep, pte); +return 0 ; +} + +static int set_page_range (pte_t *ptep,pgtable_t token,unsigned long addr, +void *data) +{ +pgprot_t prot = (pgprot_t )data; + +return __change_memory (ptep, token, addr, prot, true); +} + +static int clear_page_range (pte_t *ptep,pgtable_t token,unsigned long addr, +void *data) +{ +pgprot_t prot = (pgprot_t )data; + +return __change_memory (ptep, token, addr, prot, false); +} + +static int change_memory_common (unsigned long addr,int numpages, +pgprot_t prot,bool set) +{ +unsigned long start = addr; +unsigned long size =PAGE_SIZE *numpages; +unsigned long end = start + size; +int ret; +
Quoted 9 lines
arch/arm64/mm/pageattr.c line 73
Quoted 5 lines
arch/arm64/mm/pageattr.c new file +5
-0+if (start <MODULES_VADDR || start >=MODULES_END ) +return -EINVAL ; + +if (end <MODULES_VADDR || end >=MODULES_END ) +return -EINVAL ;Can you use
is_module_addresshere, or do you need to change the page attributes for areas where no modules are currently loaded too?
Yes, I think is_module_address should work fine.
Quoted 4 lines
Quoted 4 lines
arch/arm64/mm/pageattr.c new file +4
-0+if (set) + ret =apply_to_page_range (&init_mm, start, size, + set_page_range, (void *)prot); +else
Quoted 9 lines
arch/arm64/mm/pageattr.c line 84
Quoted 6 lines
arch/arm64/mm/pageattr.c new file +6
-0+ ret =apply_to_page_range (&init_mm, start, size, + clear_page_range, (void *)prot); + +flush_tlb_kernel_range (start, end); +isb (); +return ret;We already have an isb in
flush_tlb_kernel_range.
Yes, I'll drop the isb here.
Quoted 20 lines
Quoted 20 lines
arch/arm64/mm/pageattr.c new file +20
-0+static int change_memory_set_bit (unsigned long addr,int numpages, +pgprot_t prot) +{ +return change_memory_common (addr, numpages, prot, true); +} + +static int change_memory_clear_bit (unsigned long addr,int numpages, +pgprot_t prot) +{ +return change_memory_common (addr, numpages, prot, false); +} + +int set_memory_ro (unsigned long addr,int numpages) +{ +return change_memory_set_bit (addr, numpages,__pgprot (PTE_RDONLY )); +} +EXPORT_SYMBOL_GPL (set_memory_ro); + +int set_memory_rw (unsigned long addr,int numpages) +{
Quoted 9 lines
arch/arm64/mm/pageattr.c line 109
Quoted 3 lines
arch/arm64/mm/pageattr.c new file +3
-0+return change_memory_clear_bit (addr, numpages,__pgprot (PTE_RDONLY )); +} +EXPORT_SYMBOL_GPL (set_memory_rw);I'm slightly worried about the interaction with this and
PTE_WRITE(see linux-next). If the kernel pages are marked asPTE_DIRTY|PTE_WRITE, then setting read-only is a weird contradiction. Can you takePTE_WRITEinto account for these two please?
It sounds like the solution should be to set/clear PTE_WRITE as appropriate here, is my understanding correct?
Quoted 2 lines
Will
Thanks,
Laura
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 1/4 arm64: Add CONFIG_DEBUG_SET_MODULE_RONX support
Quoted 19 lines · Laura Abbott
Quoted 8 lines · Will Deacon
Quoted 8 lines
arch/arm64/mm/pageattr.c new file +8
-0+int set_memory_ro (unsigned long addr,int numpages) +{ +return change_memory_set_bit (addr, numpages,__pgprot (PTE_RDONLY )); +} +EXPORT_SYMBOL_GPL (set_memory_ro); + +int set_memory_rw (unsigned long addr,int numpages) +{Quoted 9 lines
arch/arm64/mm/pageattr.c line 109
Quoted 3 lines
arch/arm64/mm/pageattr.c new file +3
-0+return change_memory_clear_bit (addr, numpages,__pgprot (PTE_RDONLY )); +} +EXPORT_SYMBOL_GPL (set_memory_rw);I'm slightly worried about the interaction with this and
PTE_WRITE(see linux-next). If the kernel pages are marked asPTE_DIRTY|PTE_WRITE, then setting read-only is a weird contradiction. Can you takePTE_WRITEinto account for these two please?
Quoted 2 lines
It sounds like the solution should be to set/clear
PTE_WRITEas appropriate here, is my understanding correct?
You got it! You can look at set_pte_at if you're unsure (bearing in mind that kernel mappings are always dirty).
Will
PATCH v2 2/4 arm64: Treat handle_arch_irq as a function pointer
In reply to Laura Abbotthandle_arch_irq isn't actually text, it's just a function pointer. It doesn't need to be stored in the text section and doing so causes problems if we ever want to make the kernel text readonly. Move it to the data section.
--- arch/arm64/kernel/entry.S | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
arch/arm64/kernel/entry.S +6 -2
@@ -139,7 +139,8 @@ tsk .req x28 // current thread_info * Interrupt handling. */ .macro irq_handler - ldr x1, handle_arch_irq + ldr x1, =handle_arch_irq + ldr x1, [x1] mov x0, sp blr x1 .endm
@@ -678,5 +679,8 @@ ENTRY(sys_rt_sigreturn_wrapper) b sys_rt_sigreturn ENDPROC(sys_rt_sigreturn_wrapper) -ENTRY(handle_arch_irq) + .data + .globl handle_arch_irq + .align 4 +handle_arch_irq: .quad 0
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Mark Rutland
reviewerPATCH v2 2/4 arm64: Treat handle_arch_irq as a function pointer
Hi Laura,
Quoted 26 lines · Laura Abbott
handle_arch_irqisn't actually text, it's just a function pointer. It doesn't need to be stored in the text section and doing so causes problems if we ever want to make the kernel text readonly. Move it to the data section.Signed-off-by Laura Abbott <[email protected]>--- arch/arm64/kernel/entry.S | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)arch/arm64/kernel/entry.S +2
-1@@ -139,7 +139,8 @@ tsk .req x28 // current thread_info * Interrupt handling. */ .macro irq_handler - ldr x1, handle_arch_irq + ldr x1, =handle_arch_irq + ldr x1, [x1] mov x0, sp blr x1 .endm
arch/arm64/kernel/entry.S ENTRY(sys_rt_sigreturn_wrapper) · line 686
Quoted 9 lines
arch/arm64/kernel/entry.S +4
-1@@ -678,5 +679,8 @@ ENTRY(sys_rt_sigreturn_wrapper) b sys_rt_sigreturn ENDPROC(sys_rt_sigreturn_wrapper) -ENTRY(handle_arch_irq) + .data + .globl handle_arch_irq + .align 4 +handle_arch_irq: .quad 0
Rather than having to set the alignment and section manually, would something like the below work, moving the definition into kernel/irq.c?
Cheers,
Mark.
---->8----
arch/arm64/include/asm/irq.h +0 -1
@@ -3,7 +3,6 @@#include <asm-generic/irq.h> -extern void (*handle_arch_irq)(struct pt_regs *);extern void migrate_irqs (void );extern void set_handle_irq (void (*handle_irq)(struct pt_regs *));
arch/arm64/kernel/entry.S +2 -4
@@ -139,7 +139,8 @@ tsk .req x28 // current thread_info * Interrupt handling. */ .macro irq_handler - ldr x1, handle_arch_irq + ldr x1, =handle_arch_irq + ldr x1, [x1] mov x0, sp blr x1 .endm
@@ -677,6 +678,3 @@ ENTRY(sys_rt_sigreturn_wrapper) mov x0, sp b sys_rt_sigreturn ENDPROC(sys_rt_sigreturn_wrapper) - -ENTRY(handle_arch_irq) - .quad 0
arch/arm64/kernel/irq.c +2 -0
@@ -67,6 +67,8 @@ void handle_IRQ(unsigned int irq, struct pt_regs *regs)set_irq_regs (old_regs); } +void (*handle_arch_irq)(struct pt_regs *) =NULL ; +void __init set_handle_irq (void (*handle_irq)(struct pt_regs *)) {if (handle_arch_irq)
PATCH v2 2/4 arm64: Treat handle_arch_irq as a function pointer
Quoted 63 lines · Mark Rutland
Hi Laura,
Quoted 26 lines · Laura Abbott
handle_arch_irqisn't actually text, it's just a function pointer. It doesn't need to be stored in the text section and doing so causes problems if we ever want to make the kernel text readonly. Move it to the data section.Signed-off-by Laura Abbott <[email protected]>--- arch/arm64/kernel/entry.S | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)arch/arm64/kernel/entry.S +2
-1@@ -139,7 +139,8 @@ tsk .req x28 // current thread_info * Interrupt handling. */ .macro irq_handler - ldr x1, handle_arch_irq + ldr x1, =handle_arch_irq + ldr x1, [x1] mov x0, sp blr x1 .endmarch/arm64/kernel/entry.S ENTRY(sys_rt_sigreturn_wrapper) · line 686
Quoted 9 lines
arch/arm64/kernel/entry.S +4
-1@@ -678,5 +679,8 @@ ENTRY(sys_rt_sigreturn_wrapper) b sys_rt_sigreturn ENDPROC(sys_rt_sigreturn_wrapper) -ENTRY(handle_arch_irq) + .data + .globl handle_arch_irq + .align 4 +handle_arch_irq: .quad 0Rather than having to set the alignment and section manually, would something like the below work, moving the definition into
kernel/irq.c?Cheers,
Mark.---->8----
arch/arm64/include/asm/irq.h +0
-0@@ -3,7 +3,6 @@u
#include <asm-generic/irq.h>arch/arm64/include/asm/irq.h +0
-1-extern void (*handle_arch_irq)(struct pt_regs *);extern void migrate_irqs (void );extern void set_handle_irq (void (*handle_irq)(struct pt_regs *));arch/arm64/kernel/entry.S +0
-0
arch/arm64/kernel/entry.S tsk .req x28 // current thread_info · line 143
Quoted 6 lines
arch/arm64/kernel/entry.S +2
-1@@ -139,7 +139,8 @@ tsk .req x28 // current thread_info * Interrupt handling. */ .macro irq_handler - ldr x1, handle_arch_irq + ldr x1, =handle_arch_irq + ldr x1, [x1]
How about using
adrp x1, handle_arch_irq
ldr x1, [x1, #:lo12:handle_arch_irq]
instead, and avoid the literal pool entry?
Signature · 1 line
Ard.
Quoted 28 lines
arch/arm64/kernel/entry.S +0
-3mov x0, sp blr x1 .endm@@ -677,6 +678,3 @@ ENTRY(sys_rt_sigreturn_wrapper) mov x0, sp b sys_rt_sigreturn ENDPROC(sys_rt_sigreturn_wrapper) - -ENTRY(handle_arch_irq) - .quad 0arch/arm64/kernel/irq.c +2
-0@@ -67,6 +67,8 @@ void handle_IRQ(unsigned int irq, struct pt_regs *regs)set_irq_regs (old_regs); } +void (*handle_arch_irq)(struct pt_regs *) =NULL ; +void __init set_handle_irq (void (*handle_irq)(struct pt_regs *)) {if (handle_arch_irq)_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 3/4 arm64: Switch to ldr for loading the stub vectors
In reply to Laura AbbottThe hyp stub vectors are currently loaded using adr. This instruction has a +/- 1MB range for the loading address. If the alignment for sections is changed. the address may be more than 1MB away, resulting in reclocation errors. Switch to using ldr for getting the address to ensure we aren't affected by the location of the __hyp_stub_vectors.
--- arch/arm64/kernel/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/head.S +1 -1
@@ -208,7 +208,7 @@ CPU_LE( movk x0, #0x30d0, lsl #16 ) // Clear EE and E0E on LE systems msr vttbr_el2, xzr /* Hypervisor stub */ - adr x0, __hyp_stub_vectors + ldr x0, =__hyp_stub_vectors msr vbar_el2, x0 /* spsr */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 3/4 arm64: Switch to ldr for loading the stub vectors
Quoted 18 lines · Laura Abbott
The hyp stub vectors are currently loaded using adr. This instruction has a +/- 1MB range for the loading address. If the alignment for sections is changed. the address may be more than 1MB away, resulting in reclocation errors. Switch to using ldr for getting the address to ensure we aren't affected by the location of the
__hyp_stub_vectors.Signed-off-by Laura Abbott <[email protected]>--- arch/arm64/kernel/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/kernel/head.S +0
-0
arch/arm64/kernel/head.S CPU_LE( movk x0, #0x30d0, lsl #16 ) // Clear EE and E0E on LE systems · line 212
Quoted 6 lines
arch/arm64/kernel/head.S +1
-1@@ -208,7 +208,7 @@ CPU_LE( movk x0, #0x30d0, lsl #16 ) // Clear EE and E0E on LE systems msr vttbr_el2, xzr /* Hypervisor stub */ - adr x0, __hyp_stub_vectors + ldr x0, =__hyp_stub_vectors msr vbar_el2, x0
Or use Ard's trick with adrp and the lo12 relocation?
Will
PATCH v2 4/4 arm64: add better page protections to arm64
In reply to Laura AbbottAdd page protections for arm64 similar to those in arm or in progress for arm. This is for security reasons. The flow is currently:
- Map all memory as either RWX or RW. We round to the nearest section to avoid creating page tables before everything is mapped
- Once everything is mapped, if either end of the RWX section should not be X, we split the PMD and remap as necessary
- When initmem is to be freed, we change the permissions back to RW (using stop machine if necessary to flush the TLB)
- If CONFIG_DEBUG_RODATA is set, the read only sections are set read only.
--- arch/arm64/Kconfig.debug | 23 ++++++ arch/arm64/kernel/vmlinux.lds.S | 17 ++++ arch/arm64/mm/init.c | 1 + arch/arm64/mm/mm.h | 2 + arch/arm64/mm/mmu.c | 173 ++++++++++++++++++++++++++++++++++++---- 5 files changed, 200 insertions(+), 16 deletions(-)
arch/arm64/Kconfig.debug +23 -0
@@ -48,4 +48,27 @@ config DEBUG_SET_MODULE_RONX against certain classes of kernel exploits. If in doubt, say "N". +config DEBUG_RODATA + bool "Make kernel text and rodata read-only" + help + If this is set, kernel text and rodata will be made read-only. This + is to help catch accidental or malicious attempts to change the + kernel's executable code. Additionally splits rodata from kernel + text so it can be made explicitly non-executable. + + If in doubt, say Y + +config DEBUG_ALIGN_RODATA + depends on DEBUG_RODATA + bool "Align linker sections up to SECTION_SIZE" + help + If this option is enabled, sections that may potentially be marked as + read only or non-executable will be aligned up to the section size of + the kernel. This prevents sections from being split into pages and + avoids a potential TLB penalty. The downside is an increase in + alignment and potentially wasted space. Turn on this option if + performance is more important than memory pressure. + + If in doubt, say N + endmenu
arch/arm64/kernel/vmlinux.lds.S +17 -0
@@ -8,6 +8,7 @@ #include <asm/thread_info.h> #include <asm/memory.h> #include <asm/page.h> +#include <asm/pgtable.h> #define ARM_EXIT_KEEP(x) #define ARM_EXIT_DISCARD(x) x
@@ -52,6 +53,9 @@ SECTIONS _text = .; HEAD_TEXT } +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); +#endif .text : { /* Real text segment */ _stext = .; /* Text and read-only data */ __exception_text_start = .;
@@ -68,19 +72,32 @@ SECTIONS *(.got) /* Global offset table */ } +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); +#endif RO_DATA(PAGE_SIZE) EXCEPTION_TABLE(8) NOTES _etext = .; /* End of text and rodata section */ +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); +#else . = ALIGN(PAGE_SIZE); +#endif __init_begin = .; INIT_TEXT_SECTION(8) .exit.text : { ARM_EXIT_KEEP(EXIT_TEXT) } + +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); + __init_data_begin = .; +#else . = ALIGN(16); +#endif .init.data : { INIT_DATA INIT_SETUP(16)
arch/arm64/mm/init.c +1 -0
@@ -325,6 +325,7 @@ void __init mem_init(void)void free_initmem (void ) { +fixup_init ();free_initmem_default (0 ); }
arch/arm64/mm/mm.h +2 -0
@@ -1,2 +1,4 @@extern void __init bootmem_init (void );extern void __init arm64_swiotlb_init (void ); + +void fixup_init (void );
arch/arm64/mm/mmu.c +157 -16
259 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 4/4 arm64: add better page protections to arm64
Hi Laura,
I have some comments below:
Quoted 144 lines · Laura Abbott
Add page protections for arm64 similar to those in arm or in progress for arm. This is for security reasons. The flow is currently:
- Map all memory as either RWX or RW. We round to the nearest section to avoid creating page tables before everything is mapped
- Once everything is mapped, if either end of the RWX section should not be X, we split the PMD and remap as necessary
- When initmem is to be freed, we change the permissions back to RW (using stop machine if necessary to flush the TLB)
- IfCONFIG_DEBUG_RODATAis set, the read only sections are set read only.Signed-off-by Laura Abbott <[email protected]>--- arch/arm64/Kconfig.debug | 23 ++++++ arch/arm64/kernel/vmlinux.lds.S | 17 ++++ arch/arm64/mm/init.c | 1 + arch/arm64/mm/mm.h | 2 + arch/arm64/mm/mmu.c | 173 ++++++++++++++++++++++++++++++++++++---- 5 files changed, 200 insertions(+), 16 deletions(-)arch/arm64/Kconfig.debug +23
-0@@ -48,4 +48,27 @@ config DEBUG_SET_MODULE_RONX against certain classes of kernel exploits. If in doubt, say "N". +config DEBUG_RODATA + bool "Make kernel text and rodata read-only" + help + If this is set, kernel text and rodata will be made read-only. This + is to help catch accidental or malicious attempts to change the + kernel's executable code. Additionally splits rodata from kernel + text so it can be made explicitly non-executable. + + If in doubt, say Y + +config DEBUG_ALIGN_RODATA + depends on DEBUG_RODATA + bool "Align linker sections up to SECTION_SIZE" + help + If this option is enabled, sections that may potentially be marked as + read only or non-executable will be aligned up to the section size of + the kernel. This prevents sections from being split into pages and + avoids a potential TLB penalty. The downside is an increase in + alignment and potentially wasted space. Turn on this option if + performance is more important than memory pressure. + + If in doubt, say N + endmenuarch/arm64/kernel/vmlinux.lds.S +17
-0@@ -8,6 +8,7 @@ #include <asm/thread_info.h> #include <asm/memory.h> #include <asm/page.h> +#include <asm/pgtable.h> #define ARM_EXIT_KEEP(x) #define ARM_EXIT_DISCARD(x) x@@ -52,6 +53,9 @@ SECTIONS _text = .; HEAD_TEXT } +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); +#endif .text : { /* Real text segment */ _stext = .; /* Text and read-only data */ __exception_text_start = .;@@ -68,19 +72,32 @@ SECTIONS *(.got) /* Global offset table */ } +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); +#endif RO_DATA(PAGE_SIZE) EXCEPTION_TABLE(8) NOTES _etext = .; /* End of text and rodata section */ +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); +#else . = ALIGN(PAGE_SIZE); +#endif __init_begin = .; INIT_TEXT_SECTION(8) .exit.text : { ARM_EXIT_KEEP(EXIT_TEXT) } + +#ifdef DEBUG_ALIGN_RODATA + . = ALIGN(1<<SECTION_SHIFT); + __init_data_begin = .; +#else . = ALIGN(16); +#endif .init.data : { INIT_DATA INIT_SETUP(16)arch/arm64/mm/init.c +1
-0@@ -325,6 +325,7 @@ void __init mem_init(void)void free_initmem (void ) { +fixup_init ();free_initmem_default (0 ); }arch/arm64/mm/mm.h +2
-0@@ -1,2 +1,4 @@extern void __init bootmem_init (void );extern void __init arm64_swiotlb_init (void ); + +void fixup_init (void );arch/arm64/mm/mmu.c +1
-0@@ -26,6 +26,7 @@#include <linux/memblock.h> #include <linux/fs.h> #include <linux/io.h> +#include <linux/stop_machine.h> #include <asm/cputype.h> #include <asm/sections.h>
arch/arm64/mm/mmu.c static void __init *early_alloc(unsigned long sz) · line 173
Quoted 8 lines
arch/arm64/mm/mmu.c +3
-2@@ -167,26 +168,67 @@ static void __init *early_alloc(unsigned long sz)return ptr; } -static void __init alloc_init_pte (pmd_t *pmd,unsigned long addr, -unsigned long end,unsigned long pfn) +/* +* remap a PMD into pages +*/
As a heads-up, I've sent off a patch that makes use of 1GB pud mappings for the kernel:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/253516.html
And Catalin has applied it (to his devel tree I think?):
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/254803.html
So some logic may be needed for split_pud.
arch/arm64/mm/mmu.c static void __init *early_alloc(unsigned long sz) · line 175
Quoted 2 lines
arch/arm64/mm/mmu.c +2
-0+static noinline void __ref split_pmd(pmd_t *pmd, pgprot_t prot, bool early) +{
prot does not appear to be used by this function?
Quoted 6 lines
arch/arm64/mm/mmu.c +6
-0+pte_t *pte, *start_pte; +u64 val; +unsigned long pfn; +int i =0 ; + + val =pmd_val (*pmd);
arch/arm64/mm/mmu.c static void __init *early_alloc(unsigned long sz) · line 191
Quoted 10 lines
arch/arm64/mm/mmu.c +10
-0+ +if (early) + start_pte = pte =early_alloc (PTRS_PER_PTE *sizeof (pte_t)); +else + start_pte = pte = (pte_t *)__get_free_page (PGALLOC_GFP ); + +BUG_ON (!pte); + + + pfn =__phys_to_pfn (val &PHYS_MASK );
Would it be better to have:
pfn = pmd_pfn(*pmd);
Quoted 125 lines
arch/arm64/mm/mmu.c +56
-13+ +do { +set_pte (pte,pfn_pte (pfn,PAGE_KERNEL_EXEC )); + pfn++; + }while (pte++, i++, i <PTRS_PER_PTE ); + + +__pmd_populate (pmd,__pa (start_pte),PMD_TYPE_TABLE ); +flush_tlb_all (); +} + +static void __ref alloc_init_pte (pmd_t *pmd,unsigned long addr, +unsigned long end,unsigned long pfn, +pgprot_t prot,bool early) {pte_t *pte;if (pmd_none (*pmd)) { - pte =early_alloc (PTRS_PER_PTE *sizeof (pte_t)); +if (early) + pte =early_alloc (PTRS_PER_PTE *sizeof (pte_t)); +else + pte = (pte_t *)__get_free_page (PGALLOC_GFP ); +BUG_ON (!pte);__pmd_populate (pmd,__pa (pte),PMD_TYPE_TABLE ); } -BUG_ON (pmd_bad (*pmd)); + +if (pmd_bad (*pmd)) +split_pmd (pmd, prot, early); pte =pte_offset_kernel (pmd, addr);do { -set_pte (pte,pfn_pte (pfn,PAGE_KERNEL_EXEC )); +set_pte (pte,pfn_pte (pfn, prot)); pfn++; }while (pte++, addr +=PAGE_SIZE , addr != end); } -static void __init alloc_init_pmd (pud_t *pud,unsigned long addr, -unsigned long end,phys_addr_t phys) +static void __ref alloc_init_pmd (pud_t *pud,unsigned long addr, +unsigned long end,phys_addr_t phys, +pgprot_t sect_prot,pgprot_t pte_prot, +bool early) {pmd_t *pmd;unsigned long next;@@ -195,7 +237,11 @@ static void __init alloc_init_pmd(pud_t *pud, unsigned long addr,* Check for initial section mappings in the pgd/pud and remove them. */ if (pud_none (*pud) ||pud_bad (*pud)) { - pmd =early_alloc (PTRS_PER_PMD *sizeof (pmd_t)); +if (early) + pmd =early_alloc (PTRS_PER_PMD *sizeof (pmd_t)); +else + pmd =pmd_alloc_one (&init_mm, addr); +BUG_ON (!pmd);pud_populate (&init_mm, pud, pmd); }@@ -213,21 +259,25 @@ static void __init alloc_init_pmd(pud_t *pud, unsigned long addr,if (!pmd_none (old_pmd))flush_tlb_all (); }else { -alloc_init_pte (pmd, addr, next,__phys_to_pfn (phys)); +alloc_init_pte (pmd, addr, next,__phys_to_pfn (phys), + pte_prot, early); } phys += next - addr; }while (pmd++, addr = next, addr != end); } -static void __init alloc_init_pud (pgd_t *pgd,unsigned long addr, -unsigned long end,unsigned long phys) +static void __ref alloc_init_pud (pgd_t *pgd,unsigned long addr, +unsigned long end,unsigned long phys, +pgprot_t sect_prot,pgprot_t pte_prot, +bool early) {pud_t *pud =pud_offset (pgd, addr);unsigned long next;do { next =pud_addr_end (addr, end); -alloc_init_pmd (pud, addr, next, phys); +alloc_init_pmd (pud, addr, next, phys, sect_prot, pte_prot, + early); phys += next - addr; }while (pud++, addr = next, addr != end); }@@ -236,8 +286,10 @@ static void __init alloc_init_pud(pgd_t *pgd, unsigned long addr,* Create the page directory entries and any necessary page tables for the * mapping specified by 'md'. */ -static void __init create_mapping (phys_addr_t phys,unsigned long virt, -phys_addr_t size) +static void __ref __create_mapping (phys_addr_t phys,unsigned long virt, +phys_addr_t size, +pgprot_t sect_prot,pgprot_t pte_prot, +bool early) {unsigned long addr, length, end, next;pgd_t *pgd;@@ -255,15 +307,37 @@ static void __init create_mapping(phys_addr_t phys, unsigned long virt, end = addr + length;do { next =pgd_addr_end (addr, end); -alloc_init_pud (pgd, addr, next, phys); +alloc_init_pud (pgd, addr, next, phys, sect_prot, pte_prot, + early); phys += next - addr; }while (pgd++, addr = next, addr != end); } +static void __ref create_mapping (phys_addr_t phys,unsigned long virt, +phys_addr_t size, +pgprot_t sect_prot,pgprot_t pte_prot) +{ +return __create_mapping (phys, virt, size, sect_prot, pte_prot, true); +} + +static void __ref create_mapping_late (phys_addr_t phys,unsigned long virt, +phys_addr_t size, +pgprot_t sect_prot,pgprot_t pte_prot)
arch/arm64/mm/mmu.c static void __init create_mapping(phys_addr_t phys, unsigned long virt, · line 335
Quoted 10 lines
arch/arm64/mm/mmu.c +6
-0+{ +return __create_mapping (phys, virt, size, sect_prot, pte_prot, false); +} +static void __init map_mem (void ) {struct memblock_region *reg;phys_addr_t limit; +/* +* Set up the executable regions using the exising section mappings
nit: existing
arch/arm64/mm/mmu.c static void __init create_mapping(phys_addr_t phys, unsigned long virt, · line 336
Quoted 1 line
arch/arm64/mm/mmu.c +1
-0+* foir now. This will get more fine grained later once all memory
nit: for
arch/arm64/mm/mmu.c static void __init create_mapping(phys_addr_t phys, unsigned long virt, · line 340
Quoted 4 lines
arch/arm64/mm/mmu.c +4
-0+* is mapped +*/ +unsigned long kernel_x_start =round_down (__pa (_stext),SECTION_SIZE ); +unsigned long kernel_x_end =round_up (__pa (__init_end),SECTION_SIZE );
This logic is rounded to SECTION_SIZE, again a heads-up, the 1GB mappings would benefit from some different logic.
Quoted 17 lines
arch/arm64/mm/mmu.c +9
-1/* * Temporarily limit the memblock range. We need to do this as @@ -301,13 +375,79 @@ static void __init map_mem(void) }#endif -create_mapping (start,__phys_to_virt (start), end - start); +if (end < kernel_x_start) { +create_mapping (start,__phys_to_virt (start), end - start, + prot_sect_kernel, pgprot_default); + }else if (start >= kernel_x_end) { +create_mapping (start,__phys_to_virt (start), end - start, + prot_sect_kernel |PMD_SECT_PXN , pgprot_default |PTE_PXN ); + }else { +if (start < kernel_x_start) + create_mapping(start, __phys_to_virt(start), kernel_x_start - start,
arch/arm64/mm/mmu.c static void __init map_mem(void) · line 396
Quoted 10 lines
arch/arm64/mm/mmu.c +10
-0+ prot_sect_kernel | PMD_SECT_PXN, pgprot_default | PTE_PXN); +create_mapping (kernel_x_start,__phys_to_virt (kernel_x_start), kernel_x_end - kernel_x_start, + prot_sect_kernel, pgprot_default); +if (kernel_x_end < end) +create_mapping (kernel_x_end,__phys_to_virt (kernel_x_end), end - kernel_x_end, + prot_sect_kernel |PMD_SECT_PXN , pgprot_default |PTE_PXN ); + + + } +
Could a config option perhaps be made available for people who are happy to run with the simpler mappings? create_mapping(start, __phys_to_virt(start), end - start);
Quoted 66 lines
arch/arm64/mm/mmu.c +49
-1}/* Limit no longer required. */ memblock_set_current_limit (MEMBLOCK_ALLOC_ANYWHERE ); } +void __init fixup_executable (void ) +{ +/* now that we are actually fully mapped, make the start/end more fine grained */ +if (!IS_ALIGNED ((unsigned long )_stext,SECTION_SIZE )) { +unsigned long aligned_start =round_down (__pa (_stext),SECTION_SIZE ); + +create_mapping (aligned_start,__phys_to_virt (aligned_start), +__pa (_stext) - aligned_start, + prot_sect_kernel |PMD_SECT_PXN , + pgprot_default |PTE_PXN ); + } + +if (!IS_ALIGNED ((unsigned long )__init_end,SECTION_SIZE )) { +unsigned long aligned_end =round_up (__pa (__init_end),SECTION_SIZE ); +create_mapping (__pa (__init_end), (unsigned long )__init_end, + aligned_end -__pa (__init_end), + prot_sect_kernel |PMD_SECT_PXN , pgprot_default |PTE_PXN ); + } +} + +#ifdef CONFIG_DEBUG_RODATA +void mark_rodata_ro (void ) +{ +create_mapping_late (__pa (_stext), (unsigned long )_stext, (unsigned long )_etext - (unsigned long )_stext, + prot_sect_kernel |PMD_SECT_RDONLY , + pgprot_default |PTE_RDONLY ); + +} +#endif + +static int __flush_mappings (void *unused) +{ +flush_tlb_kernel_range ((unsigned long )__init_begin, (unsigned long )__init_end); +return 0 ; +} + +void __ref fixup_init (void ) +{ +phys_addr_t start =__pa (__init_begin); +phys_addr_t end =__pa (__init_end); + +create_mapping_late (start, (unsigned long )__init_begin, + end - start, + prot_sect_kernel |PMD_SECT_PXN , pgprot_default |PTE_PXN ); +if (!IS_ALIGNED (start,SECTION_SIZE ) || !IS_ALIGNED (end,SECTION_SIZE )) +stop_machine (__flush_mappings,NULL ,NULL ); +} +/* * paging_init() sets up the page tables, initialises the zone memory * maps and sets up the zero page. @@ -317,6 +457,7 @@ void __init paging_init(void)void *zero_page;map_mem (); +fixup_executable ();/* * Finally flush the caches and tlb to ensure that we're in a --
Quoted 3 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Cheers,
--
Steve