mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] tracing/probes: Fix use-after-free on field name/type of events with multiple probes
       [not found] <20260825102221.79713c98@gandalf.local.home>
@ 2026-08-26  3:00 ` Henry Martin
  2026-09-02  1:05   ` Masami Hiramatsu
  0 siblings, 1 reply; 2+ messages in thread
From: Henry Martin @ 2026-08-26  3:00 UTC (permalink / raw)
  To: rostedt
  Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel,
	Henry Martin

The fields of a probe-based dynamic event (kprobe, uprobe, eprobe and
fprobe events) are created in traceprobe_define_arg_fields() by handing
the probe_arg name/type strings to trace_define_field(), which only
stores the pointers without copying. Those strings are owned by the
trace_probe and are freed when that probe is removed.

An event can have several probes attached. The field list is defined
only once, by the first probe that registers the event, but it is kept
alive by any surviving sibling probe. Deleting just that first probe by
symbol -

  # primary A: fields are defined from A's args
  echo 'p:kprobes/ev vfs_read  a1=$arg1' >  kprobe_events
  # append B: shares A's event call
  echo 'p:kprobes/ev vfs_write a1=$arg1' >> kprobe_events
  # delete only A (matched by symbol), B survives
  echo '-:kprobes/ev vfs_read'           >> kprobe_events

frees A's args (trace_probe_cleanup() -> traceprobe_free_probe_arg()),
but trace_probe_unlink() keeps the trace_probe_event because the probe
list is not empty. The event call stays registered via B while its
fields now reference freed memory. Any field lookup then reads it, e.g.

  echo 'a1 == 1' > events/kprobes/ev/filter

  BUG: KASAN: slab-use-after-free in strcmp+0xa7/0xb0
  Call Trace:
   strcmp
   trace_find_event_field
   parse_pred
   process_preds
   create_filter
   apply_event_filter
   event_filter_write

