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

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:   Thu, 11 Aug 2022 10:09:42 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     liaochang1@...wei.com
CC:     Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu,
        mhiramat@...nel.org, rostedt@...dmis.org, liaochang1@...wei.com,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject:     Re: [PATCH] riscv/kprobes: allocate detour buffer from module area

On Tue, 28 Jun 2022 18:13:17 PDT (-0700), liaochang1@...wei.com 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@...wei.com>
> ---
>  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.