[PATCH v2 37/37] powerpc: Support execute-only on all powerpc

[Christophe Leroy]

Le 02/11/2023 à 06:39, Aneesh Kumar K.V a écrit :
> Christophe Leroy <christophe.leroy at 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 at csgroup.eu>
>> Cc: Russell Currey <ruscur at russell.cc>
>> Cc: Kees Cook <keescook at 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 ?

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

Christophe