Re: [PATCH] x86/kprobes: Simplify alloc_insn_page() with __vmalloc_node_range

From: Masami Hiramatsu
Date: Tue Apr 13 2021 - 09:00:36 EST


Hi,

On Tue, 13 Apr 2021 18:03:24 +0800
Jisheng Zhang <Jisheng.Zhang@xxxxxxxxxxxxx> wrote:

> Use the __vmalloc_node_range() to simplify x86's alloc_insn_page()
> implementation.

Have you checked this is equivarent to the original code on
all architecture? IIRC, some arch has a special module_alloc(),
thus I NACKed similar patch previously.

Thank you,

>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@xxxxxxxxxxxxx>
> ---
> arch/x86/kernel/kprobes/core.c | 24 ++++--------------------
> 1 file changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index df776cdca327..75081f3dbe44 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -383,26 +383,10 @@ static int prepare_boost(kprobe_opcode_t *buf, struct kprobe *p,
> /* Make page to RO mode when allocate it */
> void *alloc_insn_page(void)
> {
> - 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);
> -
> - /*
> - * TODO: Once additional kernel code protection mechanisms are set, ensure
> - * that the page was not maliciously altered and it is still zeroed.
> - */
> - set_memory_x((unsigned long)page, 1);
> -
> - return page;
> + return __vmalloc_node_range(PAGE_SIZE, PAGE_SIZE, VMALLOC_START,
> + VMALLOC_END, GFP_KERNEL, PAGE_KERNEL_ROX,
> + VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
> + __builtin_return_address(0));
> }
>
> /* Recover page to RW mode before releasing it */
> --
> 2.31.0
>


--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>