[PATCH v2] LoongArch: rethook: Do not restore percpu base register in trampoline

Wentao Guan posted 1 patch 1 month ago
arch/loongarch/kernel/rethook_trampoline.S | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH v2] LoongArch: rethook: Do not restore percpu base register in trampoline
Posted by Wentao Guan 1 month ago
The rethook trampoline saves $r21 ($u0), the percpu base, into its
frame at entry and restores it at exit. In between,
rethook_trampoline_handler() may schedule via preempt_enable_notrace();
if the task migrates to another CPU, the frame's $r21 names the old
CPU's percpu base, and restoring it poisons $r21 on the new CPU. Until
the next user->kernel transition heals $r21, this_cpu_*() accesses
(runqueues, RCU per-CPU data, timer tick programming, FPU ownership)
hit the wrong CPU's percpu area.

Under kretprobe-heavy preemptible load this corrupts scheduler and
timer state: scheduling-while-atomic splats, wrong-CPU RCU warnings,
WARN_ON_ONCE(rq != this_rq()) in nohz_balance_exit_idle(), and CPUs
parking in the idle loop with the constant timer never re-armed (hard
lockup). Reproduces on a Loongson-3A6000 with kretprobes on VFS paths
plus heavy file churn (OS install / unsquashfs).

By convention $r21 always holds the current CPU's percpu base in kernel
mode: exception entries reload it only when coming from user mode, and
RESTORE_SOME() restores it only when returning to user mode; the
context-switch path never writes it. The live $r21 at trampoline exit
is therefore already correct, and nothing in between can legitimately
change it (kernel C code cannot write a global register variable).
Drop the restore; keep the save so that the pt_regs view handed to
handlers stays fully initialized.

The same flaw existed in the pre-rethook kretprobe trampoline since
v6.3; it was carried over when rethook replaced it.

Fixes: 3f5536860086d ("LoongArch: Add kretprobes support")
Cc: stable@vger.kernel.org # v6.5+
Assisted-by: Kimi:Kimi-K3 # debug and root-cause analysis
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
---
changelog v2:
according sashiko report, keep cfi_st u0, PT_R21
Link: https://sashiko.dev/#/patchset/20260824082524.3801394-1-guanwentao%40uniontech.com

v1 link:
https://lore.kernel.org/loongarch/20260824082524.3801394-1-guanwentao@uniontech.com/T/#u
---
---
 arch/loongarch/kernel/rethook_trampoline.S | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/kernel/rethook_trampoline.S b/arch/loongarch/kernel/rethook_trampoline.S
index 2e009fbea53f2..94adead8faa5c 100644
--- a/arch/loongarch/kernel/rethook_trampoline.S
+++ b/arch/loongarch/kernel/rethook_trampoline.S
@@ -59,7 +59,14 @@
 	cfi_ld	t6, PT_R18
 	cfi_ld	t7, PT_R19
 	cfi_ld	t8, PT_R20
-	cfi_ld	u0, PT_R21
+	/*
+	 * $r21 ($u0, percpu base) is deliberately not restored: in kernel
+	 * mode it must always hold the current CPU's percpu base, and
+	 * restoring it from the frame would poison it with the old CPU's
+	 * base if the handler scheduled and we migrated.  The save side
+	 * stays so that the pt_regs view handed to handlers remains fully
+	 * initialized.
+	 */
 	cfi_ld	fp, PT_R22
 	cfi_ld	s0, PT_R23
 	cfi_ld	s1, PT_R24
-- 
2.30.2
Re: [PATCH v2] LoongArch: rethook: Do not restore percpu base register in trampoline
Posted by Huacai Chen 1 month ago
Hi, Wentao,

