On 11/6/23 6:53 PM, Christophe Leroy wrote:
>
>
> Le 02/11/2023 à 06:39, Aneesh Kumar K.V a écrit :
>> Christophe Leroy <[email protected]> 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 <[email protected]>
>>> Cc: Russell Currey <[email protected]>
>>> Cc: Kees Cook <[email protected]>
>>> ---
>>> 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 ?
>
> I don't think so based on what I see in arm64:
> https://elixir.bootlin.com/linux/v6.6/source/arch/arm64/include/asm/pgtable.h#L146
>
But then there can be a get_user_pages() (FOLL_GET) on an exec only pte right?
if we are going to access the page data(FOLL_PIN), then yes exec only mapping should
fail for that. But if we using it to do struct page manipulation we need pte_access_permitted
to return true for exec only mapping?
-aneesh