Re: [PATCH RFC v10 10/22] arm64: Enable kpkeys

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] arm64: cpufeature: cpu_enable_poe() enables TCR2_EL1_POE
  without S1PIE
--

commit 916b4efd6b8494c5d63fd0effff9032def1dba2d
Author: Kevin Brodsky <kevin.brodsky@arm.com>

arm64: Enable kpkeys

This commit enables POE at EL1 by initializing POR_EL1, setting
TCR2_EL1.POE in cpu_enable_poe(), and selecting ARCH_HAS_KPKEYS under
CONFIG_ARM64_POE.

> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 34c5c03c0c9b4..42d693197739c 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c

[ ... ]

> @@ -2334,8 +2335,10 @@ static void cpu_enable_mops(const struct arm64_cpu_capabilities *__unused)
>  #ifdef CONFIG_ARM64_POE
>  static void cpu_enable_poe(const struct arm64_cpu_capabilities *__unused)
>  {
> -	sysreg_clear_set(REG_TCR2_EL1, 0, TCR2_EL1_E0POE);
> +	write_sysreg_s(POR_EL1_INIT, SYS_POR_EL1);
> +	sysreg_clear_set(REG_TCR2_EL1, 0, TCR2_EL1_E0POE | TCR2_EL1_POE);
>  	sysreg_clear_set(CPACR_EL1, 0, CPACR_EL1_E0POE);
> +	isb();
>  }
>  #endif

[Severity: Medium]
Does enabling TCR2_EL1_POE here break privileged EL1 accesses to user
memory when S1POE is present without S1PIE (for example, in a KVM guest
configured with FEAT_S1POE=1 and FEAT_S1PIE=0)?

Commit "arm64: mm: Enable overlays for all EL1 indirect permissions" relies
on PIR_EL1 (PIE_E1) leaving user memory types without overlay (_O) so that
privileged EL1 accesses to user pages (such as futex_atomic_op_inuser()
using uaccess_enable_privileged() when FEAT_LSUI is absent) are not checked
against POR_EL1.  When S1PIE is absent, stage-1 translations use direct
permissions and TCR2_EL1_POE applies POR_EL1 to all privileged accesses,
including TTBR0 user mappings.

Because POR_EL1_INIT only permits pkey 0, a user mapping with pkey 1..7
causes futex_atomic_op_inuser() to fault against POR_EL1 while
fault_in_user_writeable() succeeds with unprivileged sttr against POR_EL0,
resulting in an unbounded retry loop in futex_wake_op().

Should TCR2_EL1_POE and arch_supports_kpkeys() (or ARM64_HAS_S1POE itself)
require S1PIE to be supported and enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-kpkeys-v10-0-aa6c4d9603bc@arm.com?part=10