linux-kernel - [PATCH tip/perf] uprobes: avoid false lockdep splat in uprobe timer callback

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-next>] [day] [month] [year] [list]

Message-ID: <20250403171831.3803479-1-andrii@kernel.org>
Date: Thu,  3 Apr 2025 10:18:31 -0700
From: Andrii Nakryiko <andrii@...nel.org>
To: linux-trace-kernel@...r.kernel.org,
	peterz@...radead.org,
	mingo@...nel.org
Cc: bpf@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	kernel-team@...a.com,
	rostedt@...dmis.org,
	oleg@...hat.com,
	mhiramat@...nel.org,
	bigeasy@...utronix.de,
	ast@...nel.org,
	Andrii Nakryiko <andrii@...nel.org>
Subject: [PATCH tip/perf] uprobes: avoid false lockdep splat in uprobe timer callback

Avoid a false-positive lockdep warning in PREEMPT_RT configuration when
using write_seqcount_begin() in uprobe timer callback by using
raw_write_* APIs. Uprobe's use of timer callback is guaranteed to not
race with itself, and as such seqcount's insistence on having hardirqs
disabled on the writer side is irrelevant. So switch to raw_ variants of
seqcount API instead of disabling hardirqs unnecessarily.

Also, point out in the comments more explicitly why we use seqcount
despite our reader side being rather simple and never retrying. We favor
well-maintained kernel primitive in favor of open-coding our own memory
barriers.

Link: https://lore.kernel.org/bpf/CAADnVQLLOHZmPO4X_dQ+cTaSDvzdWHzA0qUqQDhLFYL3D6xPxg@mail.gmail.com/
Reported-by: Alexei Starovoitov <ast@...nel.org>
Suggested-by: Sebastian Sewior <bigeasy@...utronix.de>
Fixes: 8622e45b5da1 ("uprobes: Reuse return_instances between multiple uretprobes within task")
Signed-off-by: Andrii Nakryiko <andrii@...nel.org>
---
 kernel/events/uprobes.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 70c84b9d7be3..6d7e7da0fbbc 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1944,6 +1944,9 @@ static void free_ret_instance(struct uprobe_task *utask,
 	 * to-be-reused return instances for future uretprobes. If ri_timer()
 	 * happens to be running right now, though, we fallback to safety and
 	 * just perform RCU-delated freeing of ri.
+	 * Admittedly, this is a rather simple use of seqcount, but it nicely
+	 * abstracts away all the necessary memory barriers, so we use
+	 * a well-supported kernel primitive here.
 	 */
 	if (raw_seqcount_try_begin(&utask->ri_seqcount, seq)) {
 		/* immediate reuse of ri without RCU GP is OK */
@@ -2004,12 +2007,18 @@ static void ri_timer(struct timer_list *timer)
 	/* RCU protects return_instance from freeing. */
 	guard(rcu)();
 
-	write_seqcount_begin(&utask->ri_seqcount);
+	/* See free_ret_instance() for notes on seqcount use.
+	 * We also employ raw API variants to avoid lockdep false-positive
+	 * warning complaining about hardirqs not being disabled. We have
+	 * a guarantee that this timer callback won't race with itself, so no
+	 * need to disable hardirqs.
+	 */
+	raw_write_seqcount_begin(&utask->ri_seqcount);
 
 	for_each_ret_instance_rcu(ri, utask->return_instances)
 		hprobe_expire(&ri->hprobe, false);
 
-	write_seqcount_end(&utask->ri_seqcount);
+	raw_write_seqcount_end(&utask->ri_seqcount);
 }
 
 static struct uprobe_task *alloc_utask(void)
-- 
2.47.1