linux-kernel - Re: [PATCH] x86/kprobes: Simplify alloc_insn_page() with __vmalloc_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 13 Apr 2021 22:00:30 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Jisheng Zhang <Jisheng.Zhang@...aptics.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        Jiri Olsa <jolsa@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/kprobes: Simplify alloc_insn_page() with
 __vmalloc_node_range

Hi,

On Tue, 13 Apr 2021 18:03:24 +0800
Jisheng Zhang <Jisheng.Zhang@...aptics.com> 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@...aptics.com>
> ---
>  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@...nel.org>