Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events
@ 2026-08-24  7:10 Henry Martin
  2026-08-24  7:27 ` sashiko-bot
  2026-08-24 18:43 ` Steven Rostedt
  0 siblings, 2 replies; 10+ messages in thread
From: Henry Martin @ 2026-08-24  7:10 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, linux-trace-kernel, linux-kernel, Henry Martin

Fields of a probe-based dynamic event (kprobe, uprobe and eprobe
events) are created from the argument name and type strings of the
trace_probe that first registers the event, as plain pointer
references without copying.

When several probes are appended to the same event, they share the
trace_event_call and its field list, which stays the one defined by
the primary probe. Deleting just the primary probe with
"-:group/event symbol" frees the trace_probe and its argument
strings, while the event call is kept registered by the remaining
sibling probes. field->name and field->type are left dangling, and
any field lookup - e.g. writing to events/<grp>/<ev>/filter - reads
freed memory:

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

Make the field own its strings: duplicate name and type with
kstrdup_const() in __trace_define_field() and release them with
kfree_const() in trace_destroy_fields(). Fields of static trace
events still reference their kernel/module rodata string literals
directly, as kstrdup_const()/kfree_const() only touch memory that
was actually allocated.

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>
---
 kernel/trace/trace_events.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c01b10b99f67e..ee3b93fa09ee8 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -123,8 +123,18 @@ static int __trace_define_field(struct list_head *head, const char *type,
 	if (!field)
 		return -ENOMEM;
 
-	field->name = name;
-	field->type = type;
+	field->name = kstrdup_const(name, GFP_TRACE);
+	if (!field->name) {
+		kmem_cache_free(field_cachep, field);
+		return -ENOMEM;
+	}
+
+	field->type = kstrdup_const(type, GFP_TRACE);
+	if (!field->type) {
+		kfree_const(field->name);
+		kmem_cache_free(field_cachep, field);
+		return -ENOMEM;
+	}
 
 	if (filter_type == FILTER_OTHER)
 		field->filter_type = filter_assign_type(type);
@@ -225,6 +235,8 @@ static void trace_destroy_fields(struct trace_event_call *call)
 	head = trace_get_fields(call);
 	list_for_each_entry_safe(field, next, head, link) {
 		list_del(&field->link);
+		kfree_const(field->name);
+		kfree_const(field->type);
 		kmem_cache_free(field_cachep, field);
 	}
 }
-- 
2.43.0

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

