| Message ID | 20220808115828.20113-1-ruscur@russell.cc (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/mm: Support execute-only memory on the Radix MMU | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_kernel_qemu | fail | 2 of 20 jobs failed. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | fail | sparse (ppc64le, ppc64le, ubuntu-21.10) failed at step Build. |
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | fail | 2 of 6 jobs failed. |
On 8/8/22 5:28 PM, Russell Currey wrote: > 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 <ruscur@russell.cc> > --- > 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; > if (unlikely(!vma_is_accessible(vma))) > return true; > /* -aneesh
On Mon, 2022-08-08 at 18:28 +0530, Aneesh Kumar K V wrote: > On 8/8/22 5:28 PM, Russell Currey wrote: > > 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 <ruscur@russell.cc> > > --- > > 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 > > > > > if (unlikely(!vma_is_accessible(vma))) > > return true; > > /* > > -aneesh
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; + } + if (unlikely(!vma_is_accessible(vma))) return true; /*
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 <ruscur@russell.cc> --- 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(+)