bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Leon Hwang <leon.hwang@linux.dev>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: sashiko-reviews@lists.linux.dev,
	Masami Hiramatsu <mhiramat@kernel.org>,
	bpf@vger.kernel.org
Subject: Re: [PATCH 1/2] arm64: ftrace: enable single ftrace_ops for direct calls
Date: Wed, 29 Jul 2026 14:27:48 +0800	[thread overview]
Message-ID: <c25b272d-5975-4fd1-be3f-6b5a0522cbee@linux.dev> (raw)
In-Reply-To: <20260728114602.62e8852a@gandalf.local.home>

On 28/7/26 23:46, Steven Rostedt wrote:
> On Tue, 28 Jul 2026 10:19:22 +0800
> Leon Hwang <leon.hwang@linux.dev> wrote:
> 
>> On 27/7/26 22:52, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>>
>>> Pre-existing issues:
>>> - [High] Missing RCU read barrier (`rcu_dereference()`) when accessing `direct_functions`.
>>> --
>>>
>>> --- Patch [1]: [PATCH 1/2] arm64: ftrace: enable single ftrace_ops for direct calls ---
>>> commit 738274783fa707248af104af5e45da296f7e4d84
>>> Author: Leon Hwang <leon.hwang@linux.dev>
>>>
>>> arm64: ftrace: enable single ftrace_ops for direct calls
>>>
>>> The BPF tracing multi link updates several direct-call sites through one
>>> ftrace_ops. This commit selects HAVE_SINGLE_FTRACE_DIRECT_OPS on arm64 when
>>> DYNAMIC_FTRACE_WITH_DIRECT_CALLS is enabled. It also generalizes the
>>> unreachable-trampoline comment.
>>>   
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 39eba438dad43..94fa6e72c2867 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -188,6 +188,8 @@ config ARM64
>>>>  		    CLANG_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS)
>>>>  	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS \
>>>>  		if DYNAMIC_FTRACE_WITH_ARGS
>>>> +	select HAVE_SINGLE_FTRACE_DIRECT_OPS \
>>>> +		if DYNAMIC_FTRACE_WITH_DIRECT_CALLS  
>>>
>>> [Severity: High]
>>> This is a pre-existing issue, but by enabling HAVE_SINGLE_FTRACE_DIRECT_OPS
>>> on arm64, does this expose the architecture to a missing RCU read barrier
>>> in the ftrace code?
>>>
>>> When HAVE_SINGLE_FTRACE_DIRECT_OPS is selected, call_direct_funcs() switches
>>> from using READ_ONCE(ops->direct_call) to using ftrace_find_rec_direct():
>>>
>>> kernel/trace/ftrace.c:call_direct_funcs() {
>>> ...
>>> #ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
>>> 	addr = ftrace_find_rec_direct(ip);
>>> #else
>>> ...
>>> }
>>>
>>> Looking at ftrace_find_rec_direct(), it accesses the __rcu pointer
>>> direct_functions directly without an rcu_dereference():
>>>
>>> kernel/trace/ftrace.c:ftrace_find_rec_direct() {
>>> 	struct ftrace_func_entry *entry;
>>>
>>> 	entry = __ftrace_lookup_ip(direct_functions, ip);
>>> ...
>>> }
>>>
>>> Without the READ_ONCE() provided by rcu_dereference(), the compiler might
>>> reload the pointer between computing the hash key and accessing the bucket
>>> array inside __ftrace_lookup_ip().
>>>
>>> If a concurrent writer substitutes a new hash table with a different size
>>> between these reloads, could the computed key exceed the new bucket array
>>> bounds and lead to an out-of-bounds dereference?
>>>   
>>
>>
>> Hi Steve, Masami,
>>
>> Could you take a look at this issue? The issue looks legitimate.
> 
> Yeah, it's possible (but unlikely) for the compiler to do that. But there's
> nothing harming the code to do:


Good to know it's unlikely in practice.

If you don't mind, I'll send a separate patch for the fix based on the
diff below. It compiles and works fine in my testing.

Thanks,
Leon

> 
> ( Untested and not even compiled )
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f93e34dd2328..5ec494790312 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2639,7 +2639,8 @@ unsigned long ftrace_find_rec_direct(unsigned long ip)
>  {
>  	struct ftrace_func_entry *entry;
>  
> -	entry = __ftrace_lookup_ip(direct_functions, ip);
> +	guard(preempt_notrace)();
> +	entry = __ftrace_lookup_ip(rcu_dereference_sched(direct_functions), ip);
>  	if (!entry)
>  		return 0;
>  
> 
> -- Steve
> 


  reply	other threads:[~2026-07-29  6:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-27 14:28 [PATCH 0/2] arm64: ftrace: enable single ftrace_ops for direct calls Leon Hwang
2026-07-27 14:28 ` [PATCH 1/2] " Leon Hwang
2026-07-27 14:52   ` sashiko-bot
2026-07-28  2:19     ` Leon Hwang
2026-07-28 15:46       ` Steven Rostedt
2026-07-29  6:27         ` Leon Hwang [this message]
2026-07-29 11:56   ` Jiri Olsa
2026-07-30  3:05     ` Leon Hwang
2026-07-27 14:28 ` [PATCH 2/2] selftests/bpf: Enable tracing_multi tests on arm64 Leon Hwang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c25b272d-5975-4fd1-be3f-6b5a0522cbee@linux.dev \
    --to=leon.hwang@linux.dev \
    --cc=bpf@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).