* Re: [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events
  2026-08-24  7:10 [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events Henry Martin
@ 2026-08-24  7:27 ` sashiko-bot
  2026-08-24 18:43 ` Steven Rostedt
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-24  7:27 UTC (permalink / raw)
  To: Henry Martin; +Cc: linux-trace-kernel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The commit message claims that fields of static trace events reference their kernel or module rodata string literals directly without allocation, but module strings are unconditionally duplicated.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but if a user_event's destruction fails because the trace event is busy, its fields are irreversibly destroyed, leaving the active event in a corrupted state.
--

--- Patch [1]: [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events ---
commit cf6ec74d4b06c238fc3f0cc0f2463ea2e1435590
Author: Henry Martin <bsdhenrymartin@gmail.com>

tracing: Fix use-after-free on field name/type of dynamic probe events

This commit modifies dynamic event trace field registration to duplicate
field name and type strings using kstrdup_const(). This ensures the field
owns its strings and prevents use-after-free bugs when primary probes are
deleted but event calls remain active.

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 0356cae0cf74e..13d7675bc5132 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -122,8 +122,18 @@ static int __trace_define_field(struct list_head *head, const char *type,
>  	if (!field)
>  		return -ENOMEM;
>  
> -	field->name = name;
> -	field->type = type;
> +	field->name = kstrdup_const(name, GFP_TRACE);
> +	if (!field->name) {
> +		kmem_cache_free(field_cachep, field);
> +		return -ENOMEM;
> +	}
> +
> +	field->type = kstrdup_const(type, GFP_TRACE);

[Severity: Medium]
The commit message states that fields of static trace events reference their
kernel or module rodata string literals directly, as kstrdup_const() only
touches allocated memory.

Does kstrdup_const() actually handle module rodata this way? Looking at
kstrdup_const(), it delegates to is_kernel_rodata(), which only checks core
kernel boundaries. 

Won't this cause module trace events to unconditionally duplicate their name
and type strings instead of referencing the module rodata?

> +	if (!field->type) {
> +		kfree_const(field->name);
> +		kmem_cache_free(field_cachep, field);
> +		return -ENOMEM;
> +	}
>  
>  	if (filter_type == FILTER_OTHER)

[ ... ]

While reviewing the string lifecycle in trace events, a related preexisting
issue in user events was identified:

kernel/trace/trace_events_user.c:destroy_user_event() {
        ...
        /* Must destroy fields before call removal */
        user_event_destroy_fields(user);

        ret = user_event_set_call_visible(user, false);

        if (ret)
                return ret;
        ...
}

[Severity: High]
This isn't a bug introduced by this patch, but if a user_event's destruction
fails because the trace event is busy (user_event_set_call_visible() returns
-EBUSY), the event's fields are already irreversibly destroyed.

Will this leave the active event in a corrupted state, where any subsequent
interactions with this event operate on an empty field list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824071011.3507735-1-bsdhenrymartin@gmail.com?part=1

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

* Re: [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events
  2026-08-24  7:10 [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events Henry Martin
  2026-08-24  7:27 ` sashiko-bot
@ 2026-08-24 18:43 ` Steven Rostedt
  2026-08-25 11:12   ` [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events Henry Martin
       [not found]   ` <CAEnQdOo0qHQK5veJp0Ybf3Hm3Gn2iUicL9Xj_bTNZKhoMfFOTw@mail.gmail.com>
  1 sibling, 2 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-08-24 18:43 UTC (permalink / raw)
  To: Henry Martin
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-trace-kernel,
	linux-kernel

On Mon, 24 Aug 2026 15:10:11 +0800
Henry Martin <bsdhenrymartin@gmail.com> wrote:

> Fields of a probe-based dynamic event (kprobe, uprobe and eprobe
> events) are created from the argument name and type strings of the
> trace_probe that first registers the event, as plain pointer
> references without copying.
> 
> When several probes are appended to the same event, they share the
> trace_event_call and its field list, which stays the one defined by
> the primary probe. Deleting just the primary probe with

What do you mean by "appended to the same event"? Do you mean eprobes?

Can you post a reproducer for this?

> "-:group/event symbol" frees the trace_probe and its argument
> strings, while the event call is kept registered by the remaining
> sibling probes. field->name and field->type are left dangling, and
> any field lookup - e.g. writing to events/<grp>/<ev>/filter - reads
> freed memory:
> 
>   BUG: KASAN: slab-use-after-free in strcmp+0xa7/0xb0
>   Call trace:
>    trace_find_event_field+0xd6/0x220
>    parse_pred
>    process_preds
>    create_filter
>    apply_event_filter
>    event_filter_write
> 
> Make the field own its strings: duplicate name and type with
> kstrdup_const() in __trace_define_field() and release them with
> kfree_const() in trace_destroy_fields(). Fields of static trace
> events still reference their kernel/module rodata string literals
> directly, as kstrdup_const()/kfree_const() only touch memory that
> was actually allocated.

Wrong fix.

> 
> 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>
> ---
>  kernel/trace/trace_events.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index c01b10b99f67e..ee3b93fa09ee8 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c

This is a bug with trace_probes.c and not trace_events.c. This should be
fixed without touching trace_events.c. That is, the trace_probes.c code (or
trace_eprobes.c if it's only affects eprobes) should handle this issue.

If you had an example, I could have figured out exactly where to place the
fix.

-- Steve

> @@ -123,8 +123,18 @@ static int __trace_define_field(struct list_head *head, const char *type,
>  	if (!field)
>  		return -ENOMEM;
>  
> -	field->name = name;
> -	field->type = type;
> +	field->name = kstrdup_const(name, GFP_TRACE);
> +	if (!field->name) {
> +		kmem_cache_free(field_cachep, field);
> +		return -ENOMEM;
> +	}
> +
> +	field->type = kstrdup_const(type, GFP_TRACE);
> +	if (!field->type) {
> +		kfree_const(field->name);
> +		kmem_cache_free(field_cachep, field);
> +		return -ENOMEM;
> +	}
>  
>  	if (filter_type == FILTER_OTHER)
>  		field->filter_type = filter_assign_type(type);
> @@ -225,6 +235,8 @@ static void trace_destroy_fields(struct trace_event_call *call)
>  	head = trace_get_fields(call);
>  	list_for_each_entry_safe(field, next, head, link) {
>  		list_del(&field->link);
> +		kfree_const(field->name);
> +		kfree_const(field->type);
>  		kmem_cache_free(field_cachep, field);
>  	}
>  }


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

* [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events
  2026-08-24 18:43 ` Steven Rostedt
@ 2026-08-25 11:12   ` Henry Martin
  2026-08-25 11:37     ` sashiko-bot
  2026-08-25 13:51     ` Steven Rostedt
       [not found]   ` <CAEnQdOo0qHQK5veJp0Ybf3Hm3Gn2iUicL9Xj_bTNZKhoMfFOTw@mail.gmail.com>
  1 sibling, 2 replies; 10+ messages in thread
From: Henry Martin @ 2026-08-25 11:12 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 hold several probes ("multi-probe per event", added by the
Fixes: commit below). 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.

Fix it in the probe layer, which is where the borrowing happens, so that
trace_define_field() and static trace events are left untouched. Make
traceprobe_define_arg_fields() duplicate the name and type strings and
have the trace_probe_event - which embeds the event call and outlives
every individual probe - own the copies, releasing them in
trace_probe_event_free().

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>
---
v3:
 - Steve: wrong fix / wrong file. Move the fix out of trace_events.c
   into the probe layer (traceprobe_define_arg_fields() /
   trace_probe_event_free()) so trace_define_field() and static events
   are untouched. Ownership now lives on trace_probe_event, whose
   lifetime matches the event call and its field list.
 - Clarify in the changelog that this is kprobe multi-probe-per-event
   (append_trace_kprobe), not eprobes, and add a shell reproducer.
v2:
 - Reworded the module-rodata note (dropped, superseded by v3).

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

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c4163904ba747..26a9fb3533bde 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -2552,19 +2552,48 @@ 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 a
+	 * multi-probe event 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. Make the
+	 * event own duplicates that live as long as the event call itself.
+	 */
+	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 +2605,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 related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events
  2026-08-25 11:12   ` [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events Henry Martin
@ 2026-08-25 11:37     ` sashiko-bot
  2026-08-25 14:22       ` Steven Rostedt
  2026-08-25 13:51     ` Steven Rostedt
  1 sibling, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-08-25 11:37 UTC (permalink / raw)
  To: Henry Martin; +Cc: linux-trace-kernel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Error handling in `traceprobe_define_arg_fields()` does not clean up `tpe->field_strings` or reset `tpe->nr_field_strings` on failure, leading to a memory leak, slab out-of-bounds write, and arbitrary free upon retry.
--

--- Patch [1]: [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events ---
commit b40c9f727280d82f97af5f38c001f0fd57efcf5e
Author: Henry Martin <bsdhenrymartin@gmail.com>

tracing/probes: Fix use-after-free on field name/type of multi-probe events

This commit addresses a use-after-free issue where an event's fields might
reference freed memory after one of its probes is removed. It fixes this by
allocating a duplicate set of name and type strings directly owned by the
trace_probe_event, ensuring they outlive any individual probe.

> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 36dff277de464..3057d31c57376 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -965,19 +965,48 @@ 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 a
> +	 * multi-probe event 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. Make the
> +	 * event own duplicates that live as long as the event call itself.
> +	 */
> +	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);
> +		if (ret)
> +			return ret;

[Severity: High]
If trace_define_field() fails, does this error path leak the newly allocated
strings and the tpe->field_strings array?

Looking at event_define_fields() in kernel/trace/trace_events.c, the return
value from this function is ignored:

    if (field->type == TRACE_FUNCTION_TYPE) {
        field->define_fields(call);
        break;
    }

Since the return value is ignored and the event's field list remains empty
if the first argument fails, couldn't the system invoke this function again 
on retry if there are multiple trace array instances?

If it is retried, the initial kcalloc() unconditionally overwrites 
tpe->field_strings, leaking the old array.

Additionally, since tpe->nr_field_strings is not reset on failure, wouldn't 
a retry start appending string pointers out of bounds of the new array?  
This appears like it could eventually cause trace_probe_event_free() to read 
out-of-bounds pointers and pass them to kfree().

>  	}
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825111210.3271443-1-bsdhenrymartin@gmail.com?part=1

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

* Re: [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events
       [not found]   ` <CAEnQdOo0qHQK5veJp0Ybf3Hm3Gn2iUicL9Xj_bTNZKhoMfFOTw@mail.gmail.com>
@ 2026-08-25 13:38     ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-08-25 13:38 UTC (permalink / raw)
  To: henry martin
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-trace-kernel,
	linux-kernel

On Tue, 25 Aug 2026 19:03:03 +0800
henry martin <bsdhenrymartin@gmail.com> wrote:

> > >
> > > When several probes are appended to the same event, they share the
> > > trace_event_call and its field list, which stays the one defined by
> > > the primary probe. Deleting just the primary probe with  
> >
> > What do you mean by "appended to the same event"? Do you mean eprobes?
> >  
> 
> Not eprobes -- I mean the kprobe multi-probe-per-event feature from the
> Fixes: commit (append_trace_kprobe()): two probes registered under one
> event name with identical arg names/types but different symbols, so they
> share a single trace_event_call. eprobe/uprobe/fprobe are affected too
> only because they all define their fields through the same helper
> (traceprobe_define_arg_fields()), but the reproducer below is plain
> kprobe.
> 
> > Can you post a reproducer for this?  
> 
> Run as root with KASAN, inside the guest:

Does it matter being inside a guest?

> 
>   cd /sys/kernel/tracing
>   # 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
>   # field lookup -> strcmp() on the freed name
>   echo 'a1 == 1' > events/kprobes/ev/filter
> 
>   BUG: KASAN: slab-use-after-free in strcmp+0xa7/0xb0
>    trace_find_event_field
>    parse_pred
>    process_preds
>    create_filter
>    apply_event_filter
>    event_filter_write
> 
>   Freed by: traceprobe_free_probe_arg / trace_probe_cleanup /
>             free_trace_kprobe / ... / dyn_event_release

The above is useful information to include in the change log.

> 
> Deleting A runs trace_probe_cleanup(A), which frees A's args, then
> trace_probe_unlink(A) keeps the trace_probe_event because B is still on
> the probe list. The event survives via B while its fields still point at
> A's freed arg->name (and, for array args, arg->fmt). Both the delete and
> the filter write hold event_mutex, so it is a dangling reference after
> removal, not a race -- it triggers every time.
> 
> >  
> > > "-:group/event symbol" frees the trace_probe and its argument
> > > strings, while the event call is kept registered by the remaining
> > > sibling probes. field->name and field->type are left dangling, and
> > > any field lookup - e.g. writing to events/<grp>/<ev>/filter - reads
> > > freed memory:
> > >
> > >   BUG: KASAN: slab-use-after-free in strcmp+0xa7/0xb0
> > >   Call trace:
> > >    trace_find_event_field+0xd6/0x220
> > >    parse_pred
> > >    process_preds
> > >    create_filter
> > >    apply_event_filter
> > >    event_filter_write
> > >
> > > Make the field own its strings: duplicate name and type with
> > > kstrdup_const() in __trace_define_field() and release them with
> > > kfree_const() in trace_destroy_fields(). Fields of static trace
> > > events still reference their kernel/module rodata string literals
> > > directly, as kstrdup_const()/kfree_const() only touch memory that
> > > was actually allocated.  
> >
> > Wrong fix.
> >  
> > >
> > > 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>
> > > ---
> > >  kernel/trace/trace_events.c | 14 ++++++++++++--
> > >  1 file changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> > > index c01b10b99f67e..ee3b93fa09ee8 100644
> > > --- a/kernel/trace/trace_events.c
> > > +++ b/kernel/trace/trace_events.c  
> >
> > This is a bug with trace_probes.c and not trace_events.c. This should be
> > fixed without touching trace_events.c. That is, the trace_probes.c code
> >  
> (or
> > trace_eprobes.c if it's only affects eprobes) should handle this issue.
> >  
> 
> Agreed, that was the wrong place. v3 keeps trace_define_field() and all
> static events untouched and fixes it where the borrowing happens:
> traceprobe_define_arg_fields() now kstrdup()s the name/type, and the
> copies are owned by the trace_probe_event (which embeds the event call
> and outlives every individual probe), freed in trace_probe_event_free().
> It is one helper plus its teardown, both in trace_probe.c, plus two
> fields on struct trace_probe_event.
> 
> v3 is posted as a reply in this thread, tested with KASAN +

Please post new versions as a separate thread. It helps with tooling.

> kasan_multi_shot. The reproducer above triggers the UAF reliably on the
> unpatched tree; with the patch, deleting the primary probe leaves
> format/filter intact on the surviving event and the report is gone. I
> also checked the array-arg
> case (arr=+0($arg1):u64[2]), where field->type borrows the kmalloc'd
> parg->fmt: the type string survives the primary delete and full teardown
> afterwards shows no double-free.
> 
> Thanks for the review.

I'll look at your other patch.

Thanks,

-- Steve

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

* Re: [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events
  2026-08-25 11:12   ` [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events Henry Martin
  2026-08-25 11:37     ` sashiko-bot
@ 2026-08-25 13:51     ` Steven Rostedt
  1 sibling, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-08-25 13:51 UTC (permalink / raw)
  To: Henry Martin
  Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel

On Tue, 25 Aug 2026 19:12:10 +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 hold several probes ("multi-probe per event", added by the
> Fixes: commit below).

You can remove the ", added by the Fixes: commit below" as that is already
assumed when there's a Fixes tag.

> 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.
> 
> Fix it in the probe layer, which is where the borrowing happens, so that
> trace_define_field() and static trace events are left untouched. Make

You can remove that first sentence. It's not useful information for this
commit. It's just stating why you changed it from a previous version.

> traceprobe_define_arg_fields() duplicate the name and type strings and
> have the trace_probe_event - which embeds the event call and outlives
> every individual probe - own the copies, releasing them in
> trace_probe_event_free().
> 
> 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>
> ---
> v3:
>  - Steve: wrong fix / wrong file. Move the fix out of trace_events.c
>    into the probe layer (traceprobe_define_arg_fields() /
>    trace_probe_event_free()) so trace_define_field() and static events
>    are untouched. Ownership now lives on trace_probe_event, whose
>    lifetime matches the event call and its field list.
>  - Clarify in the changelog that this is kprobe multi-probe-per-event
>    (append_trace_kprobe), not eprobes, and add a shell reproducer.
> v2:
>  - Reworded the module-rodata note (dropped, superseded by v3).
> 
>  kernel/trace/trace_probe.c | 36 +++++++++++++++++++++++++++++++++++-
>  kernel/trace/trace_probe.h |  2 ++
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index c4163904ba747..26a9fb3533bde 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -2552,19 +2552,48 @@ 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 a
> +	 * multi-probe event the field list is defined once by the first probe

Nit. I find "multi-probe event" somewhat ambiguous. Could you reword that
to "an event with multiple probes attached," ?

> +	 * but kept alive by the surviving siblings, so removing that first
> +	 * probe would leave the fields referencing freed memory. Make the
> +	 * event own duplicates that live as long as the event call itself.

The last sentence doesn't parse for me. Can you reword it?

Other than that, the rest looks good.

Masami, have any thoughts?

-- Steve


> +	 */
> +	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 +2605,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[];
>  };
>  


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

* Re: [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events
  2026-08-25 11:37     ` sashiko-bot
@ 2026-08-25 14:22       ` Steven Rostedt
  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-08-26  3:11         ` [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events henry martin
  0 siblings, 2 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-08-25 14:22 UTC (permalink / raw)
  To: sashiko-bot; +Cc: sashiko-reviews, Henry Martin, linux-trace-kernel

On Tue, 25 Aug 2026 11:37:13 +0000
sashiko-bot@kernel.org wrote:

> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index 36dff277de464..3057d31c57376 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -965,19 +965,48 @@ 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 a
> > +	 * multi-probe event 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. Make the
> > +	 * event own duplicates that live as long as the event call itself.
> > +	 */
> > +	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);
> > +		if (ret)
> > +			return ret;  
> 
> [Severity: High]
> If trace_define_field() fails, does this error path leak the newly allocated
> strings and the tpe->field_strings array?
> 
> Looking at event_define_fields() in kernel/trace/trace_events.c, the return
> value from this function is ignored:
> 
>     if (field->type == TRACE_FUNCTION_TYPE) {
>         field->define_fields(call);
>         break;
>     }
> 
> Since the return value is ignored and the event's field list remains empty
> if the first argument fails, couldn't the system invoke this function again 
> on retry if there are multiple trace array instances?
> 
> If it is retried, the initial kcalloc() unconditionally overwrites 
> tpe->field_strings, leaking the old array.
> 
> Additionally, since tpe->nr_field_strings is not reset on failure, wouldn't 
> a retry start appending string pointers out of bounds of the new array?  
> This appears like it could eventually cause trace_probe_event_free() to read 
> out-of-bounds pointers and pass them to kfree().
> 

The above looks legit too. Please address it.

-- Steve


> >  	}
> >  	return 0;
> >  }  
> 


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

* [PATCH v4] tracing/probes: Fix use-after-free on field name/type of events with multiple probes
  2026-08-25 14:22       ` Steven Rostedt
@ 2026-08-26  3:00         ` Henry Martin
  2026-08-26  3:11         ` [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events henry martin
  1 sibling, 0 replies; 10+ 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 related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events
  2026-08-25 14:22       ` Steven Rostedt
  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-08-26  3:11         ` henry martin
  1 sibling, 0 replies; 10+ messages in thread
From: henry martin @ 2026-08-26  3:11 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: sashiko-bot, sashiko-reviews, linux-trace-kernel

[1]: <20260826030009.1855331-1-bsdhenrymartin@gmail.com>

Thanks,

-- Henry


Steven Rostedt <rostedt@goodmis.org> 于2026年8月25日周二 22:21写道:
>
> On Tue, 25 Aug 2026 11:37:13 +0000
> sashiko-bot@kernel.org wrote:
>
> > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > > index 36dff277de464..3057d31c57376 100644
> > > --- a/kernel/trace/trace_probe.c
> > > +++ b/kernel/trace/trace_probe.c
> > > @@ -965,19 +965,48 @@ 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 a
> > > +    * multi-probe event 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. Make the
> > > +    * event own duplicates that live as long as the event call itself.
> > > +    */
> > > +   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);
> > > +           if (ret)
> > > +                   return ret;
> >
> > [Severity: High]
> > If trace_define_field() fails, does this error path leak the newly allocated
> > strings and the tpe->field_strings array?
> >
> > Looking at event_define_fields() in kernel/trace/trace_events.c, the return
> > value from this function is ignored:
> >
> >     if (field->type == TRACE_FUNCTION_TYPE) {
> >         field->define_fields(call);
> >         break;
> >     }
> >
> > Since the return value is ignored and the event's field list remains empty
> > if the first argument fails, couldn't the system invoke this function again
> > on retry if there are multiple trace array instances?
> >
> > If it is retried, the initial kcalloc() unconditionally overwrites
> > tpe->field_strings, leaking the old array.
> >
> > Additionally, since tpe->nr_field_strings is not reset on failure, wouldn't
> > a retry start appending string pointers out of bounds of the new array?
> > This appears like it could eventually cause trace_probe_event_free() to read
> > out-of-bounds pointers and pass them to kfree().
> >
>
> The above looks legit too. Please address it.
>
> -- Steve
>
>
> > >     }
> > >     return 0;
> > >  }
> >
>

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

end of thread, other threads:[~2026-08-26  3:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24  7:10 [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events Henry Martin
2026-08-24  7:27 ` sashiko-bot
2026-08-24 18:43 ` Steven Rostedt
2026-08-25 11:12   ` [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events Henry Martin
2026-08-25 11:37     ` sashiko-bot
2026-08-25 14:22       ` Steven Rostedt
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-08-26  3:11         ` [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events henry martin
2026-08-25 13:51     ` Steven Rostedt
     [not found]   ` <CAEnQdOo0qHQK5veJp0Ybf3Hm3Gn2iUicL9Xj_bTNZKhoMfFOTw@mail.gmail.com>
2026-08-25 13:38     ` [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events Steven Rostedt

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