* [PATCH v3 1/2] tracing: add missing trace_probe_log_clear for eprobes
2025-05-04 18:27 [PATCH v3 0/2] tracing: fix race when creating trace probe log error message Paul Cacheux via B4 Relay
@ 2025-05-04 18:27 ` Paul Cacheux via B4 Relay
2025-05-07 21:31 ` Steven Rostedt
2025-05-04 18:27 ` [PATCH v3 2/2] tracing: protect trace_probe_log with mutex Paul Cacheux via B4 Relay
2025-05-07 21:33 ` [PATCH v3 0/2] tracing: fix race when creating trace probe log error message Steven Rostedt
2 siblings, 1 reply; 6+ messages in thread
From: Paul Cacheux via B4 Relay @ 2025-05-04 18:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Namhyung Kim
Cc: linux-kernel, linux-trace-kernel, Paul Cacheux
From: Paul Cacheux <paulcacheux@gmail.com>
Make sure trace_probe_log_clear is called in the tracing
eprobe code path, matching the trace_probe_log_init call.
Signed-off-by: Paul Cacheux <paulcacheux@gmail.com>
---
kernel/trace/trace_eprobe.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index c08355c3ef32b4124ac944d7e3a03efb66492269..916555f0de811f03feed9d923c63078b0e4c993b 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -969,10 +969,13 @@ static int __trace_eprobe_create(int argc, const char *argv[])
goto error;
}
}
+ trace_probe_log_clear();
return ret;
+
parse_error:
ret = -EINVAL;
error:
+ trace_probe_log_clear();
trace_event_probe_cleanup(ep);
return ret;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] tracing: protect trace_probe_log with mutex
2025-05-04 18:27 [PATCH v3 0/2] tracing: fix race when creating trace probe log error message Paul Cacheux via B4 Relay
2025-05-04 18:27 ` [PATCH v3 1/2] tracing: add missing trace_probe_log_clear for eprobes Paul Cacheux via B4 Relay
@ 2025-05-04 18:27 ` Paul Cacheux via B4 Relay
2025-05-07 21:29 ` Steven Rostedt
2025-05-07 21:33 ` [PATCH v3 0/2] tracing: fix race when creating trace probe log error message Steven Rostedt
2 siblings, 1 reply; 6+ messages in thread
From: Paul Cacheux via B4 Relay @ 2025-05-04 18:27 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Namhyung Kim
Cc: linux-kernel, linux-trace-kernel, Paul Cacheux
From: Paul Cacheux <paulcacheux@gmail.com>
The shared trace_probe_log variable can be accessed and modified
by multiple processes using tracefs at the same time, this new
mutex will guarantee it's always in a coherent state.
There is no guarantee that multiple errors happening at the same
time will each have the correct error message, but at least this
won't crash.
Fixes: ab105a4fb894 ("tracing: Use tracing error_log with probe events")
Signed-off-by: Paul Cacheux <paulcacheux@gmail.com>
---
kernel/trace/trace_probe.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 2eeecb6c95eea55502b83af6775b7b6f0cc5ab94..d538964c87a52b06646ec1b8c3469be7399f04fa 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -153,10 +153,19 @@ static const struct fetch_type *find_fetch_type(const char *type, unsigned long
return NULL;
}
+/*
+ * The trace_probe_log_lock only protects against the individual
+ * modification of the trace_probe_log. It does not protect against
+ * the log from producing garbage if two probes use it at the same
+ * time. That would only happen if two admins were trying to add
+ * probes simultaneously which they shouldn't be doing.
+ */
+static DEFINE_MUTEX(trace_probe_log_lock);
static struct trace_probe_log trace_probe_log;
void trace_probe_log_init(const char *subsystem, int argc, const char **argv)
{
+ guard(mutex)(&trace_probe_log_lock);
trace_probe_log.subsystem = subsystem;
trace_probe_log.argc = argc;
trace_probe_log.argv = argv;
@@ -165,11 +174,13 @@ void trace_probe_log_init(const char *subsystem, int argc, const char **argv)
void trace_probe_log_clear(void)
{
+ guard(mutex)(&trace_probe_log_lock);
memset(&trace_probe_log, 0, sizeof(trace_probe_log));
}
void trace_probe_log_set_index(int index)
{
+ guard(mutex)(&trace_probe_log_lock);
trace_probe_log.index = index;
}
@@ -178,6 +189,8 @@ void __trace_probe_log_err(int offset, int err_type)
char *command, *p;
int i, len = 0, pos = 0;
+ guard(mutex)(&trace_probe_log_lock);
+
if (!trace_probe_log.argv)
return;
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3 2/2] tracing: protect trace_probe_log with mutex
2025-05-04 18:27 ` [PATCH v3 2/2] tracing: protect trace_probe_log with mutex Paul Cacheux via B4 Relay
@ 2025-05-07 21:29 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2025-05-07 21:29 UTC (permalink / raw)
To: Paul Cacheux via B4 Relay
Cc: paulcacheux, Masami Hiramatsu, Mathieu Desnoyers, Namhyung Kim,
linux-kernel, linux-trace-kernel
On Sun, 04 May 2025 20:27:53 +0200
Paul Cacheux via B4 Relay <devnull+paulcacheux.gmail.com@kernel.org> wrote:
> From: Paul Cacheux <paulcacheux@gmail.com>
>
> The shared trace_probe_log variable can be accessed and modified
> by multiple processes using tracefs at the same time, this new
> mutex will guarantee it's always in a coherent state.
>
> There is no guarantee that multiple errors happening at the same
> time will each have the correct error message, but at least this
> won't crash.
>
> Fixes: ab105a4fb894 ("tracing: Use tracing error_log with probe events")
> Signed-off-by: Paul Cacheux <paulcacheux@gmail.com>
> ---
> kernel/trace/trace_probe.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] tracing: fix race when creating trace probe log error message
2025-05-04 18:27 [PATCH v3 0/2] tracing: fix race when creating trace probe log error message Paul Cacheux via B4 Relay
2025-05-04 18:27 ` [PATCH v3 1/2] tracing: add missing trace_probe_log_clear for eprobes Paul Cacheux via B4 Relay
2025-05-04 18:27 ` [PATCH v3 2/2] tracing: protect trace_probe_log with mutex Paul Cacheux via B4 Relay
@ 2025-05-07 21:33 ` Steven Rostedt
2 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2025-05-07 21:33 UTC (permalink / raw)
To: Paul Cacheux via B4 Relay
Cc: paulcacheux, Masami Hiramatsu, Mathieu Desnoyers, Namhyung Kim,
linux-kernel, linux-trace-kernel
Masami,
Do you want to take this?
-- Steve
On Sun, 04 May 2025 20:27:51 +0200
Paul Cacheux via B4 Relay <devnull+paulcacheux.gmail.com@kernel.org> wrote:
> Hello,
>
> As reported in [1] a race exists in the shared trace probe log
> used to build error messages. This can cause kernel crashes
> when building the actual error message, but the race happens
> even for non-error tracefs uses, it's just not visible.
>
> Reproducer first reported that is still crashing:
>
> # 'p4' is invalid command which make kernel run into trace_probe_log_err()
> cd /sys/kernel/debug/tracing
> while true; do
> echo 'p4:myprobe1 do_sys_openat2 dfd=%ax filename=%dx flags=%cx mode=+4($stack)' >> kprobe_events &
> echo 'p4:myprobe2 do_sys_openat2' >> kprobe_events &
> echo 'p4:myprobe3 do_sys_openat2 dfd=%ax filename=%dx' >> kprobe_events &
> done;
>
> The original email suggested to use a mutex or to allocate the
> trace_probe_log on the stack. This patch implements a simpler
> solution suggest during the review of the v1 where we only protect
> access to the shared trace_probe_log with a mutex. This will prevent
> any crash from happening.
>
> [1] https://lore.kernel.org/all/20221121081103.3070449-1-zhengyejian1@huawei.com/T/
>
> Signed-off-by: Paul Cacheux <paulcacheux@gmail.com>
> ---
> Changes in v3:
> - add some comment around the new mutex definition
> - Link to v2: https://lore.kernel.org/r/20250502-fix-trace-probe-log-race-v2-0-511ecc1521ec@gmail.com
>
> Changes in v2:
> - change approach, and use the mutex based solution suggested during
> review
> - Link to v1: https://lore.kernel.org/r/20250422-fix-trace-probe-log-race-v1-1-d2728d42cacb@gmail.com
>
> ---
> Paul Cacheux (2):
> tracing: add missing trace_probe_log_clear for eprobes
> tracing: protect trace_probe_log with mutex
>
> kernel/trace/trace_eprobe.c | 3 +++
> kernel/trace/trace_probe.c | 13 +++++++++++++
> 2 files changed, 16 insertions(+)
> ---
> base-commit: 95d3481af6dc90fd7175a7643fd108cdcb808ce5
> change-id: 20250420-fix-trace-probe-log-race-cc89d8e5fb15
>
> Best regards,
^ permalink raw reply [flat|nested] 6+ messages in thread