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

Hi, Wentao,

On Mon, Aug 24, 2026 at 5:34 PM Wentao Guan <[email protected]> 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: [email protected] # v6.5+
> Assisted-by: Kimi:Kimi-K3 # debug and root-cause analysis
> Signed-off-by: Wentao Guan <[email protected]>
> ---
> 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/[email protected]/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