Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
* fix a hyperv W^X violation and remove vmalloc_exec
@ 2020-06-18  6:43 Christoph Hellwig
  2020-06-18  6:43 ` [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Christoph Hellwig @ 2020-06-18  6:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits
  2020-06-18  6:43 fix a hyperv W^X violation and remove vmalloc_exec Christoph Hellwig
@ 2020-06-18  6:43 ` Christoph Hellwig
  2020-06-18 11:47   ` Wei Liu
  2020-06-18  6:43 ` [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page Christoph Hellwig
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2020-06-18  6:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

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.

Fixes: 78bb17f76edc ("x86/hyperv: use vmalloc_exec for the hypercall page")
Reported-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/hyperv/hv_init.c            | 4 +++-
 arch/x86/include/asm/pgtable_types.h | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index a54c6a401581dd..2bdc72e6890eca 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -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;
diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index 2da1f95b88d761..816b31c685505f 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -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)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-18  6:43 fix a hyperv W^X violation and remove vmalloc_exec Christoph Hellwig
  2020-06-18  6:43 ` [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits Christoph Hellwig
@ 2020-06-18  6:43 ` Christoph Hellwig
  2020-06-18  8:55   ` David Hildenbrand
                     ` (3 more replies)
  2020-06-18  6:43 ` [PATCH 3/3] mm: remove vmalloc_exec Christoph Hellwig
  2020-06-18  9:28 ` fix a hyperv W^X violation and " Peter Zijlstra
  3 siblings, 4 replies; 19+ messages in thread
From: Christoph Hellwig @ 2020-06-18  6:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
page read-only just after the allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/kernel/probes/kprobes.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index d1c95dcf1d7833..cbe49cd117cfec 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -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 */
-- 
2.26.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3/3] mm: remove vmalloc_exec
  2020-06-18  6:43 fix a hyperv W^X violation and remove vmalloc_exec Christoph Hellwig
  2020-06-18  6:43 ` [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits Christoph Hellwig
  2020-06-18  6:43 ` [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page Christoph Hellwig
@ 2020-06-18  6:43 ` Christoph Hellwig
  2020-06-18  8:53   ` David Hildenbrand
  2020-06-18  9:28 ` fix a hyperv W^X violation and " Peter Zijlstra
  3 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2020-06-18  6:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

Merge 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.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/vmalloc.h |  1 -
 kernel/module.c         |  4 +++-
 mm/nommu.c              | 17 -----------------
 mm/vmalloc.c            | 20 --------------------
 4 files changed, 3 insertions(+), 39 deletions(-)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 48bb681e6c2aed..0221f852a7e1a3 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -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);
diff --git a/kernel/module.c b/kernel/module.c
index e8a198588f26ee..0c6573b98c3662 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -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)
diff --git a/mm/nommu.c b/mm/nommu.c
index cdcad5d61dd194..f32a69095d509e 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -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
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 3091c2ca60dfd1..f60948aac0d0e4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -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)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] mm: remove vmalloc_exec
  2020-06-18  6:43 ` [PATCH 3/3] mm: remove vmalloc_exec Christoph Hellwig
@ 2020-06-18  8:53   ` David Hildenbrand
  0 siblings, 0 replies; 19+ messages in thread
From: David Hildenbrand @ 2020-06-18  8:53 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton
  Cc: Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On 18.06.20 08:43, Christoph Hellwig wrote:
> Merge 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.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/vmalloc.h |  1 -
>  kernel/module.c         |  4 +++-
>  mm/nommu.c              | 17 -----------------
>  mm/vmalloc.c            | 20 --------------------
>  4 files changed, 3 insertions(+), 39 deletions(-)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 48bb681e6c2aed..0221f852a7e1a3 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -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);
> diff --git a/kernel/module.c b/kernel/module.c
> index e8a198588f26ee..0c6573b98c3662 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -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)
> diff --git a/mm/nommu.c b/mm/nommu.c
> index cdcad5d61dd194..f32a69095d509e 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -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
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 3091c2ca60dfd1..f60948aac0d0e4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -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)
> 

LGTM

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-18  6:43 ` [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page Christoph Hellwig
@ 2020-06-18  8:55   ` David Hildenbrand
  2020-06-18 10:35     ` Peter Zijlstra
  2020-06-18  9:27   ` Peter Zijlstra
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: David Hildenbrand @ 2020-06-18  8:55 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton
  Cc: Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On 18.06.20 08:43, Christoph Hellwig wrote:
> Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> page read-only just after the allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/arm64/kernel/probes/kprobes.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index d1c95dcf1d7833..cbe49cd117cfec 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -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__);

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

>  }
>  
>  /* arm kprobe: install breakpoint in text */
> 


-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-18  6:43 ` [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page Christoph Hellwig
  2020-06-18  8:55   ` David Hildenbrand
@ 2020-06-18  9:27   ` Peter Zijlstra
  2020-06-21  2:16   ` Andrew Morton
  2020-06-27  7:34   ` Ard Biesheuvel
  3 siblings, 0 replies; 19+ messages in thread
From: Peter Zijlstra @ 2020-06-18  9:27 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, Dexuan Cui, Vitaly Kuznetsov, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On Thu, Jun 18, 2020 at 08:43:06AM +0200, Christoph Hellwig wrote:
> Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> page read-only just after the allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/arm64/kernel/probes/kprobes.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index d1c95dcf1d7833..cbe49cd117cfec 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -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__);
>  }

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: fix a hyperv W^X violation and remove vmalloc_exec
  2020-06-18  6:43 fix a hyperv W^X violation and remove vmalloc_exec Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-06-18  6:43 ` [PATCH 3/3] mm: remove vmalloc_exec Christoph Hellwig
@ 2020-06-18  9:28 ` Peter Zijlstra
  3 siblings, 0 replies; 19+ messages in thread
From: Peter Zijlstra @ 2020-06-18  9:28 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, Dexuan Cui, Vitaly Kuznetsov, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On Thu, Jun 18, 2020 at 08:43:04AM +0200, Christoph Hellwig wrote:
> 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.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> 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!

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-18  8:55   ` David Hildenbrand
@ 2020-06-18 10:35     ` Peter Zijlstra
  2020-06-18 13:50       ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Zijlstra @ 2020-06-18 10:35 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Christoph Hellwig, Andrew Morton, Dexuan Cui, Vitaly Kuznetsov,
	Catalin Marinas, Will Deacon, Jessica Yu, x86, linux-arm-kernel,
	linux-kernel, linux-hyperv, linux-mm

On Thu, Jun 18, 2020 at 10:55:58AM +0200, David Hildenbrand wrote:
> On 18.06.20 08:43, Christoph Hellwig wrote:
> > Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> > page read-only just after the allocation.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  arch/arm64/kernel/probes/kprobes.c | 12 +++---------
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> > index d1c95dcf1d7833..cbe49cd117cfec 100644
> > --- a/arch/arm64/kernel/probes/kprobes.c
> > +++ b/arch/arm64/kernel/probes/kprobes.c
> > @@ -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__);
> 
> 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?

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.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits
  2020-06-18  6:43 ` [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits Christoph Hellwig
@ 2020-06-18 11:47   ` Wei Liu
  0 siblings, 0 replies; 19+ messages in thread
From: Wei Liu @ 2020-06-18 11:47 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Jessica Yu, x86, linux-arm-kernel,
	linux-kernel, linux-hyperv, linux-mm, Wei Liu

