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

On Fri, May 7, 2021 at 3:35 PM Christophe Leroy
[off-list ref] wrote:

Le 06/05/2021 à 04:34, Jordan Niethe a écrit :

quoted

From: Russell Currey <redacted>

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 <redacted>
[jpn: - predicate on !PPC_BOOK3S_604
       - make module_alloc() use PAGE_KERNEL protection]
Signed-off-by: Jordan Niethe <redacted>
---
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.

Thanks, I will change to that.

quoted

      /*
       * 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));
  }