[PATCH v1] arm64: poe: fix stale POR_EL0 values for ptrace
Mark Rutland
mark.rutland at arm.com
Wed Jan 21 07:44:21 PST 2026
Hi Joey,
On Wed, Jan 21, 2026 at 01:56:39PM +0000, Joey Gouly wrote:
> If a process wrote to POR_EL0 and then crashed before a context switch
> happened, the coredump would contain an incorrect value for POR_EL0.
>
> The value read in poe_get() would be a stale value left in thread.por_el0. Fix
> this by reading the value from the system register, if the target thread is the
> current thread.
>
> This matches what gcs/fpsimd do.
>
> Fixes: 175198199262 ("arm64/ptrace: add support for FEAT_POE")
> Reported-by: David Spickett <david.spickett at arm.com>
> Cc: stable at vger.kernel.org
> Signed-off-by: Joey Gouly <joey.gouly at arm.com>
> Cc: Kevin Brodsky <kevin.brodsky at arm.com>
> Cc: Mark Rutland <mark.rutland at arm.com>
I have a couple of comments below, but as-is this looks functionally
correct to me. With or without the changes suggested below:
Acked-by: Mark Rutland <mark.rutland at arm.com>
> ---
> arch/arm64/include/asm/por.h | 2 ++
> arch/arm64/kernel/process.c | 7 ++++++-
> arch/arm64/kernel/ptrace.c | 5 +++++
> 3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/por.h b/arch/arm64/include/asm/por.h
> index d913d5b529e4..46f1356837e2 100644
> --- a/arch/arm64/include/asm/por.h
> +++ b/arch/arm64/include/asm/por.h
> @@ -31,4 +31,6 @@ static inline bool por_elx_allows_exec(u64 por, u8 pkey)
> return perm & POE_X;
> }
>
> +void poe_preserve_current_state(void);
Is it possible to have a static inline here, i.e.
static inline void poe_preserve_current_state(void)
{
current->thread.por_el0 = read_sysreg_s(SYS_POR_EL0);
}
... or will that cause some header dependency problem?
If we can have this as a static inline, we can use it everywhere
consistently, and avoid needing a function call for a trivial number of
instructions.
Otherwise, see below for another option.
> +
> #endif /* _ASM_ARM64_POR_H */
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 489554931231..400182099784 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -665,12 +665,17 @@ static int do_set_tsc_mode(unsigned int val)
> return 0;
> }
>
> +void poe_preserve_current_state(void)
> +{
> + current->thread.por_el0 = read_sysreg_s(SYS_POR_EL0);
> +}
> +
> static void permission_overlay_switch(struct task_struct *next)
> {
> if (!system_supports_poe())
> return;
>
> - current->thread.por_el0 = read_sysreg_s(SYS_POR_EL0);
> + poe_preserve_current_state();
> if (current->thread.por_el0 != next->thread.por_el0) {
> write_sysreg_s(next->thread.por_el0, SYS_POR_EL0);
> /*
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index b9bdd83fbbca..276d8ee630cd 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -37,6 +37,7 @@
> #include <asm/gcs.h>
> #include <asm/mte.h>
> #include <asm/pointer_auth.h>
> +#include <asm/por.h>
> #include <asm/stacktrace.h>
> #include <asm/syscall.h>
> #include <asm/traps.h>
> @@ -1486,6 +1487,10 @@ static int poe_get(struct task_struct *target,
> if (!system_supports_poe())
> return -EINVAL;
>
> + if (target == current) {
> + poe_preserve_current_state();
> + }
If we can't do the static inline, it might be best to just open code the
read here, i.e. make this:
if (target == current)
current->thread.por_el0 = read_sysreg_s(SYS_POR_EL0);
... since permission_overlay_switch() writes the register directly,
we're not really gaining abstraction by factoring this out. Open coding
would make the diff a bit smaller, and avoid the function call.
That all said, this is functionally correct either way, so if Catalin or
Will disagree, go with whatever they prefer!
Mark.
More information about the linux-arm-kernel
mailing list