field->name references parg->name (kstrdup'd, freed with the probe) and,
for array arguments, field->type references parg->fmt (kmalloc'd, freed
with the probe) - the scalar type otherwise points at the static
fmttype rodata, which is safe.

Have traceprobe_define_arg_fields() duplicate the name and type strings
and anchor the copies on the trace_probe_event, which embeds the event
call and outlives every individual probe; trace_probe_event_free()
releases them.

The reproducer above triggers reliably; the field lookup and the delete
both run under event_mutex, so this is a dangling reference after
removal rather than a race.

The issue was found by the autokbug dynamic kernel fuzzer at Tencent
Yunding Lab.

Fixes: ca89bc071d5e4 ("tracing/kprobe: Add multi-probe per event support")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
v4:
 - Drop the redundant "added by the Fixes: commit below" and reword the
   code comment ("an event with multiple probes attached"; clearer last
   sentence), per Steve's review.
 - Steve: drop the sentence explaining the move from the previous
   version from the changelog.
 - sashiko-bot (ack'd by Steve): traceprobe_define_arg_fields() may be
   called again after a failed first attempt, since event_define_fields()
   ignores this hook's return value. Freeing and resetting the leftover
   duplicates at entry avoids leaking the previous array and writing
   past the new one.
v3:
 - Move the fix out of trace_events.c into the probe layer
   (traceprobe_define_arg_fields()/trace_probe_event_free()). Ownership
   lives on trace_probe_event, whose lifetime matches the field list.
 - Clarify this is kprobe multi-probe-per-event, not eprobes, and add a
   shell reproducer.
v2:
 - Changelog wording (superseded by v3).

 kernel/trace/trace_probe.c | 48 ++++++++++++++++++++++++++++++++++++-
 kernel/trace/trace_probe.h |  2 ++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c4163904ba747..0dfeb6d5eec07 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -2552,19 +2552,60 @@ int traceprobe_set_print_fmt(struct trace_probe *tp, enum probe_print_type ptype
 int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 				 size_t offset, struct trace_probe *tp)
 {
+	struct trace_probe_event *tpe = trace_probe_event_from_call(event_call);
 	int ret, i;
 
+	/*
+	 * A field created by trace_define_field() only stores the name and
+	 * type pointers, it does not copy the strings. Here they point into
+	 * the probe_arg of @tp, which is freed when @tp is removed. For an
+	 * event with multiple probes attached, the field list is defined
+	 * once by the first probe but kept alive by the surviving siblings,
+	 * so removing that first probe would leave the fields referencing
+	 * freed memory. Duplicate the strings and anchor the copies on the
+	 * trace_probe_event, which lives as long as the field list itself.
+	 *
+	 * event_define_fields() ignores the return value of this hook, so
+	 * if a previous attempt failed before creating any field, it may
+	 * call here again. Release duplicates left behind by such an
+	 * attempt before starting over.
+	 */
+	for (i = 0; i < tpe->nr_field_strings; i++)
+		kfree(tpe->field_strings[i]);
+	kfree(tpe->field_strings);
+	tpe->field_strings = NULL;
+	tpe->nr_field_strings = 0;
+
+	if (tp->nr_args) {
+		tpe->field_strings = kcalloc(tp->nr_args * 2, sizeof(char *),
+					     GFP_KERNEL);
+		if (!tpe->field_strings)
+			return -ENOMEM;
+	}
+
 	/* Set argument names as fields */
 	for (i = 0; i < tp->nr_args; i++) {
 		struct probe_arg *parg = &tp->args[i];
 		const char *fmt = parg->type->fmttype;
 		int size = parg->type->size;
+		char *name, *type;
 
 		if (parg->fmt)
 			fmt = parg->fmt;
 		if (parg->count)
 			size *= parg->count;
-		ret = trace_define_field(event_call, fmt, parg->name,
+
+		name = kstrdup(parg->name, GFP_KERNEL);
+		type = kstrdup(fmt, GFP_KERNEL);
+		if (!name || !type) {
+			kfree(name);
+			kfree(type);
+			return -ENOMEM;
+		}
+		tpe->field_strings[tpe->nr_field_strings++] = name;
+		tpe->field_strings[tpe->nr_field_strings++] = type;
+
+		ret = trace_define_field(event_call, type, name,
 					 offset + parg->offset, size,
 					 parg->type->is_signed,
 					 FILTER_OTHER);
@@ -2576,6 +2617,11 @@ int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 
 static void trace_probe_event_free(struct trace_probe_event *tpe)
 {
+	int i;
+
+	for (i = 0; i < tpe->nr_field_strings; i++)
+		kfree(tpe->field_strings[i]);
+	kfree(tpe->field_strings);
 	kfree(tpe->class.system);
 	kfree(tpe->call.name);
 	kfree(tpe->call.print_fmt);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index fba1af092a9bd..d1fb3520700fb 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -264,6 +264,8 @@ struct trace_probe_event {
 	struct trace_event_call		call;
 	struct list_head 		files;
 	struct list_head		probes;
+	char				**field_strings;
+	int				nr_field_strings;
 	struct trace_uprobe_filter	filter[];
 };
 
-- 
2.43.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v4] tracing/probes: Fix use-after-free on field name/type of events with multiple probes
  2026-08-26  3:00 ` [PATCH v4] tracing/probes: Fix use-after-free on field name/type of events with multiple probes Henry Martin
@ 2026-09-02  1:05   ` Masami Hiramatsu
  0 siblings, 0 replies; 2+ messages in thread
From: Masami Hiramatsu @ 2026-09-02  1:05 UTC (permalink / raw)
  To: Henry Martin
  Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel

On Wed, 26 Aug 2026 11:00:09 +0800
Henry Martin <bsdhenrymartin@gmail.com> wrote:

> The fields of a probe-based dynamic event (kprobe, uprobe, eprobe and
> fprobe events) are created in traceprobe_define_arg_fields() by handing
> the probe_arg name/type strings to trace_define_field(), which only
> stores the pointers without copying. Those strings are owned by the
> trace_probe and are freed when that probe is removed.
> 
> An event can have several probes attached. The field list is defined
> only once, by the first probe that registers the event, but it is kept
> alive by any surviving sibling probe. Deleting just that first probe by
> symbol -
> 
>   # primary A: fields are defined from A's args
>   echo 'p:kprobes/ev vfs_read  a1=$arg1' >  kprobe_events
>   # append B: shares A's event call
>   echo 'p:kprobes/ev vfs_write a1=$arg1' >> kprobe_events
>   # delete only A (matched by symbol), B survives
>   echo '-:kprobes/ev vfs_read'           >> kprobe_events
> 
> frees A's args (trace_probe_cleanup() -> traceprobe_free_probe_arg()),
> but trace_probe_unlink() keeps the trace_probe_event because the probe
> list is not empty. The event call stays registered via B while its
> fields now reference freed memory. Any field lookup then reads it, e.g.
> 
>   echo 'a1 == 1' > events/kprobes/ev/filter
> 
>   BUG: KASAN: slab-use-after-free in strcmp+0xa7/0xb0
>   Call Trace:
>    strcmp
>    trace_find_event_field
>    parse_pred
>    process_preds
>    create_filter
>    apply_event_filter
>    event_filter_write
> 
> field->name references parg->name (kstrdup'd, freed with the probe) and,
> for array arguments, field->type references parg->fmt (kmalloc'd, freed
> with the probe) - the scalar type otherwise points at the static
> fmttype rodata, which is safe.
> 
> Have traceprobe_define_arg_fields() duplicate the name and type strings
> and anchor the copies on the trace_probe_event, which embeds the event
> call and outlives every individual probe; trace_probe_event_free()
> releases them.
> 
> The reproducer above triggers reliably; the field lookup and the delete
> both run under event_mutex, so this is a dangling reference after
> removal rather than a race.
> 
> The issue was found by the autokbug dynamic kernel fuzzer at Tencent
> Yunding Lab.
> 
> Fixes: ca89bc071d5e4 ("tracing/kprobe: Add multi-probe per event support")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>

Thanks! this looks good to me.

Hmm, maybe I need to update kselftest to check this issue.
Since the current multiprobe test case does not set any argument, this
was not caught. I have checked this patch with below update.

Let me pick this fix.

Thank you,

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_multiprobe.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_multiprobe.tc
index f0d5b7777ed7..10633d54130c 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_multiprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_multiprobe.tc
@@ -30,3 +30,27 @@ cat kprobe_events | grep "$DEF2"
 
 :;: "Appending different type must fail" ;:
 ! echo "$DEF1 \$stack" >> kprobe_events
+
+:;: "Remove remaining probe" ;:
+echo "-:$EVENT_NAME" >> kprobe_events
+
+:;: "Define multiprobe with arguments and verify format and filter after primary removal" ;:
+DEF1_ARG="p:$EVENT_NAME $SYM1 a1=\$stack"
+DEF2_ARG="p:$EVENT_NAME $SYM2 a1=\$stack"
+echo $DEF1_ARG >> kprobe_events
+echo $DEF2_ARG >> kprobe_events
+
+# Remove primary probe that defined the fields
+echo "-:$EVENT_NAME $SYM1" >> kprobe_events
+grep -q "$DEF2_ARG" kprobe_events
+! grep -q "$DEF1_ARG" kprobe_events
+
+# Verify format and filter on remaining event (reads field->name and field->type)
+cat events/$EVENT_NAME/format > /dev/null
+echo 'a1 == 0' > events/$EVENT_NAME/filter
+echo 0 > events/$EVENT_NAME/filter
+
+# Clean up
+echo "-:$EVENT_NAME" >> kprobe_events
+test `cat kprobe_events | wc -l` -eq 0
+



Thanks,

> ---
> v4:
>  - Drop the redundant "added by the Fixes: commit below" and reword the
>    code comment ("an event with multiple probes attached"; clearer last
>    sentence), per Steve's review.
>  - Steve: drop the sentence explaining the move from the previous
>    version from the changelog.
>  - sashiko-bot (ack'd by Steve): traceprobe_define_arg_fields() may be
>    called again after a failed first attempt, since event_define_fields()
>    ignores this hook's return value. Freeing and resetting the leftover
>    duplicates at entry avoids leaking the previous array and writing
>    past the new one.
> v3:
>  - Move the fix out of trace_events.c into the probe layer
>    (traceprobe_define_arg_fields()/trace_probe_event_free()). Ownership
>    lives on trace_probe_event, whose lifetime matches the field list.
>  - Clarify this is kprobe multi-probe-per-event, not eprobes, and add a
>    shell reproducer.
> v2:
>  - Changelog wording (superseded by v3).
> 
>  kernel/trace/trace_probe.c | 48 ++++++++++++++++++++++++++++++++++++-
>  kernel/trace/trace_probe.h |  2 ++
>  2 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index c4163904ba747..0dfeb6d5eec07 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -2552,19 +2552,60 @@ int traceprobe_set_print_fmt(struct trace_probe *tp, enum probe_print_type ptype
>  int traceprobe_define_arg_fields(struct trace_event_call *event_call,
>  				 size_t offset, struct trace_probe *tp)
>  {
> +	struct trace_probe_event *tpe = trace_probe_event_from_call(event_call);
>  	int ret, i;
>  
> +	/*
> +	 * A field created by trace_define_field() only stores the name and
> +	 * type pointers, it does not copy the strings. Here they point into
> +	 * the probe_arg of @tp, which is freed when @tp is removed. For an
> +	 * event with multiple probes attached, the field list is defined
> +	 * once by the first probe but kept alive by the surviving siblings,
> +	 * so removing that first probe would leave the fields referencing
> +	 * freed memory. Duplicate the strings and anchor the copies on the
> +	 * trace_probe_event, which lives as long as the field list itself.
> +	 *
> +	 * event_define_fields() ignores the return value of this hook, so
> +	 * if a previous attempt failed before creating any field, it may
> +	 * call here again. Release duplicates left behind by such an
> +	 * attempt before starting over.
> +	 */
> +	for (i = 0; i < tpe->nr_field_strings; i++)
> +		kfree(tpe->field_strings[i]);
> +	kfree(tpe->field_strings);
> +	tpe->field_strings = NULL;
> +	tpe->nr_field_strings = 0;
> +
> +	if (tp->nr_args) {
> +		tpe->field_strings = kcalloc(tp->nr_args * 2, sizeof(char *),
> +					     GFP_KERNEL);
> +		if (!tpe->field_strings)
> +			return -ENOMEM;
> +	}
> +
>  	/* Set argument names as fields */
>  	for (i = 0; i < tp->nr_args; i++) {
>  		struct probe_arg *parg = &tp->args[i];
>  		const char *fmt = parg->type->fmttype;
>  		int size = parg->type->size;
> +		char *name, *type;
>  
>  		if (parg->fmt)
>  			fmt = parg->fmt;
>  		if (parg->count)
>  			size *= parg->count;
> -		ret = trace_define_field(event_call, fmt, parg->name,
> +
> +		name = kstrdup(parg->name, GFP_KERNEL);
> +		type = kstrdup(fmt, GFP_KERNEL);
> +		if (!name || !type) {
> +			kfree(name);
> +			kfree(type);
> +			return -ENOMEM;
> +		}
> +		tpe->field_strings[tpe->nr_field_strings++] = name;
> +		tpe->field_strings[tpe->nr_field_strings++] = type;
> +
> +		ret = trace_define_field(event_call, type, name,
>  					 offset + parg->offset, size,
>  					 parg->type->is_signed,
>  					 FILTER_OTHER);
> @@ -2576,6 +2617,11 @@ int traceprobe_define_arg_fields(struct trace_event_call *event_call,
>  
>  static void trace_probe_event_free(struct trace_probe_event *tpe)
>  {
> +	int i;
> +
> +	for (i = 0; i < tpe->nr_field_strings; i++)
> +		kfree(tpe->field_strings[i]);
> +	kfree(tpe->field_strings);
>  	kfree(tpe->class.system);
>  	kfree(tpe->call.name);
>  	kfree(tpe->call.print_fmt);
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index fba1af092a9bd..d1fb3520700fb 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -264,6 +264,8 @@ struct trace_probe_event {
>  	struct trace_event_call		call;
>  	struct list_head 		files;
>  	struct list_head		probes;
> +	char				**field_strings;
> +	int				nr_field_strings;
>  	struct trace_uprobe_filter	filter[];
>  };
>  
> -- 
> 2.43.0


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-02  1:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20260825102221.79713c98@gandalf.local.home>
2026-08-26  3:00 ` [PATCH v4] tracing/probes: Fix use-after-free on field name/type of events with multiple probes Henry Martin
2026-09-02  1:05   ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®