linux-kernel - Re: [PATCH] tracing: fgraph: Protect return handler from recursion loop

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250921185203.561676ad@batman.local.home>
Date: Sun, 21 Sep 2025 18:52:03 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Steven Rostedt
 <rostedt@...nel.org>, Menglong Dong <menglong8.dong@...il.com>,
 jolsa@...nel.org, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 kees@...nel.org, samitolvanen@...gle.com, rppt@...nel.org, luto@...nel.org,
 ast@...nel.org, andrii@...nel.org, linux-kernel@...r.kernel.org,
 bpf@...r.kernel.org
Subject: Re: [PATCH] tracing: fgraph: Protect return handler from recursion
 loop

On Sun, 21 Sep 2025 13:05:19 +0900
Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:

>  
> > The reason I would say not to have the warn on, is because we don't have a
> > warn on for recursion happening at the entry handler. Because this now is
> > exposed by fprobe allowing different routines to be called at exit than
> > what is used in entry, it can easily be triggered.  
> 
> At the entry, if it detect recursion, it exits soon. But here we have to
> process stack unwind to get the return address. This recursion_trylock()
> is to mark this is in the critical section, not detect it.

Ah, because the first instance of the exit callback sets the recursion
bit. This will cause recursed entry calls to detect the recursion bit
and return without setting the exit handler to be called.

That is, by setting the recursion bit in the exit handler, it will cause
a recursion in entry to fail before the exit is called again.

I'd like to update the comment:

+	bit = ftrace_test_recursion_trylock(trace.func, ret);
+	/*
+	 * This must be succeeded because the entry handler returns before
+	 * modifying the return address if it is nested. Anyway, we need to
+	 * avoid calling user callbacks if it is nested.
+	 */
+	if (WARN_ON_ONCE(bit < 0))
+		goto out;
+

to:

	/*
	 * Setting the recursion bit here will cause the graph entry to
	 * detect recursion before the exit handle will. If the ext
	 * handler detects recursion, something went wrong.
	 */
	if (WARN_ON_ONCE(bit < 0))

-- Steve