[PATCH v1] ftrace: Fix lost recursion records in ftrace_record_recursion()

[Andrea Parri <parri.andrea@gmail.com>]

cached_function is set to the current ip before the cmpxchg() that
claims a slot in recursed_functions[]. When that cmpxchg() loses a
race for the same slot, the code bumps index and retries via "goto
again", but the retry immediately matches its own cached_function
write and returns without ever reaching the bumped-index cmpxchg().

Concretely, for two writers racing on the same index:

  CPU 0 (ip = A)                    CPU 1 (ip = B)
  --------------                    --------------
                                     cmpxchg(&recursed_functions[index].ip,
                                             0, B);      // succeeds
  cached_function = A;
  cmpxchg(&recursed_functions[index].ip,
          0, A);
    // fails, old == B
  index++;
  goto again;
  if (A == cached_function)         // true: matches A's own store
          return;                   // A is dropped, not retried

Once dropped this way, cached_function stays set to A, so every later
recursion of A also hits the fast path and returns before reaching the
retry, until some unrelated ip overwrites the cache.

Set cached_function only after the cmpxchg() has claimed the slot. The
retry path then leaves the cache untouched and no longer matches its
own store. When the cmpxchg() finds ip already present, the writer
that installed it sets the cache.

The bug was found with the help of an LLM.

Fixes: 773c16705058e ("ftrace: Add recording of functions that caused recursion")
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
Changes since RFC:
- Drop the cached_function update in the old == ip branch, as suggested
  by Steve, and adjust the changelog and the cached_function comment.
- Drop the stale "(use i for the next index)" remark.
- Drop the Assisted-by tag, which Documentation/process no longer
  documents since commit 1e035297ffdad ("docs: remove Assisted-by
  references"), and note in the changelog that an LLM found the bug.
- Rebase onto 6b4b543fa1875.

RFC: https://lore.kernel.org/r/20260917090217.6738-1-parri.andrea@gmail.com

Tested on x86_64 in a virtme-ng guest (4 CPUs) with a reproducer that
releases two kthreads to record distinct ips concurrently and checks that
both show up in recursed_functions: the base commit fails within a few
iterations, this patch passes 500 of 500.

The smp_mb__after_atomic() usage discussed on the RFC is left for a
separate patch.

 kernel/trace/trace_recursion_record.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/trace_recursion_record.c b/kernel/trace/trace_recursion_record.c
index bac4bc844ccd8..4f63bec052fca 100644
--- a/kernel/trace/trace_recursion_record.c
+++ b/kernel/trace/trace_recursion_record.c
@@ -17,8 +17,8 @@ static struct recursed_functions recursed_functions[CONFIG_FTRACE_RECORD_RECURSI
 static atomic_t nr_records;
 
 /*
- * Cache the last found function. Yes, updates to this is racey, but
- * so is memory cache ;-)
+ * Cache the last function found in or added to recursed_functions[].
+ * Updates to this are racy, but this is only a best-effort cache.
  */
 static unsigned long cached_function;
 
@@ -67,24 +67,25 @@ void ftrace_record_recursion(unsigned long ip, unsigned long parent_ip)
 		}
 	}
 
-	cached_function = ip;
-
 	/*
 	 * We only want to add a function if it hasn't been added before.
-	 * Add to the current location before incrementing the count.
-	 * If it fails to add, then increment the index (save in i)
-	 * and try again.
+	 * Claim the current slot before incrementing the count. If the slot
+	 * is occupied by another function, advance to the next slot and retry.
+	 *
+	 * Do not update cached_function until ip is known to be present;
+	 * otherwise the retry would match its own cache update.
 	 */
 	old = cmpxchg(&recursed_functions[index].ip, 0, ip);
 	if (old != 0) {
 		/* Did something else already added this for us? */
 		if (old == ip)
 			return;
-		/* Try the next location (use i for the next index) */
+		/* Try the next location */
 		index++;
 		goto again;
 	}
 
+	cached_function = ip;
 	recursed_functions[index].parent_ip = parent_ip;
 
 	/*

base-commit: 6b4b543fa18757297ac756a6fe6e64f506b7844a
-- 
2.53.0