Re: [PATCH v12 6/8] powerpc: Set ARCH_HAS_STRICT_MODULE_RWX

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 06/05/2021 à 04:34, Jordan Niethe a écrit :
> From: Russell Currey <ruscur@russell.cc>
> 
> To enable strict module RWX on powerpc, set:
> 
>      CONFIG_STRICT_MODULE_RWX=y
> 
> You should also have CONFIG_STRICT_KERNEL_RWX=y set to have any real
> security benefit.
> 
> ARCH_HAS_STRICT_MODULE_RWX is set to require ARCH_HAS_STRICT_KERNEL_RWX.
> This is due to a quirk in arch/Kconfig and arch/powerpc/Kconfig that
> makes STRICT_MODULE_RWX *on by default* in configurations where
> STRICT_KERNEL_RWX is *unavailable*.
> 
> Since this doesn't make much sense, and module RWX without kernel RWX
> doesn't make much sense, having the same dependencies as kernel RWX
> works around this problem.
> 
> With STRICT_MODULE_RWX, now make module_alloc() allocate pages with
> KERNEL_PAGE protection rather than KERNEL_PAGE_EXEC.
> 
> Book32s/32 processors with a hash mmu (i.e. 604 core) can not set memory
> protection on a page by page basis so do not enable.
> 
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> [jpn: - predicate on !PPC_BOOK3S_604
>        - make module_alloc() use PAGE_KERNEL protection]
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v10: - Predicate on !PPC_BOOK3S_604
>       - Make module_alloc() use PAGE_KERNEL protection
> v11: - Neaten up
> ---
>   arch/powerpc/Kconfig         | 1 +
>   arch/powerpc/kernel/module.c | 4 +++-
>   2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index cce0a137b046..cb5d9d862c35 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -140,6 +140,7 @@ config PPC
>   	select ARCH_HAS_SCALED_CPUTIME		if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
>   	select ARCH_HAS_SET_MEMORY
>   	select ARCH_HAS_STRICT_KERNEL_RWX	if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
> +	select ARCH_HAS_STRICT_MODULE_RWX	if ARCH_HAS_STRICT_KERNEL_RWX && !PPC_BOOK3S_604
>   	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
>   	select ARCH_HAS_UACCESS_FLUSHCACHE
>   	select ARCH_HAS_COPY_MC			if PPC64
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index 3f35c8d20be7..33e4011228b0 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -92,12 +92,14 @@ int module_finalize(const Elf_Ehdr *hdr,
>   static __always_inline void *
>   __module_alloc(unsigned long size, unsigned long start, unsigned long end)
>   {
> +	pgprot_t prot = IS_ENABLED(CONFIG_STRICT_MODULE_RWX) ? PAGE_KERNEL :
> +							       PAGE_KERNEL_EXEC;

I'm not sure this test is OK, because strict kernel/module rwx can be disabled at boottime.
There is a global variable 'rodata_enabled' to reflect that.

We have a helper in powerpc asm/mmu.h called strict_kernel_rwx_enabled() to check it.


>   	/*
>   	 * Don't do huge page allocations for modules yet until more testing
>   	 * is done. STRICT_MODULE_RWX may require extra work to support this
>   	 * too.
>   	 */
> -	return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, PAGE_KERNEL_EXEC,
> +	return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, prot,
>   				    VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
>   				    NUMA_NO_NODE, __builtin_return_address(0));
>   }
>