lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251218012656.GA75051@chenghao-pc>
Date: Thu, 18 Dec 2025 09:26:56 +0800
From: Chenghao Duan <duanchenghao@...inos.cn>
To: bot+bpf-ci@...nel.org
Cc: yangtiezhu@...ngson.cn, rostedt@...dmis.org, mhiramat@...nel.org,
	mark.rutland@....com, hengqi.chen@...il.com, chenhuacai@...nel.org,
	kernel@...0n.name, zhangtianyang@...ngson.cn, masahiroy@...nel.org,
	linux-kernel@...r.kernel.org, loongarch@...ts.linux.dev,
	bpf@...r.kernel.org, youling.tang@...ux.dev, jianghaoran@...inos.cn,
	vincent.mc.li@...il.com, linux-trace-kernel@...r.kernel.org,
	ast@...nel.org, andrii@...nel.org, daniel@...earbox.net,
	martin.lau@...nel.org, eddyz87@...il.com, yonghong.song@...ux.dev,
	clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH v4 1/7] LoongArch: ftrace: Refactor register restoration
 in ftrace_common_return

On Wed, Dec 17, 2025 at 06:55:00AM +0000, bot+bpf-ci@...nel.org wrote:
> > diff --git a/arch/loongarch/kernel/mcount_dyn.S b/arch/loongarch/kernel/mcount_dyn.S
> > index d6b474ad1d5e..5729c20e5b8b 100644
> > --- a/arch/loongarch/kernel/mcount_dyn.S
> > +++ b/arch/loongarch/kernel/mcount_dyn.S
> > @@ -94,7 +94,6 @@ SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL)
> >   * at the callsite, so there is no need to restore the T series regs.
> >   */
> >  ftrace_common_return:
> > -	PTR_L		ra, sp, PT_R1
> >  	PTR_L		a0, sp, PT_R4
> >  	PTR_L		a1, sp, PT_R5
> >  	PTR_L		a2, sp, PT_R6
> > @@ -104,12 +103,17 @@ ftrace_common_return:
> >  	PTR_L		a6, sp, PT_R10
> >  	PTR_L		a7, sp, PT_R11
> >  	PTR_L		fp, sp, PT_R22
> > -	PTR_L		t0, sp, PT_ERA
> >  	PTR_L		t1, sp, PT_R13
> > -	PTR_ADDI	sp, sp, PT_SIZE
> >  	bnez		t1, .Ldirect
> > +
> > +	PTR_L		ra, sp, PT_R1
> > +	PTR_L		t0, sp, PT_ERA
> > +	PTR_ADDI	sp, sp, PT_SIZE
> >  	jr		t0
> >  .Ldirect:
> > +	PTR_L		t0, sp, PT_R1
> > +	PTR_L		ra, sp, PT_ERA
>                      ^^^^^^^^^^^^^^^^^^^
> Does this break existing ftrace direct trampolines?
> 
> Before this change, direct trampolines received:
>   ra = parent return address (from PT_R1)
>   t0 = traced function address (from PT_ERA)
> 
> After this change, direct trampolines receive:
>   t0 = parent return address (from PT_R1)
>   ra = traced function address (from PT_ERA)
> 
> The sample trampolines in samples/ftrace/ were written for the old
> convention. For example, samples/ftrace/ftrace-direct.c does:
> 
>   my_tramp:
>     st.d  $t0, $sp, 8    # Save what it thinks is traced func addr
>     st.d  $ra, $sp, 16   # Save what it thinks is parent addr
>     bl    my_direct_func
>     ld.d  $t0, $sp, 8    # Restore
>     ld.d  $ra, $sp, 16   # Restore
>     jr    $t0            # Jump to what it thinks is traced function
> 
> With the new convention, the sample would save parent address in the
> t0 slot, restore it to t0, and then jump to the parent instead of the
> traced function, skipping the traced function entirely.
> 
> This was fixed later in commit 055e392f3be7 "LoongArch: ftrace: Adjust
> register stack restore order in direct call trampolines" which updated
> all the samples to match the new convention. However, any out-of-tree
> direct trampolines written before this commit would be broken by this
> change.
> 
> Was this ABI break intentional? The commit message mentions matching
> "the state when ftrace was entered" but doesn't explain why breaking
> compatibility was necessary. RISC-V uses the opposite convention
> (t0 = traced func, ra = parent), which the original LoongArch code
> matched.

The patch "LoongArch: ftrace: Refactor register restoration in
ftrace_common_return" changed the conventions for t0 and ra after
entering the direct call function, which is exactly the reason forthe
changes made in this patch.

Chenghao

> 
> > +	PTR_ADDI	sp, sp, PT_SIZE
> >  	jr		t1
> >  SYM_CODE_END(ftrace_common)
> 
> [ ... ]
> 
> > @@ -161,6 +165,8 @@ SYM_CODE_END(return_to_handler)
> >  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> >  SYM_CODE_START(ftrace_stub_direct_tramp)
> >  	UNWIND_HINT_UNDEFINED
> > -	jr		t0
> > +	move		t1, ra
> > +	move		ra, t0
> > +	jr		t1
> >  SYM_CODE_END(ftrace_stub_direct_tramp)
> >  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20293821855


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