Re: [PATCH v1] arm64: poe: fix stale POR_EL0 values for ptrace
From: Joey Gouly <joey.gouly@arm.com>
Date: 2026-01-21 16:59:56
Also in:
stable
On Wed, Jan 21, 2026 at 03:44:21PM +0000, Mark Rutland wrote:
Hi Joey, On Wed, Jan 21, 2026 at 01:56:39PM +0000, Joey Gouly wrote:quoted
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 <redacted> Cc: stable@vger.kernel.org Signed-off-by: Joey Gouly <joey.gouly@arm.com> Cc: Kevin Brodsky <redacted> Cc: Mark Rutland <mark.rutland@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@arm.com>quoted
--- 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?
It is possible to do as a static inline..
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.quoted
+ #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);
.. however I think this approach is better, no need for abstraction over this. I will send a v2 in a day or so, in case there are other comments / disagreements with that approach. Thanks, Joey
... 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.