All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Ihor Solodrai <ihor.solodrai@linux.dev>
Cc: Jiri Olsa <olsajiri@gmail.com>,
	Steven Rostedt <rostedt@kernel.org>,
	Florent Revest <revest@google.com>,
	Mark Rutland <mark.rutland@arm.com>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Menglong Dong <menglong8.dong@gmail.com>,
	Song Liu <song@kernel.org>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>
Subject: Re: [PATCHv6 bpf-next 9/9] bpf,x86: Use single ftrace_ops for direct calls
Date: Fri, 27 Feb 2026 22:24:37 +0100	[thread overview]
Message-ID: <aaILlSuEgaYDq41r@krava> (raw)
In-Reply-To: <aaIAoBZGkP7RQrvc@krava>

On Fri, Feb 27, 2026 at 09:37:52PM +0100, Jiri Olsa wrote:
> On Fri, Feb 27, 2026 at 09:40:12AM -0800, Ihor Solodrai wrote:
> > On 12/30/25 6:50 AM, Jiri Olsa wrote:
> > > Using single ftrace_ops for direct calls update instead of allocating
> > > ftrace_ops object for each trampoline.
> > > 
> > > With single ftrace_ops object we can use update_ftrace_direct_* api
> > > that allows multiple ip sites updates on single ftrace_ops object.
> > > 
> > > Adding HAVE_SINGLE_FTRACE_DIRECT_OPS config option to be enabled on
> > > each arch that supports this.
> > > 
> > > At the moment we can enable this only on x86 arch, because arm relies
> > > on ftrace_ops object representing just single trampoline image (stored
> > > in ftrace_ops::direct_call). Archs that do not support this will continue
> > > to use *_ftrace_direct api.
> > > 
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > 
> > Hi Jiri,
> > 
> > Me and Kumar stumbled on kernel splats with "ftrace failed to modify",
> > and if running with KASAN:
> > 
> >   BUG: KASAN: slab-use-after-free in __get_valid_kprobe+0x224/0x2a0
> > 
> > Pasting a full splat example at the bottom.
> > 
> > I was able to create a reproducer with AI, and then used it to bisect
> > to this patch. You can run it with ./test_progs -t ftrace_direct_race
> > 
> > Below is my (human-generated, haha) summary of AI's analysis of what's
> > happening. It makes sense to me conceptually, but I don't know enough
> > details here to call bullshit. Please take a look:
> 
> hi, nice :)
> 
> > 
> >     With CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS ftrace_replace_code()
> >     operates on all call sites in the shared ops. Then if a concurrent
> >     ftrace user (like kprobe) modifies a call site in between
> >     ftrace_replace_code's verify pass and its patch pass, then ftrace_bug
> >     fires and sets ftrace_disabled to 1.
> 
> hum, I'd think that's all under ftrace_lock/direct_mutex,
> but we might be missing some paths
> 

could you please try with change below? I can no longer trigger the bug with it

thanks,
jirka


---
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 827fb9a0bf0d..e333749a5896 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6404,7 +6404,9 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
 			new_filter_hash = old_filter_hash;
 		}
 	} else {
+		mutex_lock(&ftrace_lock);
 		err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+		mutex_unlock(&ftrace_lock);
 		/*
 		 * new_filter_hash is dup-ed, so we need to release it anyway,
 		 * old_filter_hash either stays on error or is already released
@@ -6530,7 +6532,9 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
 			ops->func_hash->filter_hash = NULL;
 		}
 	} else {
+		mutex_lock(&ftrace_lock);
 		err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+		mutex_unlock(&ftrace_lock);
 		/*
 		 * new_filter_hash is dup-ed, so we need to release it anyway,
 		 * old_filter_hash either stays on error or is already released

  reply	other threads:[~2026-02-27 21:24 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-30 14:50 [PATCHv6 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag Jiri Olsa
2025-12-31  0:49   ` kernel test robot
2026-01-10  0:36   ` Andrii Nakryiko
2025-12-30 14:50 ` [PATCHv6 bpf-next 2/9] ftrace: Make alloc_and_copy_ftrace_hash direct friendly Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 3/9] ftrace: Export some of hash related functions Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 4/9] ftrace: Add update_ftrace_direct_add function Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 5/9] ftrace: Add update_ftrace_direct_del function Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 6/9] ftrace: Add update_ftrace_direct_mod function Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 7/9] bpf: Add trampoline ip hash table Jiri Olsa
2026-01-10  0:36   ` Andrii Nakryiko
2026-01-12 21:27     ` Jiri Olsa
2026-01-13 11:02       ` Alan Maguire
2026-01-13 11:58         ` Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 8/9] ftrace: Factor ftrace_ops ops_func interface Jiri Olsa
2025-12-30 14:50 ` [PATCHv6 bpf-next 9/9] bpf,x86: Use single ftrace_ops for direct calls Jiri Olsa
2026-01-10  0:36   ` Andrii Nakryiko
2026-02-27 17:40   ` Ihor Solodrai
2026-02-27 20:37     ` Jiri Olsa
2026-02-27 21:24       ` Jiri Olsa [this message]
2026-02-27 22:00         ` Ihor Solodrai
2026-02-28 20:39         ` Steven Rostedt
2026-03-02  8:08           ` Jiri Olsa
2026-03-02 15:10             ` Steven Rostedt
2026-01-15 18:54 ` [PATCHv6 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Andrii Nakryiko
2026-01-26  9:48   ` Jiri Olsa
2026-01-28 14:48 ` Steven Rostedt
2026-01-28 20:00 ` patchwork-bot+netdevbpf

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=aaILlSuEgaYDq41r@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=ihor.solodrai@linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=memxor@gmail.com \
    --cc=menglong8.dong@gmail.com \
    --cc=revest@google.com \
    --cc=rostedt@kernel.org \
    --cc=song@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.