Re: [PATCH 1/2] arm64: ftrace: enable single ftrace_ops for direct calls

On 27/7/26 22:52, [email protected] 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 <[email protected]>
> 
> 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.

Thanks,
Leon