linux-kernel - Re: [GIT PULL] ftrace: Fix for v6.13

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: <20250102103413.63f42d8d@gandalf.local.home>
Date: Thu, 2 Jan 2025 10:34:13 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Mark Rutland <mark.rutland@....com>, Zilin Guan <zilin@....edu.cn>
Subject: Re: [GIT PULL] ftrace: Fix for v6.13

On Thu, 2 Jan 2025 10:20:46 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> Linus,
> 
> Fix for function graph tracer in v6.13:

Linus,

If you want, you can hold off on pulling this. There's another unrelated
fix to ftrace that I need to add and test. Then I can send a pull request
for both fixes instead of two individual ones.

-- Steve


> 
> - Add needed READ_ONCE() around access to the fgraph array element
> 
>   The updates to the fgraph array can happen when callbacks are registered
>   and deregistered. The __ftrace_return_to_handler() can handle reading
>   either the old value or the new value. But once it reads that value
>   it must stay consistent otherwise the check that looks to see if the
>   value is a stub may show false, but if the compiler decides to re-read
>   after that check, it can be true which can cause the code to crash
>   later on.
> 
> 
> Please pull the latest ftrace-v6.13-rc5 tree, which can be found at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
> ftrace-v6.13-rc5
> 
> Tag SHA1: 0ba23030ff298e46f7d435ee1e11040d00239871
> Head SHA1: e99b3d0cb6cd0b051fea113bb9a4088d595c4693
> 
> 
> Zilin Guan (1):
>       fgraph: Add READ_ONCE() when accessing fgraph_array[]
> 
> ----
>  kernel/trace/fgraph.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> ---------------------------
> commit e99b3d0cb6cd0b051fea113bb9a4088d595c4693
> Author: Zilin Guan <zilin@....edu.cn>
> Date:   Tue Dec 31 11:37:31 2024 +0000
> 
>     fgraph: Add READ_ONCE() when accessing fgraph_array[]
>     
>     In __ftrace_return_to_handler(), a loop iterates over the fgraph_array[]
>     elements, which are fgraph_ops. The loop checks if an element is a
>     fgraph_stub to prevent using a fgraph_stub afterward.
>     
>     However, if the compiler reloads fgraph_array[] after this check, it might
>     race with an update to fgraph_array[] that introduces a fgraph_stub. This
>     could result in the stub being processed, but the stub contains a null
>     "func_hash" field, leading to a NULL pointer dereference.
>     
>     To ensure that the gops compared against the fgraph_stub matches the gops
>     processed later, add a READ_ONCE(). A similar patch appears in commit
>     63a8dfb ("function_graph: Add READ_ONCE() when accessing fgraph_array[]").
>     
>     Cc:stable@...r.kernel.org
>     Fixes: 37238abe3cb47 ("ftrace/function_graph: Pass fgraph_ops to function graph callbacks")
>     Link: https://lore.kernel.org/20241231113731.277668-1-zilin@seu.edu.cn
>     Signed-off-by: Zilin Guan <zilin@....edu.cn>
>     Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> 
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index ddedcb50917f..30e3ddc8a8a8 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -833,7 +833,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
>  #endif
>  	{
>  		for_each_set_bit(i, &bitmap, sizeof(bitmap) * BITS_PER_BYTE) {
> -			struct fgraph_ops *gops = fgraph_array[i];
> +			struct fgraph_ops *gops = READ_ONCE(fgraph_array[i]);
>  
>  			if (gops == &fgraph_stub)
>  				continue;