On Mon, Aug 24, 2026 at 5:34 PM Wentao Guan <guanwentao@uniontech.com> wrote:
>
> The rethook trampoline saves $r21 ($u0), the percpu base, into its
> frame at entry and restores it at exit. In between,
> rethook_trampoline_handler() may schedule via preempt_enable_notrace();
> if the task migrates to another CPU, the frame's $r21 names the old
> CPU's percpu base, and restoring it poisons $r21 on the new CPU. Until
> the next user->kernel transition heals $r21, this_cpu_*() accesses
> (runqueues, RCU per-CPU data, timer tick programming, FPU ownership)
> hit the wrong CPU's percpu area.
>
> Under kretprobe-heavy preemptible load this corrupts scheduler and
> timer state: scheduling-while-atomic splats, wrong-CPU RCU warnings,
> WARN_ON_ONCE(rq != this_rq()) in nohz_balance_exit_idle(), and CPUs
> parking in the idle loop with the constant timer never re-armed (hard
> lockup). Reproduces on a Loongson-3A6000 with kretprobes on VFS paths
> plus heavy file churn (OS install / unsquashfs).
>
> By convention $r21 always holds the current CPU's percpu base in kernel
> mode: exception entries reload it only when coming from user mode, and
> RESTORE_SOME() restores it only when returning to user mode; the
> context-switch path never writes it. The live $r21 at trampoline exit
> is therefore already correct, and nothing in between can legitimately
> change it (kernel C code cannot write a global register variable).
> Drop the restore; keep the save so that the pt_regs view handed to
> handlers stays fully initialized.
>
> The same flaw existed in the pre-rethook kretprobe trampoline since
> v6.3; it was carried over when rethook replaced it.
>
> Fixes: 3f5536860086d ("LoongArch: Add kretprobes support")
> Cc: stable@vger.kernel.org # v6.5+
> Assisted-by: Kimi:Kimi-K3 # debug and root-cause analysis
> Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
> ---
> changelog v2:
> according sashiko report, keep cfi_st u0, PT_R21
> Link: https://sashiko.dev/#/patchset/20260824082524.3801394-1-guanwentao%40uniontech.com
I don't think AI is correct here, if "leaking" a random stack value is
unacceptable, then "leaking" $r21 is better?

Huacai

>
> v1 link:
> https://lore.kernel.org/loongarch/20260824082524.3801394-1-guanwentao@uniontech.com/T/#u
> ---
> ---
>  arch/loongarch/kernel/rethook_trampoline.S | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/kernel/rethook_trampoline.S b/arch/loongarch/kernel/rethook_trampoline.S
> index 2e009fbea53f2..94adead8faa5c 100644
> --- a/arch/loongarch/kernel/rethook_trampoline.S
> +++ b/arch/loongarch/kernel/rethook_trampoline.S
> @@ -59,7 +59,14 @@
>         cfi_ld  t6, PT_R18
>         cfi_ld  t7, PT_R19
>         cfi_ld  t8, PT_R20
> -       cfi_ld  u0, PT_R21
> +       /*
> +        * $r21 ($u0, percpu base) is deliberately not restored: in kernel
> +        * mode it must always hold the current CPU's percpu base, and
> +        * restoring it from the frame would poison it with the old CPU's
> +        * base if the handler scheduled and we migrated.  The save side
> +        * stays so that the pt_regs view handed to handlers remains fully
> +        * initialized.
> +        */
>         cfi_ld  fp, PT_R22
>         cfi_ld  s0, PT_R23
>         cfi_ld  s1, PT_R24
> --
> 2.30.2
Re: [PATCH v2] LoongArch: rethook: Do not restore percpu base register in trampoline
Posted by Wentao Guan 1 month ago
> > ---
> > changelog v2:
> > according sashiko report, keep cfi_st u0, PT_R21
> > Link: https://sashiko.dev/#/patchset/20260824082524.3801394-1-guanwentao%40uniontech.com
> I don't think AI is correct here, if "leaking" a random stack value is
> unacceptable, then "leaking" $r21 is better?

So, 1. v3a back to dropping both or 2. v3b keeping the frame deterministic
via st.d zero, sp, PT_R21 or 3. v3c keep store RT_R21 for just a completeness
stack image?

BRs
Wentao Guan
Re: [PATCH v2] LoongArch: rethook: Do not restore percpu base register in trampoline
Posted by Wentao Guan 1 month ago
> > ---
> > changelog v2:
> > according sashiko report, keep cfi_st u0, PT_R21
> > Link: https://sashiko.dev/#/patchset/20260824082524.3801394-1-guanwentao%40uniontech.com
> I don't think AI is correct here, if "leaking" a random stack value is
> unacceptable, then "leaking" $r21 is better?

Right, in the place are secret to unprivileged users, so delete the pair is tidy.

BRs
Wentao Guan