[RFC PATCH 1/2] arm64: Support execute-only permissions with Enhanced PAN
Vladimir Murzin
vladimir.murzin at arm.com
Wed Nov 18 07:37:40 EST 2020
On 11/17/20 4:48 PM, Catalin Marinas wrote:
> On Fri, Nov 13, 2020 at 03:20:22PM +0000, Vladimir Murzin wrote:
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index 4ff12a7..d1f68d2 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -113,8 +113,15 @@ extern unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)];
>> #define pte_dirty(pte) (pte_sw_dirty(pte) || pte_hw_dirty(pte))
>>
>> #define pte_valid(pte) (!!(pte_val(pte) & PTE_VALID))
>> -#define pte_valid_not_user(pte) \
>> - ((pte_val(pte) & (PTE_VALID | PTE_USER)) == PTE_VALID)
>> +#define pte_valid_not_user(pte) \
>> +({ \
>> + int __val; \
>> + if (cpus_have_const_cap(ARM64_HAS_EPAN)) \
>> + __val = (pte_val(pte) & (PTE_VALID | PTE_USER | PTE_UXN)) == (PTE_VALID | PTE_UXN); \
>> + else \
>> + __val = (pte_val(pte) & (PTE_VALID | PTE_USER)) == PTE_VALID; \
>> + __val; \
>
> Is it worth having the cap check here? I'd go with the PTE_VALID|PTE_UXN
> check only.
>
I do not know to be honest. I do not have full picture in mind and what could be side effects of the
change (that's why RFC). 24cecc377463 the PTE_VALID|PTE_UXN moved to PTE_VALID, so I decided to be
safe than sorry...
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index dcc165b..2033e0b 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -1602,6 +1602,13 @@ static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
>> }
>> #endif /* CONFIG_ARM64_PAN */
>>
>> +#ifdef CONFIG_ARM64_EPAN
>> +static void cpu_enable_epan(const struct arm64_cpu_capabilities *__unused)
>> +{
>> + sysreg_clear_set(sctlr_el1, 0, SCTLR_EL1_EPAN);
>> +}
>> +#endif /* CONFIG_ARM64_EPAN */
>
> I checked the spec (2020 arch updates) and the EPAN bit is permitted to
> be cached in the TLB. I think we get away with this because this
> function is called before cnp is enabled. Maybe we should make it
> explicit and move the CnP entry last with a comment.
>
Hmm, so we rely on CnP's enable method to (indirectly) involve local_flush_tlb_all()? It doesn't seem
robust since CONFIG_ARM64_CNP could be unset. I can add local_flush_tlb_all() into cpu_enable_epan()
or we can have something like
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index bb2016c..0f0a27b 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2416,6 +2416,8 @@ static int cpu_enable_non_boot_scope_capabilities(void *__unused)
if (cap->cpu_enable)
cap->cpu_enable(cap);
}
+
+ local_flush_tlb_all();
return 0;
}
@@ -2467,6 +2469,8 @@ static void __init enable_cpu_capabilities(u16 scope_mask)
if (!boot_scope)
stop_machine(cpu_enable_non_boot_scope_capabilities,
NULL, cpu_online_mask);
+ else
+ local_flush_tlb_all();
}
/*
What would be your preference?
Cheers
Vladimir
More information about the linux-arm-kernel
mailing list