Re: [PATCH] powerpc/mm: Support execute-only memory on the Radix MMU

On Mon, 2022-08-08 at 18:28 +0530, Aneesh Kumar K V wrote:

On 8/8/22 5:28 PM, Russell Currey wrote:

quoted

The Hash MMU already supports XOM (i.e. mmap with PROT_EXEC only)
through the execute-only pkey.  A PROT_ONLY mapping will actually
map to
RX, and then the pkey will be applied on top of it.

Radix doesn't have pkeys, but it does have execute permissions
built-in
to the MMU, so all we have to do to support XOM is expose it.

Signed-off-by: Russell Currey <redacted>
---
quick test:
https://raw.githubusercontent.com/ruscur/junkcode/main/mmap_test.c
I can make it a selftest.

 arch/powerpc/include/asm/book3s/64/radix.h |  3 +++
 arch/powerpc/mm/book3s64/radix_pgtable.c   |  4 ++++
 arch/powerpc/mm/fault.c                    | 10 ++++++++++
 3 files changed, 17 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/64/radix.h

b/arch/powerpc/include/asm/book3s/64/radix.h
index 686001eda936..bf316b773d73 100644

--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h

@@ -19,6 +19,9 @@

 #include <asm/cpu_has_feature.h>
 #endif
 
+/* Execute-only page protections, Hash can use RX + execute-only
pkey */
+#define PAGE_EXECONLY  __pgprot(_PAGE_BASE | _PAGE_EXEC)
+
 /* An empty PTE can still have a R or C writeback */
 #define RADIX_PTE_NONE_MASK            (_PAGE_DIRTY |
_PAGE_ACCESSED)
 

diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c

b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 698274109c91..2edb56169805 100644

--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c

@@ -617,6 +617,10 @@ void __init radix__early_init_mmu(void)

        __pmd_frag_nr = RADIX_PMD_FRAG_NR;
        __pmd_frag_size_shift = RADIX_PMD_FRAG_SIZE_SHIFT;
 
+       /* Radix directly supports execute-only page protections */
+       protection_map[VM_EXEC] = PAGE_EXECONLY;
+       protection_map[VM_EXEC | VM_SHARED] = PAGE_EXECONLY;
+
        radix_init_pgtable();
 
        if (!firmware_has_feature(FW_FEATURE_LPAR)) {

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 014005428687..887c0cc45ca6 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c

@@ -270,6 +270,16 @@ static bool access_error(bool is_write, bool

is_exec, struct vm_area_struct *vma
                return false;
        }
 
+       if (unlikely(!(vma->vm_flags & VM_READ))) {
+               /*
+                * If we're on Radix, then this could be a read
attempt on
+                * execute-only memory.  On other MMUs, an "exec-
only" page
+                * will be given RX flags, so this might be
redundant.
+                */
+               if (radix_enabled())
+                       return true;
+       }
+

should we do 

/* This cover both PROT_NONE (due to check above) and exec only
mapping */
if (radix_enabled() && !(vma->vm_flags & VM_READ)) {
    return true;
/* PROT_NONE check */
else if (!vma_is_accessible(vma)) 
   return true;

return false;

That is better, thanks.

- Russell

quoted

        if (unlikely(!vma_is_accessible(vma)))
                return true;
        /*

-aneesh