fix a hyperv W^X violation and remove vmalloc_exec
Subsystems (7)
Hi all,
Dexuan reported a W^X violation due to the fact that the hyper hypercall page due switching it to be allocated using vmalloc_exec. The problem is that PAGE_KERNEL_EXEC as used by vmalloc_exec actually sets writable permissions in the pte. This series fixes the issue by switching to the low-level __vmalloc_node_range interface that allows specifing more detailed permissions instead. It then also open codes the other two callers and removes the somewhat confusing vmalloc_exec interface.
Peter noted that the hyper hypercall page allocation also has another long standing issue in that it shouldn't use the full vmalloc but just the module space. This issue is so far theoretical as the allocation is done early in the boot process. I plan to fix it with another bigger series for 5.9.
PATCH 1/3 x86/hyperv: allocate the hypercall page with only read and execute bits
Avoid a W^X violation cause by the fact that PAGE_KERNEL_EXEC includes the writable bit.
For this resurrect the removed PAGE_KERNEL_RX definitіon, but as PAGE_KERNEL_ROX to match arm64 and powerpc.
--- arch/x86/hyperv/hv_init.c | 4 +++- arch/x86/include/asm/pgtable_types.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-)
arch/x86/hyperv/hv_init.c +3 -1
@@ -375,7 +375,9 @@ void __init hyperv_init(void) guest_id =generate_guest_id (0 ,LINUX_VERSION_CODE ,0 );wrmsrl (HV_X64_MSR_GUEST_OS_ID , guest_id); - hv_hypercall_pg =vmalloc_exec (PAGE_SIZE ); + hv_hypercall_pg =__vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START , +VMALLOC_END ,GFP_KERNEL ,PAGE_KERNEL_ROX , +VM_FLUSH_RESET_PERMS ,NUMA_NO_NODE , __func__);if (hv_hypercall_pg ==NULL ) {wrmsrl (HV_X64_MSR_GUEST_OS_ID ,0 ); goto remove_cpuhp_state;
arch/x86/include/asm/pgtable_types.h +2 -0
@@ -194,6 +194,7 @@ enum page_cache_mode {#define _PAGE_TABLE_NOENC (__PP|__RW|_USR|___A| 0|___D| 0| 0)#define _PAGE_TABLE (__PP|__RW|_USR|___A| 0|___D| 0| 0| _ENC)#define __PAGE_KERNEL_RO (__PP| 0| 0|___A|__NX|___D| 0|___G) +#define __PAGE_KERNEL_ROX (__PP| 0| 0|___A| 0|___D| 0|___G)#define __PAGE_KERNEL_NOCACHE (__PP|__RW| 0|___A|__NX|___D| 0|___G| __NC)#define __PAGE_KERNEL_VVAR (__PP| 0|_USR|___A|__NX|___D| 0|___G)#define __PAGE_KERNEL_LARGE (__PP|__RW| 0|___A|__NX|___D|_PSE|___G)
@@ -219,6 +220,7 @@ enum page_cache_mode {#define PAGE_KERNEL_RO __pgprot_mask(__PAGE_KERNEL_RO | _ENC)#define PAGE_KERNEL_EXEC __pgprot_mask(__PAGE_KERNEL_EXEC | _ENC)#define PAGE_KERNEL_EXEC_NOENC __pgprot_mask(__PAGE_KERNEL_EXEC | 0) +#define PAGE_KERNEL_ROX __pgprot_mask(__PAGE_KERNEL_ROX | _ENC)#define PAGE_KERNEL_NOCACHE __pgprot_mask(__PAGE_KERNEL_NOCACHE | _ENC)#define PAGE_KERNEL_LARGE __pgprot_mask(__PAGE_KERNEL_LARGE | _ENC)#define PAGE_KERNEL_LARGE_EXEC __pgprot_mask(__PAGE_KERNEL_LARGE_EXEC | _ENC)
Signature · 1 line
2.26.2
Re: [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits
Quoted 10 lines · Christoph Hellwig
Avoid a W^X violation cause by the fact that
PAGE_KERNEL_EXECincludes the writable bit.For this resurrect the removed
PAGE_KERNEL_RXdefinitіon, but asPAGE_KERNEL_ROXto match arm64 and powerpc.Fixes 78bb17f76edc ("x86/hyperv: use vmalloc_exec for the hypercall page")Reported-by Dexuan Cui <[email protected]>Signed-off-by Christoph Hellwig <[email protected]>Tested-by Vitaly Kuznetsov <[email protected]>
PATCH 2/3 arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
In reply to Christoph HellwigUse PAGE_KERNEL_ROX directly instead of allocating RWX and setting the page read-only just after the allocation.
--- arch/arm64/kernel/probes/kprobes.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
arch/arm64/kernel/probes/kprobes.c +3 -9
@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; - - page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }/* arm kprobe: install breakpoint in text */
Signature · 1 line
2.26.2
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 18 lines · Christoph Hellwig
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.Signed-off-by Christoph Hellwig <[email protected]>--- arch/arm64/kernel/probes/kprobes.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)arch/arm64/kernel/probes/kprobes.c +0
-2@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; -
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 125
Quoted 10 lines
arch/arm64/kernel/probes/kprobes.c +3
-7- page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__);
I do wonder if something like vmalloc_prot(size, prot) would make this (and the other two users) easier to read.
So instead of ripping out vmalloc_exec(), converting it into vmalloc_prot() instead.
Did you consider that?
Apart from that LGTM
Quoted 4 lines
arch/arm64/kernel/probes/kprobes.c +0
-0}/* arm kprobe: install breakpoint in text */
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 33 lines · David Hildenbrand
Quoted 18 lines · Christoph Hellwig
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.Signed-off-by Christoph Hellwig <[email protected]>--- arch/arm64/kernel/probes/kprobes.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)arch/arm64/kernel/probes/kprobes.c +0
-2@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; -arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 125
Quoted 10 lines
arch/arm64/kernel/probes/kprobes.c +3
-7- page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__);I do wonder if something like
vmalloc_prot(size, prot)would make this (and the other two users) easier to read.
Quoted 4 lines
So instead of ripping out
vmalloc_exec(), converting it intovmalloc_prot()instead.Did you consider that?
For x86 Christoph did module_alloc_prot(), which is in his more extensive set of patches addressing this. I suspect that would be the right thing for ARM64 as well.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 34 lines · Peter Zijlstra
Quoted 33 lines · David Hildenbrand
Quoted 18 lines · Christoph Hellwig
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.Signed-off-by Christoph Hellwig <[email protected]>--- arch/arm64/kernel/probes/kprobes.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)arch/arm64/kernel/probes/kprobes.c +0
-2@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; -arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 125
Quoted 10 lines
arch/arm64/kernel/probes/kprobes.c +3
-7- page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__);I do wonder if something like
vmalloc_prot(size, prot)would make this (and the other two users) easier to read.
Quoted 8 lines
Quoted 4 lines
So instead of ripping out
vmalloc_exec(), converting it intovmalloc_prot()instead.Did you consider that?
For x86 Christoph did
module_alloc_prot(), which is in his more extensive set of patches addressing this. I suspect that would be the right thing for ARM64 as well.
Yes. The somewhat hacky way I added it cause problems for UML, so I instead plan to do a series converting all architectures over to module_alloc_prot, plus lots of other cleanups in the area that I noticed.
I don't think vmalloc_prot is a good idea per se, as there only few potential users, and I don't want too many vmalloc APIs.
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
In reply to Christoph HellwigQuoted 19 lines · Christoph Hellwig
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.Signed-off-by Christoph Hellwig <[email protected]>--- arch/arm64/kernel/probes/kprobes.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)arch/arm64/kernel/probes/kprobes.c +0
-3@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; - - page =vmalloc_exec (PAGE_SIZE );
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 126
Quoted 10 lines
arch/arm64/kernel/probes/kprobes.c +3
-6-if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }
I think this has the exact same range issue as the x86 user. But it might be less fatal if their PLT magic can cover the full range.
Andrew Morton
maintainerRe: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
In reply to Christoph HellwigQuoted 14 lines · On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <[email protected]> wrote:
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.arch/arm64/kernel/probes/kprobes.c +0
-5@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; - - page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 );
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 128
Quoted 10 lines
arch/arm64/kernel/probes/kprobes.c +3
-4-set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }/* arm kprobe: install breakpoint in text */
But why. I think this is just a cleanup, doesn't address any runtime issue?
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 25 lines · Andrew Morton
Quoted 23 lines · On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <[email protected]> wrote:
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.arch/arm64/kernel/probes/kprobes.c +3
-9@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; - - page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }
Quoted 3 lines
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 128
Quoted 1 line
arch/arm64/kernel/probes/kprobes.c +0
-0/* arm kprobe: install breakpoint in text */ But why. I think this is just a cleanup, doesn't address any runtime issue?
It doesn't "fix" an issue - it just simplifies and speeds up the code.
Will Deacon
maintainerRe: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 26 lines · Christoph Hellwig
Quoted 25 lines · Andrew Morton
Quoted 23 lines · On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <[email protected]> wrote:
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.arch/arm64/kernel/probes/kprobes.c +3
-9@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; - - page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }
Quoted 5 lines
Quoted 3 lines
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 128
Quoted 1 line
arch/arm64/kernel/probes/kprobes.c +0
-0/* arm kprobe: install breakpoint in text */ But why. I think this is just a cleanup, doesn't address any runtime issue?
It doesn't "fix" an issue - it just simplifies and speeds up the code.
Ok, but I don't understand the PLT comment from Peter in [email protected]:
| I think this has the exact same range issue as the x86 user. But it | might be less fatal if their PLT magic can cover the full range.
Peter, please could you elaborate on your concern? I feel like I'm missing some context.
Cheers,
Will
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 33 lines · Will Deacon
Quoted 26 lines · Christoph Hellwig
Quoted 25 lines · Andrew Morton
Quoted 23 lines · On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <[email protected]> wrote:
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.arch/arm64/kernel/probes/kprobes.c +3
-9@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; - - page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }Quoted 5 lines
Quoted 3 lines
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 128
Quoted 1 line
arch/arm64/kernel/probes/kprobes.c +0
-0/* arm kprobe: install breakpoint in text */ But why. I think this is just a cleanup, doesn't address any runtime issue?
It doesn't "fix" an issue - it just simplifies and speeds up the code.
Quoted 8 lines
Ok, but I don't understand the PLT comment from Peter in [email protected]:
| I think this has the exact same range issue as the x86 user. But it | might be less fatal if their PLT magic can cover the full range.
Peter, please could you elaborate on your concern? I feel like I'm missing some context.
On x86 we can only directly call code in a (signed) 32bit immediate range (2G) and our kernel text and module range are constrained by that.
IIRC ARM64 has an even smaller immediate range and needs to play fixup games with trampolines or somesuch (there was an ARM specific name for it that I've misplaced again). Does that machinery cover the entire vmalloc space or are you only able to fix up for a smaller range?
Your arch/arm64/kernel/module.c:module_alloc() implementation seems to have an explicit module range different from the full vmalloc range, I'm thinking this is for a reason.
Will Deacon
maintainerRe: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 42 lines · Peter Zijlstra
Quoted 29 lines · Will Deacon
Quoted 22 lines · Christoph Hellwig
Quoted 21 lines · Andrew Morton
Quoted 20 lines · On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <[email protected]> wrote:
arch/arm64/kernel/probes/kprobes.c +3
-9@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; - - page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }Quoted 5 lines
Quoted 3 lines
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 128
Quoted 1 line
arch/arm64/kernel/probes/kprobes.c +0
-0/* arm kprobe: install breakpoint in text */ But why. I think this is just a cleanup, doesn't address any runtime issue?
It doesn't "fix" an issue - it just simplifies and speeds up the code.
Quoted 8 lines
Ok, but I don't understand the PLT comment from Peter in [email protected]:
| I think this has the exact same range issue as the x86 user. But it | might be less fatal if their PLT magic can cover the full range.
Peter, please could you elaborate on your concern? I feel like I'm missing some context.
On x86 we can only directly call code in a (signed) 32bit immediate range (2G) and our kernel text and module range are constrained by that.
Quoted 8 lines
IIRC ARM64 has an even smaller immediate range and needs to play fixup games with trampolines or somesuch (there was an ARM specific name for it that I've misplaced again). Does that machinery cover the entire vmalloc space or are you only able to fix up for a smaller range?
Your arch/arm64/kernel/module.c:module_alloc() implementation seems to have an explicit module range different from the full vmalloc range, I'm thinking this is for a reason.
Ah, gotcha. In this case, we're talking about the kprobe out-of-line buffer. We don't directly branch to that; instead we take a BRK exception and either exception return + singlestep the OOL buffer, or we simulate the instruction if it's doing anything PC-relative, so I don't see the need for a PLT.
Will
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
In reply to Christoph HellwigQuoted 19 lines · Christoph Hellwig
Use
PAGE_KERNEL_ROXdirectly instead of allocating RWX and setting the page read-only just after the allocation.Signed-off-by Christoph Hellwig <[email protected]>--- arch/arm64/kernel/probes/kprobes.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)arch/arm64/kernel/probes/kprobes.c +0
-2@@ -120,15 +120,9 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)void *alloc_insn_page (void ) { -void *page; -
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 125
Quoted 10 lines
arch/arm64/kernel/probes/kprobes.c +3
-7- page =vmalloc_exec (PAGE_SIZE ); -if (page) { -set_memory_ro ((unsigned long )page,1 ); -set_vm_flush_reset_perms (page); - } - -return page; +return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__);
Why is this passing a string for the 'caller' argument, and not the address of the caller?
Quoted 11 lines
arch/arm64/kernel/probes/kprobes.c +0
-1}/* arm kprobe: install breakpoint in text */ --2.26.2
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 6 lines · Ard Biesheuvel
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 125
Quoted 3 lines
arch/arm64/kernel/probes/kprobes.c +3
-0+return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__);Why is this passing a string for the 'caller' argument, and not the address of the caller?
Dementia. Fix sent.
Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
Quoted 10 lines · Christoph Hellwig
Quoted 6 lines · Ard Biesheuvel
arch/arm64/kernel/probes/kprobes.c int __kprobes arch_prepare_kprobe(struct kprobe *p) · line 125
Quoted 3 lines
arch/arm64/kernel/probes/kprobes.c +3
-0+return __vmalloc_node_range (PAGE_SIZE ,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_ROX ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__);Why is this passing a string for the 'caller' argument, and not the address of the caller?
Dementia. Fix sent.
:-)
PATCH 3/3 mm: remove vmalloc_exec
In reply to Christoph HellwigMerge vmalloc_exec into its only caller. Note that for !CONFIG_MMU __vmalloc_node_range maps to __vmalloc, which directly clears the __GFP_HIGHMEM added by the vmalloc_exec stub anyway.
--- include/linux/vmalloc.h | 1 - kernel/module.c | 4 +++- mm/nommu.c | 17 ----------------- mm/vmalloc.c | 20 -------------------- 4 files changed, 3 insertions(+), 39 deletions(-)
include/linux/vmalloc.h +0 -1
@@ -106,7 +106,6 @@ extern void *vzalloc(unsigned long size);extern void *vmalloc_user (unsigned long size);extern void *vmalloc_node (unsigned long size,int node);extern void *vzalloc_node (unsigned long size,int node); -extern void *vmalloc_exec (unsigned long size);extern void *vmalloc_32 (unsigned long size);extern void *vmalloc_32_user (unsigned long size);extern void *__vmalloc (unsigned long size,gfp_t gfp_mask);
kernel/module.c +3 -1
@@ -2783,7 +2783,9 @@ static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)void *__weak module_alloc (unsigned long size) { -return vmalloc_exec (size); +return __vmalloc_node_range (size,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_EXEC ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }bool __weak module_init_section (const char *name)
mm/nommu.c +0 -17
@@ -290,23 +290,6 @@ void *vzalloc_node(unsigned long size, int node) }EXPORT_SYMBOL (vzalloc_node); -/** -* vmalloc_exec - allocate virtually contiguous, executable memory -* @size: allocation size -* -* Kernel-internal function to allocate enough pages to cover @size -* the page level allocator and map them into contiguous and -* executable kernel virtual space. -* -* For tight control over page level allocator and protection flags -* use __vmalloc() instead. -*/ - -void *vmalloc_exec (unsigned long size) -{ -return __vmalloc (size,GFP_KERNEL | __GFP_HIGHMEM); -} -/** * vmalloc_32 - allocate virtually contiguous memory (32bit addressable) * @size: allocation size
mm/vmalloc.c +0 -20
@@ -2696,26 +2696,6 @@ void *vzalloc_node(unsigned long size, int node) }EXPORT_SYMBOL (vzalloc_node); -/** -* vmalloc_exec - allocate virtually contiguous, executable memory -* @size: allocation size -* -* Kernel-internal function to allocate enough pages to cover @size -* the page level allocator and map them into contiguous and -* executable kernel virtual space. -* -* For tight control over page level allocator and protection flags -* use __vmalloc() instead. -* -* Return: pointer to the allocated memory or %NULL on error -*/ -void *vmalloc_exec (unsigned long size) -{ -return __vmalloc_node_range (size,1 ,VMALLOC_START ,VMALLOC_END , -GFP_KERNEL ,PAGE_KERNEL_EXEC ,VM_FLUSH_RESET_PERMS , -NUMA_NO_NODE ,__builtin_return_address (0 )); -} -#if defined(CONFIG_64BIT ) && defined(CONFIG_ZONE_DMA32 )#define GFP_VMALLOC32 (GFP_DMA32 | GFP_KERNEL)#elif defined(CONFIG_64BIT ) && defined(CONFIG_ZONE_DMA )
Signature · 1 line
2.26.2
Re: [PATCH 3/3] mm: remove vmalloc_exec
Quoted 89 lines · Christoph Hellwig
Merge
vmalloc_execinto its only caller. Note that for !CONFIG_MMU__vmalloc_node_rangemaps to__vmalloc, which directly clears the__GFP_HIGHMEMadded by thevmalloc_execstub anyway.Signed-off-by Christoph Hellwig <[email protected]>--- include/linux/vmalloc.h | 1 - kernel/module.c | 4 +++- mm/nommu.c | 17 ----------------- mm/vmalloc.c | 20 -------------------- 4 files changed, 3 insertions(+), 39 deletions(-)include/linux/vmalloc.h +0
-1@@ -106,7 +106,6 @@ extern void *vzalloc(unsigned long size);extern void *vmalloc_user (unsigned long size);extern void *vmalloc_node (unsigned long size,int node);extern void *vzalloc_node (unsigned long size,int node); -extern void *vmalloc_exec (unsigned long size);extern void *vmalloc_32 (unsigned long size);extern void *vmalloc_32_user (unsigned long size);extern void *__vmalloc (unsigned long size,gfp_t gfp_mask);kernel/module.c +3
-1@@ -2783,7 +2783,9 @@ static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)void *__weak module_alloc (unsigned long size) { -return vmalloc_exec (size); +return __vmalloc_node_range (size,1 ,VMALLOC_START ,VMALLOC_END , +GFP_KERNEL ,PAGE_KERNEL_EXEC ,VM_FLUSH_RESET_PERMS , +NUMA_NO_NODE , __func__); }bool __weak module_init_section (const char *name)mm/nommu.c +0
-17@@ -290,23 +290,6 @@ void *vzalloc_node(unsigned long size, int node) }EXPORT_SYMBOL (vzalloc_node); -/** -* vmalloc_exec - allocate virtually contiguous, executable memory -* @size: allocation size -* -* Kernel-internal function to allocate enough pages to cover @size -* the page level allocator and map them into contiguous and -* executable kernel virtual space. -* -* For tight control over page level allocator and protection flags -* use __vmalloc() instead. -*/ - -void *vmalloc_exec (unsigned long size) -{ -return __vmalloc (size,GFP_KERNEL | __GFP_HIGHMEM); -} -/** * vmalloc_32 - allocate virtually contiguous memory (32bit addressable) * @size: allocation size mm/vmalloc.c +0
-13@@ -2696,26 +2696,6 @@ void *vzalloc_node(unsigned long size, int node) }EXPORT_SYMBOL (vzalloc_node); -/** -* vmalloc_exec - allocate virtually contiguous, executable memory -* @size: allocation size -* -* Kernel-internal function to allocate enough pages to cover @size -* the page level allocator and map them into contiguous and -* executable kernel virtual space. -* -* For tight control over page level allocator and protection flags -* use __vmalloc() instead. -* -* Return: pointer to the allocated memory or %NULL on error -*/
mm/vmalloc.c void *vzalloc_node(unsigned long size, int node) · line 2701
Quoted 10 lines
mm/vmalloc.c +0
-7-void *vmalloc_exec (unsigned long size) -{ -return __vmalloc_node_range (size,1 ,VMALLOC_START ,VMALLOC_END , -GFP_KERNEL ,PAGE_KERNEL_EXEC ,VM_FLUSH_RESET_PERMS , -NUMA_NO_NODE ,__builtin_return_address (0 )); -} -#if defined(CONFIG_64BIT ) && defined(CONFIG_ZONE_DMA32 )#define GFP_VMALLOC32 (GFP_DMA32 | GFP_KERNEL)#elif defined(CONFIG_64BIT ) && defined(CONFIG_ZONE_DMA )
LGTM
Signature · 3 lines
Thanks, David / dhildenb
Peter Zijlstra
Acked-byQuoted 9 lines · Christoph Hellwig
Hi all,
Dexuan reported a W^X violation due to the fact that the hyper hypercall page due switching it to be allocated using
vmalloc_exec. The problem is thatPAGE_KERNEL_EXECas used byvmalloc_execactually sets writable permissions in the pte. This series fixes the issue by switching to the low-level__vmalloc_node_rangeinterface that allows specifing more detailed permissions instead. It then also open codes the other two callers and removes the somewhat confusingvmalloc_execinterface.
Quoted 5 lines
Peter noted that the hyper hypercall page allocation also has another long standing issue in that it shouldn't use the full vmalloc but just the module space. This issue is so far theoretical as the allocation is done early in the boot process. I plan to fix it with another bigger series for 5.9.
Thanks!
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel