Re: [PATCH] riscv/kprobes: allocate detour buffer from module area

[Palmer Dabbelt]

On Tue, 28 Jun 2022 18:13:17 PDT (-0700), liaochang1@xxxxxxxxxx wrote:
> To address the limitation of PC-relative branch instruction
> on riscv architecture, detour buffer slot is allocated from
> a area, the distance of which from kernel should be less than 4GB.
>
> For the time being, Modules region always live before the kernel.
> But Vmalloc region reside far away from kernel, the distance is
> half of the kernel address space.
>
> Signed-off-by: Liao Chang <liaochang1@xxxxxxxxxx>
> ---
>  arch/riscv/kernel/probes/kprobes.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> index e6e950b7cf32..bc027a663b17 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -6,12 +6,14 @@
>  #include <linux/extable.h>
>  #include <linux/slab.h>
>  #include <linux/stop_machine.h>
> +#include <linux/moduleloader.h>
>  #include <asm/ptrace.h>
>  #include <linux/uaccess.h>
>  #include <asm/sections.h>
>  #include <asm/cacheflush.h>
>  #include <asm/bug.h>
>  #include <asm/patch.h>
> +#include <asm/set_memory.h>
>
>  #include "decode-insn.h"
>
> @@ -86,10 +88,28 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  #ifdef CONFIG_MMU
>  void *alloc_insn_page(void)
>  {
> +#if defined(CONFIG_MODULES) && defined(CONFIG_64BIT)
> +	void *page;
> +
> +	page = module_alloc(PAGE_SIZE);
> +	if (!page)
> +		return NULL;
> +
> +	set_vm_flush_reset_perms(page);
> +	/*
> +	 * First make the page read-only, and only then make it executable to
> +	 * prevent it from being W+X in between.
> +	 */
> +	set_memory_ro((unsigned long)page, 1);
> +	set_memory_x((unsigned long)page, 1);
> +
> +	return page;
> +#else
>  	return  __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
>  				     GFP_KERNEL, PAGE_KERNEL_READ_EXEC,
>  				     VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>  				     __builtin_return_address(0));
> +#endif
>  }
>  #endif

This probably shouldn't depend on CONFIG_MODULES, it's just as broken to 
assume we can jump to the vmalloc region regardless of whether modules 
are enabled or disabled.  We should be able to just allocate from the 
jump-safe region either way.