From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
Michael Ellerman <mpe@ellerman.id.au>,
Nicholas Piggin <npiggin@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Russell Currey <ruscur@russell.cc>
Subject: Re: [PATCH v2 37/37] powerpc: Support execute-only on all powerpc
Date: Thu, 02 Nov 2023 11:09:37 +0530 [thread overview]
Message-ID: <874ji4af3a.fsf@linux.ibm.com> (raw)
In-Reply-To: <4283ea9cbef9ff2fbee468904800e1962bc8fc18.1695659959.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Introduce PAGE_EXECONLY_X macro which provides exec-only rights.
> The _X may be seen as redundant with the EXECONLY but it helps
> keep consistancy, all macros having the EXEC right have _X.
>
> And put it next to PAGE_NONE as PAGE_EXECONLY_X is
> somehow PAGE_NONE + EXEC just like all other SOMETHING_X are
> just SOMETHING + EXEC.
>
> On book3s/64 PAGE_EXECONLY becomes PAGE_READONLY_X.
>
> On book3s/64, as PAGE_EXECONLY is only valid for Radix add
> VM_READ flag in vm_get_page_prot() for non-Radix.
>
> And update access_error() so that a non exec fault on a VM_EXEC only
> mapping is always invalid, even when the underlying layer don't
> always generate a fault for that.
>
> For 8xx, set PAGE_EXECONLY_X as _PAGE_NA | _PAGE_EXEC.
> For others, only set it as just _PAGE_EXEC
>
> With that change, 8xx, e500 and 44x fully honor execute-only
> protection.
>
> On 40x that is a partial implementation of execute-only. The
> implementation won't be complete because once a TLB has been loaded
> via the Instruction TLB miss handler, it will be possible to read
> the page. But at least it can't be read unless it is executed first.
>
> On 603 MMU, TLB missed are handled by SW and there are separate
> DTLB and ITLB. Execute-only is therefore now supported by not loading
> DTLB when read access is not permitted.
>
> On hash (604) MMU it is more tricky because hash table is common to
> load/store and execute. Nevertheless it is still possible to check
> whether _PAGE_READ is set before loading hash table for a load/store
> access. At least it can't be read unless it is executed first.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Russell Currey <ruscur@russell.cc>
> Cc: Kees Cook <keescook@chromium.org>
> ---
> arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +-
> arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +---
> arch/powerpc/include/asm/nohash/32/pte-8xx.h | 1 +
> arch/powerpc/include/asm/nohash/pgtable.h | 2 +-
> arch/powerpc/include/asm/nohash/pte-e500.h | 1 +
> arch/powerpc/include/asm/pgtable-masks.h | 2 ++
> arch/powerpc/mm/book3s64/pgtable.c | 10 ++++------
> arch/powerpc/mm/fault.c | 9 +++++----
> arch/powerpc/mm/pgtable.c | 4 ++--
> 9 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
> index 244621c88510..52971ee30717 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> @@ -425,7 +425,7 @@ static inline bool pte_access_permitted(pte_t pte, bool write)
> {
> /*
> * A read-only access is controlled by _PAGE_READ bit.
> - * We have _PAGE_READ set for WRITE and EXECUTE
> + * We have _PAGE_READ set for WRITE
> */
> if (!pte_present(pte) || !pte_read(pte))
> return false;
>
Should this now be updated to check for EXEC bit ?
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 0fd12bdc7b5e..751b01227e36 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -18,6 +18,7 @@
> #define _PAGE_WRITE 0x00002 /* write access allowed */
> #define _PAGE_READ 0x00004 /* read access allowed */
> #define _PAGE_NA _PAGE_PRIVILEGED
> +#define _PAGE_NAX _PAGE_EXEC
> #define _PAGE_RO _PAGE_READ
> #define _PAGE_ROX (_PAGE_READ | _PAGE_EXEC)
> #define _PAGE_RW (_PAGE_READ | _PAGE_WRITE)
> @@ -141,9 +142,6 @@
>
> #include <asm/pgtable-masks.h>
>
> -/* Radix only, Hash uses PAGE_READONLY_X + execute-only pkey instead */
> -#define PAGE_EXECONLY __pgprot(_PAGE_BASE | _PAGE_EXEC)
> -
> /* Permission masks used for kernel mappings */
> #define PAGE_KERNEL __pgprot(_PAGE_BASE | _PAGE_KERNEL_RW)
> #define PAGE_KERNEL_NC __pgprot(_PAGE_BASE_NC | _PAGE_KERNEL_RW | _PAGE_TOLERANT)
> diff --git a/arch/powerpc/include/asm/nohash/32/pte-8xx.h b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
> index 1ee38befd29a..137dc3c84e45 100644
> --- a/arch/powerpc/include/asm/nohash/32/pte-8xx.h
> +++ b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
> @@ -48,6 +48,7 @@
>
> #define _PAGE_HUGE 0x0800 /* Copied to L1 PS bit 29 */
>
> +#define _PAGE_NAX (_PAGE_NA | _PAGE_EXEC)
> #define _PAGE_ROX (_PAGE_RO | _PAGE_EXEC)
> #define _PAGE_RW 0
> #define _PAGE_RWX _PAGE_EXEC
> diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
> index f922c84b23eb..a50be1de9f83 100644
> --- a/arch/powerpc/include/asm/nohash/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/pgtable.h
> @@ -203,7 +203,7 @@ static inline bool pte_access_permitted(pte_t pte, bool write)
> {
> /*
> * A read-only access is controlled by _PAGE_READ bit.
> - * We have _PAGE_READ set for WRITE and EXECUTE
> + * We have _PAGE_READ set for WRITE
> */
> if (!pte_present(pte) || !pte_read(pte))
> return false;
>
Same here. if so I guess book3s/64 also will need an update?
> diff --git a/arch/powerpc/include/asm/nohash/pte-e500.h b/arch/powerpc/include/asm/nohash/pte-e500.h
> index 31d2c3ea7df8..f516f0b5b7a8 100644
> --- a/arch/powerpc/include/asm/nohash/pte-e500.h
> +++ b/arch/powerpc/include/asm/nohash/pte-e500.h
> @@ -57,6 +57,7 @@
> #define _PAGE_KERNEL_ROX (_PAGE_BAP_SR | _PAGE_BAP_SX)
>
> #define _PAGE_NA 0
> +#define _PAGE_NAX _PAGE_BAP_UX
> #define _PAGE_RO _PAGE_READ
> #define _PAGE_ROX (_PAGE_READ | _PAGE_BAP_UX)
> #define _PAGE_RW (_PAGE_READ | _PAGE_WRITE)
> diff --git a/arch/powerpc/include/asm/pgtable-masks.h b/arch/powerpc/include/asm/pgtable-masks.h
> index 808a3b9e8fc0..6e8e2db26a5a 100644
> --- a/arch/powerpc/include/asm/pgtable-masks.h
> +++ b/arch/powerpc/include/asm/pgtable-masks.h
> @@ -4,6 +4,7 @@
>
> #ifndef _PAGE_NA
> #define _PAGE_NA 0
> +#define _PAGE_NAX _PAGE_EXEC
> #define _PAGE_RO _PAGE_READ
> #define _PAGE_ROX (_PAGE_READ | _PAGE_EXEC)
> #define _PAGE_RW (_PAGE_READ | _PAGE_WRITE)
> @@ -20,6 +21,7 @@
>
> /* Permission masks used to generate the __P and __S table */
> #define PAGE_NONE __pgprot(_PAGE_BASE | _PAGE_NA)
> +#define PAGE_EXECONLY_X __pgprot(_PAGE_BASE | _PAGE_NAX)
> #define PAGE_SHARED __pgprot(_PAGE_BASE | _PAGE_RW)
> #define PAGE_SHARED_X __pgprot(_PAGE_BASE | _PAGE_RWX)
> #define PAGE_COPY __pgprot(_PAGE_BASE | _PAGE_RO)
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 8f8a62d3ff4d..be229290a6a7 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -635,12 +635,10 @@ pgprot_t vm_get_page_prot(unsigned long vm_flags)
> unsigned long prot;
>
> /* Radix supports execute-only, but protection_map maps X -> RX */
> - if (radix_enabled() && ((vm_flags & VM_ACCESS_FLAGS) == VM_EXEC)) {
> - prot = pgprot_val(PAGE_EXECONLY);
> - } else {
> - prot = pgprot_val(protection_map[vm_flags &
> - (VM_ACCESS_FLAGS | VM_SHARED)]);
> - }
> + if (!radix_enabled() && ((vm_flags & VM_ACCESS_FLAGS) == VM_EXEC))
> + vm_flags |= VM_READ;
> +
> + prot = pgprot_val(protection_map[vm_flags & (VM_ACCESS_FLAGS | VM_SHARED)]);
>
> if (vm_flags & VM_SAO)
> prot |= _PAGE_SAO;
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index b1723094d464..9e49ede2bc1c 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -266,14 +266,15 @@ static bool access_error(bool is_write, bool is_exec, struct vm_area_struct *vma
> }
>
> /*
> - * VM_READ, VM_WRITE and VM_EXEC all imply read permissions, as
> - * defined in protection_map[]. Read faults can only be caused by
> - * a PROT_NONE mapping, or with a PROT_EXEC-only mapping on Radix.
> + * VM_READ, VM_WRITE and VM_EXEC may imply read permissions, as
> + * defined in protection_map[]. In that case Read faults can only be
> + * caused by a PROT_NONE mapping. However a non exec access on a
> + * VM_EXEC only mapping is invalid anyway, so report it as such.
> */
> if (unlikely(!vma_is_accessible(vma)))
> return true;
>
> - if (unlikely(radix_enabled() && ((vma->vm_flags & VM_ACCESS_FLAGS) == VM_EXEC)))
> + if ((vma->vm_flags & VM_ACCESS_FLAGS) == VM_EXEC)
> return true;
>
> /*
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index 781a68c69c2f..79508c1d15d7 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -492,7 +492,7 @@ const pgprot_t protection_map[16] = {
> [VM_READ] = PAGE_READONLY,
> [VM_WRITE] = PAGE_COPY,
> [VM_WRITE | VM_READ] = PAGE_COPY,
> - [VM_EXEC] = PAGE_READONLY_X,
> + [VM_EXEC] = PAGE_EXECONLY_X,
> [VM_EXEC | VM_READ] = PAGE_READONLY_X,
> [VM_EXEC | VM_WRITE] = PAGE_COPY_X,
> [VM_EXEC | VM_WRITE | VM_READ] = PAGE_COPY_X,
> @@ -500,7 +500,7 @@ const pgprot_t protection_map[16] = {
> [VM_SHARED | VM_READ] = PAGE_READONLY,
> [VM_SHARED | VM_WRITE] = PAGE_SHARED,
> [VM_SHARED | VM_WRITE | VM_READ] = PAGE_SHARED,
> - [VM_SHARED | VM_EXEC] = PAGE_READONLY_X,
> + [VM_SHARED | VM_EXEC] = PAGE_EXECONLY_X,
> [VM_SHARED | VM_EXEC | VM_READ] = PAGE_READONLY_X,
> [VM_SHARED | VM_EXEC | VM_WRITE] = PAGE_SHARED_X,
> [VM_SHARED | VM_EXEC | VM_WRITE | VM_READ] = PAGE_SHARED_X
> --
> 2.41.0
next prev parent reply other threads:[~2023-11-02 5:40 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-25 18:31 [PATCH v2 00/37] Implement execute-only protection on powerpc Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 01/37] powerpc/8xx: Fix pte_access_permitted() for PAGE_NONE Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 02/37] powerpc/64e: Fix wrong test in __ptep_test_and_clear_young() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 03/37] powerpc/40x: Remove stale PTE_ATOMIC_UPDATES macro Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 04/37] powerpc: Remove pte_ERROR() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 05/37] powerpc: Deduplicate prototypes of ptep_set_access_flags() and phys_mem_access_prot() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 06/37] powerpc: Refactor update_mmu_cache_range() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 07/37] powerpc: Untangle fixmap.h and pgtable.h and mmu.h Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 08/37] powerpc/nohash: Remove {pte/pmd}_protnone() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 09/37] powerpc/nohash: Refactor declaration of {map/unmap}_kernel_page() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 10/37] powerpc/nohash: Move 8xx version of pte_update() into pte-8xx.h Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 11/37] powerpc/nohash: Replace #ifdef CONFIG_44x by IS_ENABLED(CONFIG_44x) in pgtable.h Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 12/37] powerpc/nohash: Refactor pte_update() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 13/37] powerpc/nohash: Refactor checking of no-change in pte_update() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 14/37] powerpc/nohash: Deduplicate _PAGE_CHG_MASK Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 15/37] powerpc/nohash: Deduplicate pte helpers Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 16/37] powerpc/nohash: Refactor ptep_test_and_clear_young() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 17/37] powerpc/nohash: Deduplicate ptep_set_wrprotect() and ptep_get_and_clear() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 18/37] powerpc/nohash: Refactor pte_clear() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 19/37] powerpc/nohash: Refactor __ptep_set_access_flags() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 20/37] powerpc/e500: Simplify pte_mkexec() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 21/37] powerpc: Implement and use pgprot_nx() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 22/37] powerpc: Fail ioremap() instead of silently ignoring flags when PAGE_USER is set Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 23/37] powerpc: Remove pte_mkuser() and pte_mkpriviledged() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 24/37] powerpc: Rely on address instead of pte_user() Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 25/37] powerpc: Refactor permission masks used for __P/__S table and kernel memory flags Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 26/37] powerpc/8xx: Use generic permission masks Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 27/37] powerpc/64s: " Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 28/37] powerpc/nohash: Add _PAGE_WRITE to supplement _PAGE_RW Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 29/37] powerpc/nohash: Replace pte_user() by pte_read() Christophe Leroy
2023-10-31 10:15 ` Aneesh Kumar K.V
2023-11-06 13:21 ` Christophe Leroy
2023-11-07 13:34 ` Aneesh Kumar K.V
2023-11-09 18:20 ` Christophe Leroy
2023-11-13 10:23 ` Aneesh Kumar K.V
2023-11-23 15:55 ` Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 30/37] powerpc/e500: Introduce _PAGE_READ and remove _PAGE_USER Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 31/37] powerpc/44x: " Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 32/37] powerpc/40x: " Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 33/37] powerpc/32s: Add _PAGE_WRITE to supplement _PAGE_RW Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 34/37] powerpc/32s: Introduce _PAGE_READ and remove _PAGE_USER Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 35/37] powerpc/ptdump: Display _PAGE_READ and _PAGE_WRITE Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 36/37] powerpc: Finally remove _PAGE_USER Christophe Leroy
2023-09-25 18:31 ` [PATCH v2 37/37] powerpc: Support execute-only on all powerpc Christophe Leroy
2023-11-02 5:39 ` Aneesh Kumar K.V [this message]
2023-11-06 13:23 ` Christophe Leroy
2023-11-07 6:15 ` Aneesh Kumar K V
2023-11-09 17:38 ` Christophe Leroy
2023-10-15 10:00 ` (subset) [PATCH v2 00/37] Implement execute-only protection on powerpc Michael Ellerman
2023-10-27 9:59 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874ji4af3a.fsf@linux.ibm.com \
--to=aneesh.kumar@linux.ibm.com \
--cc=christophe.leroy@csgroup.eu \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
--cc=ruscur@russell.cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).