Re: [PATCH RESEND v8 11/16] powerpc: extend execmem_params for kprobes allocations

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Hi Mike,

Looks like I missed this change ...

Le 05/05/2024 à 18:06, Mike Rapoport a écrit :
> From: "Mike Rapoport (IBM)" <rppt@kernel.org>
> 
> powerpc overrides kprobes::alloc_insn_page() to remove writable
> permissions when STRICT_MODULE_RWX is on.
> 
> Add definition of EXECMEM_KRPOBES to execmem_params to allow using the
> generic kprobes::alloc_insn_page() with the desired permissions.
> 
> As powerpc uses breakpoint instructions to inject kprobes, it does not
> need to constrain kprobe allocations to the modules area and can use the
> entire vmalloc address space.

Using breakpoint means the segment must still be executable.
On book3s/32 VMALLOC segment is not executable, while module segment is.

See commit 6ca055322da8 ("powerpc/32s: Use dedicated segment for modules 
with STRICT_KERNEL_RWX")

See report and fix at 
https://lore.kernel.org/all/ed03d290d440d579a81ae0b7de5243721291d903.1794543909.git.ritesh.list@gmail.com/

Christophe



> 
> Signed-off-by: Mike Rapoport (IBM) <rppt@kernel.org>
> ---
>   arch/powerpc/kernel/kprobes.c | 20 --------------------
>   arch/powerpc/kernel/module.c  |  7 +++++++
>   2 files changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 9fcd01bb2ce6..14c5ddec3056 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -126,26 +126,6 @@ kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offse
>   	return (kprobe_opcode_t *)(addr + offset);
>   }
>   
> -void *alloc_insn_page(void)
> -{
> -	void *page;
> -
> -	page = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
> -	if (!page)
> -		return NULL;
> -
> -	if (strict_module_rwx_enabled()) {
> -		int err = set_memory_rox((unsigned long)page, 1);
> -
> -		if (err)
> -			goto error;
> -	}
> -	return page;
> -error:
> -	execmem_free(page);
> -	return NULL;
> -}
> -
>   int arch_prepare_kprobe(struct kprobe *p)
>   {
>   	int ret = 0;
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index ac80559015a3..2a23cf7e141b 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -94,6 +94,7 @@ static struct execmem_info execmem_info __ro_after_init;
>   
>   struct execmem_info __init *execmem_arch_setup(void)
>   {
> +	pgprot_t kprobes_prot = strict_module_rwx_enabled() ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
>   	pgprot_t prot = strict_module_rwx_enabled() ? PAGE_KERNEL : PAGE_KERNEL_EXEC;
>   	unsigned long fallback_start = 0, fallback_end = 0;
>   	unsigned long start, end;
> @@ -132,6 +133,12 @@ struct execmem_info __init *execmem_arch_setup(void)
>   				.fallback_start	= fallback_start,
>   				.fallback_end	= fallback_end,
>   			},
> +			[EXECMEM_KPROBES] = {
> +				.start	= VMALLOC_START,
> +				.end	= VMALLOC_END,
> +				.pgprot	= kprobes_prot,
> +				.alignment = 1,
> +			},
>   			[EXECMEM_MODULE_DATA] = {
>   				.start	= VMALLOC_START,
>   				.end	= VMALLOC_END,