On Thu, Jun 18, 2020 at 08:43:05AM +0200, Christoph Hellwig wrote:
> 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.
> 
> Fixes: 78bb17f76edc ("x86/hyperv: use vmalloc_exec for the hypercall page")
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Acked-by: Wei Liu <wei.liu@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-18 10:35     ` Peter Zijlstra
@ 2020-06-18 13:50       ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2020-06-18 13:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Hildenbrand, Christoph Hellwig, Andrew Morton, Dexuan Cui,
	Vitaly Kuznetsov, Catalin Marinas, Will Deacon, Jessica Yu, x86,
	linux-arm-kernel, linux-kernel, linux-hyperv, linux-mm

On Thu, Jun 18, 2020 at 12:35:06PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 18, 2020 at 10:55:58AM +0200, David Hildenbrand wrote:
> > On 18.06.20 08:43, Christoph Hellwig wrote:
> > > Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> > > page read-only just after the allocation.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  arch/arm64/kernel/probes/kprobes.c | 12 +++---------
> > >  1 file changed, 3 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> > > index d1c95dcf1d7833..cbe49cd117cfec 100644
> > > --- a/arch/arm64/kernel/probes/kprobes.c
> > > +++ b/arch/arm64/kernel/probes/kprobes.c
> > > @@ -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__);
> > 
> > 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?
> 
> 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-18  6:43 ` [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page Christoph Hellwig
  2020-06-18  8:55   ` David Hildenbrand
  2020-06-18  9:27   ` Peter Zijlstra
@ 2020-06-21  2:16   ` Andrew Morton
  2020-06-23  9:05     ` Christoph Hellwig
  2020-06-27  7:34   ` Ard Biesheuvel
  3 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2020-06-21  2:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <hch@lst.de> wrote:

> Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> page read-only just after the allocation.
> 
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -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 */

But why.  I think this is just a cleanup, doesn't address any runtime issue?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-21  2:16   ` Andrew Morton
@ 2020-06-23  9:05     ` Christoph Hellwig
  2020-06-23  9:07       ` Will Deacon
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2020-06-23  9:05 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Jessica Yu, x86, linux-arm-kernel,
	linux-kernel, linux-hyperv, linux-mm

On Sat, Jun 20, 2020 at 07:16:16PM -0700, Andrew Morton wrote:
> On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <hch@lst.de> wrote:
> 
> > Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> > page read-only just after the allocation.
> > 
> > --- a/arch/arm64/kernel/probes/kprobes.c
> > +++ b/arch/arm64/kernel/probes/kprobes.c
> > @@ -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 */
> 
> 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-23  9:05     ` Christoph Hellwig
@ 2020-06-23  9:07       ` Will Deacon
  2020-06-23  9:37         ` Peter Zijlstra
  0 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2020-06-23  9:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, Dexuan Cui, Vitaly Kuznetsov, Peter Zijlstra,
	Catalin Marinas, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On Tue, Jun 23, 2020 at 11:05:05AM +0200, Christoph Hellwig wrote:
> On Sat, Jun 20, 2020 at 07:16:16PM -0700, Andrew Morton wrote:
> > On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <hch@lst.de> wrote:
> > 
> > > Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> > > page read-only just after the allocation.
> > > 
> > > --- a/arch/arm64/kernel/probes/kprobes.c
> > > +++ b/arch/arm64/kernel/probes/kprobes.c
> > > @@ -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 */
> > 
> > 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
20200618092754.GF576905@hirez.programming.kicks-ass.net:

  | 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-23  9:07       ` Will Deacon
@ 2020-06-23  9:37         ` Peter Zijlstra
  2020-06-23  9:57           ` Will Deacon
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Zijlstra @ 2020-06-23  9:37 UTC (permalink / raw)
  To: Will Deacon
  Cc: Christoph Hellwig, Andrew Morton, Dexuan Cui, Vitaly Kuznetsov,
	Catalin Marinas, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On Tue, Jun 23, 2020 at 10:07:58AM +0100, Will Deacon wrote:
> On Tue, Jun 23, 2020 at 11:05:05AM +0200, Christoph Hellwig wrote:
> > On Sat, Jun 20, 2020 at 07:16:16PM -0700, Andrew Morton wrote:
> > > On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <hch@lst.de> wrote:
> > > 
> > > > Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> > > > page read-only just after the allocation.
> > > > 
> > > > --- a/arch/arm64/kernel/probes/kprobes.c
> > > > +++ b/arch/arm64/kernel/probes/kprobes.c
> > > > @@ -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 */
> > > 
> > > 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
> 20200618092754.GF576905@hirez.programming.kicks-ass.net:
> 
>   | 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-23  9:37         ` Peter Zijlstra
@ 2020-06-23  9:57           ` Will Deacon
  0 siblings, 0 replies; 19+ messages in thread
From: Will Deacon @ 2020-06-23  9:57 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Christoph Hellwig, Andrew Morton, Dexuan Cui, Vitaly Kuznetsov,
	Catalin Marinas, Jessica Yu, x86, linux-arm-kernel, linux-kernel,
	linux-hyperv, linux-mm

On Tue, Jun 23, 2020 at 11:37:14AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 23, 2020 at 10:07:58AM +0100, Will Deacon wrote:
> > On Tue, Jun 23, 2020 at 11:05:05AM +0200, Christoph Hellwig wrote:
> > > On Sat, Jun 20, 2020 at 07:16:16PM -0700, Andrew Morton wrote:
> > > > On Thu, 18 Jun 2020 08:43:06 +0200 Christoph Hellwig <hch@lst.de> wrote:
> > > > > --- a/arch/arm64/kernel/probes/kprobes.c
> > > > > +++ b/arch/arm64/kernel/probes/kprobes.c
> > > > > @@ -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 */
> > > > 
> > > > 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
> > 20200618092754.GF576905@hirez.programming.kicks-ass.net:
> > 
> >   | 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.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-18  6:43 ` [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page Christoph Hellwig
                     ` (2 preceding siblings ...)
  2020-06-21  2:16   ` Andrew Morton
@ 2020-06-27  7:34   ` Ard Biesheuvel
  2020-06-27  7:56     ` Christoph Hellwig
  3 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2020-06-27  7:34 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, linux-hyperv, Peter Zijlstra, Catalin Marinas,
	X86 ML, Dexuan Cui, Linux Kernel Mailing List, linux-mm,
	Jessica Yu, Vitaly Kuznetsov, Will Deacon, Linux ARM

On Thu, 18 Jun 2020 at 08:44, Christoph Hellwig <hch@lst.de> wrote:
>
> Use PAGE_KERNEL_ROX directly instead of allocating RWX and setting the
> page read-only just after the allocation.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/arm64/kernel/probes/kprobes.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index d1c95dcf1d7833..cbe49cd117cfec 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -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__);

Why is this passing a string for the 'caller' argument, and not the
address of the caller?


>  }
>
>  /* arm kprobe: install breakpoint in text */
> --
> 2.26.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-27  7:34   ` Ard Biesheuvel
@ 2020-06-27  7:56     ` Christoph Hellwig
  2020-06-27  7:57       ` Ard Biesheuvel
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2020-06-27  7:56 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Christoph Hellwig, Andrew Morton, linux-hyperv, Peter Zijlstra,
	Catalin Marinas, X86 ML, Dexuan Cui, Linux Kernel Mailing List,
	linux-mm, Jessica Yu, Vitaly Kuznetsov, Will Deacon, Linux ARM

On Sat, Jun 27, 2020 at 09:34:42AM +0200, Ard Biesheuvel wrote:
> > +       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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page
  2020-06-27  7:56     ` Christoph Hellwig
@ 2020-06-27  7:57       ` Ard Biesheuvel
  0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2020-06-27  7:57 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, linux-hyperv, Peter Zijlstra, Catalin Marinas,
	X86 ML, Dexuan Cui, Linux Kernel Mailing List, linux-mm,
	Jessica Yu, Vitaly Kuznetsov, Will Deacon, Linux ARM

On Sat, 27 Jun 2020 at 09:57, Christoph Hellwig <hch@lst.de> wrote:
>
> On Sat, Jun 27, 2020 at 09:34:42AM +0200, Ard Biesheuvel wrote:
> > > +       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.

:-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, back to index

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18  6:43 fix a hyperv W^X violation and remove vmalloc_exec Christoph Hellwig
2020-06-18  6:43 ` [PATCH 1/3] x86/hyperv: allocate the hypercall page with only read and execute bits Christoph Hellwig
2020-06-18 11:47   ` Wei Liu
2020-06-18  6:43 ` [PATCH 2/3] arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page Christoph Hellwig
2020-06-18  8:55   ` David Hildenbrand
2020-06-18 10:35     ` Peter Zijlstra
2020-06-18 13:50       ` Christoph Hellwig
2020-06-18  9:27   ` Peter Zijlstra
2020-06-21  2:16   ` Andrew Morton
2020-06-23  9:05     ` Christoph Hellwig
2020-06-23  9:07       ` Will Deacon
2020-06-23  9:37         ` Peter Zijlstra
2020-06-23  9:57           ` Will Deacon
2020-06-27  7:34   ` Ard Biesheuvel
2020-06-27  7:56     ` Christoph Hellwig
2020-06-27  7:57       ` Ard Biesheuvel
2020-06-18  6:43 ` [PATCH 3/3] mm: remove vmalloc_exec Christoph Hellwig
2020-06-18  8:53   ` David Hildenbrand
2020-06-18  9:28 ` fix a hyperv W^X violation and " Peter Zijlstra

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org
	public-inbox-index linux-hyperv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git