Re: [PATCH v17 10/13] tracing/wprobe: Add set_wprobe and clear_wprobe event triggers

On Tue, 22 Sep 2026 04:43:24 +0000
sashiko-bot@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] tracing/wprobe: NMI deadlock in wprobe_trigger() when count is not specified
- [High] tracing/wprobe: CPU hotplug race in __register_trace_wprobe() misses new CPUs
- [Medium] tracing/wprobe: TOCTOU race in wprobe_trigger() corrupts watchpoint address
--

commit b9f67542e6bf772512bb8cbf3c3011379c301eb4
Author: Masami Hiramatsu (Google) [off-list ref]

tracing/wprobe: Add set_wprobe and clear_wprobe event triggers

Add set_wprobe and clear_wprobe event triggers to dynamically attach
and detach hardware breakpoint address monitoring based on event field
contents.

Link: https://lore.kernel.org/all/59637b96946653393a7ad3c7de094094796b39c2.1785067572.git.wangjinchao600@gmail.com/ (local)

quoted

diff --git a/kernel/trace/trace_wprobe.c b/kernel/trace/trace_wprobe.c
index 9c6100b63e8d9..f1d6aa320f213 100644
--- a/kernel/trace/trace_wprobe.c
+++ b/kernel/trace/trace_wprobe.c

@@ -193,20 +217,130 @@ static int __register_trace_wprobe(struct trace_wprobe *tw)

[ ... ]

quoted

 	tw->bp_event = register_wide_hw_breakpoint(&attr, wprobe_perf_handler, tw);
 	if (IS_ERR_PCPU(tw->bp_event)) {
 		int ret = PTR_ERR_PCPU(tw->bp_event);
 
 		tw->bp_event = NULL;
 		return ret;
 	}
 
+	mutex_lock(&wprobe_mutex);
+	list_add(&tw->active_list, &active_wprobes);
+	mutex_unlock(&wprobe_mutex);
+

[Severity: High]
Does this code have a race condition with CPU hotplug?

Looking at __register_trace_wprobe(), it calls register_wide_hw_breakpoint()
which installs breakpoints and releases the hotplug lock internally.

If a new CPU comes online immediately after that lock is released, but before
this wprobe is added to active_wprobes under wprobe_mutex, could the
wprobe_cpu_online() callback miss the wprobe? This would leave the new CPU
without hardware breakpoints installed for this probe.

Hmm, OK. Taking a cpus_read_lock() for both can avoid this issue.

quoted

 	return 0;
 }

[ ... ]

quoted

@@ -767,9 +923,608 @@ static int trace_wprobe_show(struct seq_file *m, struct dyn_event *ev)

[ ... ]

quoted

+static void wprobe_trigger(struct event_trigger_data *data,
+			   struct trace_buffer *buffer,  void *rec,
+			   struct ring_buffer_event *event)
+{
+	struct wprobe_trigger_data *wprobe_data = data->private_data;
+	struct trace_wprobe *tw = wprobe_data->tw;
+	unsigned long target_addr, addr = 0, flags;
+
+	if (wprobe_data->field) {
+		if (!rec)
+			return;
+		addr = *(unsigned long *)((char *)rec + wprobe_data->offset);
+		addr += wprobe_data->adjust;
+	}
+
+	raw_spin_lock_irqsave(&tw->lock, flags);
+

[Severity: High]
Could this raw_spin_lock_irqsave() cause a deadlock if interrupted by an NMI?

When no count limit is specified (EVENT_TRIGGER_FL_COUNT is not set),
wprobe_trigger() takes tw->lock before calling __wprobe_count_func().

If an NMI interrupts a CPU already holding tw->lock and hits a trace event
that fires the same uncounted trigger, it will attempt to acquire tw->lock
again here. Since the in_nmi() check is located inside __wprobe_count_func(),
the NMI would spin forever before realizing it shouldn't proceed.

Ah, indeed. It should check in_nmi() earlier than the spin lock.

quoted

+	if (!(data->flags & EVENT_TRIGGER_FL_COUNT)) {
+		if (!__wprobe_count_func(data, addr)) {
+			atomic_inc(&tw->missed);
+			goto out;
+		}
+	}

[Severity: Medium]
Is there a time-of-check to time-of-use (TOCTOU) race here for counted
triggers?

For counted triggers, wprobe_count_func() is called locklessly by the trace
core, which evaluates __wprobe_count_func(). Then, wprobe_trigger() is called
and takes tw->lock. Because EVENT_TRIGGER_FL_COUNT is set, the state
validation check (__wprobe_count_func()) is explicitly skipped under the lock.

If a counted trigger fires concurrently on multiple CPUs, both might
observe the unset address locklessly. Could the second CPU unconditionally
overwrite tw->addr, breaking the exclusive trigger constraint?

Hmm, indeed. I think the current event trigger subsystem is not well
designed for the concurrency. Let me move the data->count update to
wprobe_trigger() inside tw->lock critical region.

Thank you,

-- 
Masami Hiramatsu (Google) [off-list ref]