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

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

[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 built-in trace
events still reference their kernel rodata string literals directly,
as kstrdup_const()/kfree_const() only touch memory that was actually
allocated. Module trace events pay one extra copy per string, since
module rodata is outside the core kernel rodata range checked by
is_kernel_rodata(); the copy also makes field strings immune to
module unload edge cases (e.g. forced unload) where the module text
may be freed while its trace event structures are still referenced.

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>
---
v2: Clarify in the commit message that module rodata strings are
duplicated rather than referenced (kstrdup_const() checks only the
core kernel rodata range), and scope the module-unload benefit to
edge cases rather than the normal removal path, which already tears
down module events via the module notifier.

 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

Re: [PATCH v2] tracing: Fix use-after-free on field name/type of dynamic probe events

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double free of `field->type` string during module unload for trace events utilizing enums or sizeofs.
- [Medium] Original `field->type` string allocated via `kstrdup_const` is permanently leaked when updated.
--

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

tracing: Fix use-after-free on field name/type of dynamic probe events
    
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.

[ ... ]

> 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]
Will this original field->type string be leaked when the field uses enums
or sizeofs?

When a module defining trace events is loaded, this kstrdup_const() allocates
the string for field->type, because module rodata falls outside the core
kernel rodata range.

Later in the initialization sequence, trace_event_eval_update() invokes
update_event_fields(), which replaces field->type without freeing the
original allocation:

kernel/trace/trace_events.c:update_event_fields() {
    ...
		str = kstrdup(field->type, GFP_KERNEL);
        ...
		if (call->module)
			add_str_to_module(call->module, str);

		field->type = str;
    ...
}

Should the original dynamically allocated pointer be freed before being
overwritten here?

> +	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);

[ ... ]

> @@ -205,6 +215,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);

[Severity: High]
Could this introduce a double free during module unload for events using enums
or sizeofs?

During module load, update_event_fields() allocates a new string via kstrdup()
and assigns it to field->type, and also adds it to the module_strings list.

During module unload, trace_module_remove_events() iterates through the
module's events and calls __trace_remove_event_call(), which calls
trace_destroy_fields(). This newly added kfree_const(field->type) will free
the string allocated by update_event_fields().

Immediately after doing that, trace_module_remove_events() iterates the
module_strings list:

kernel/trace/trace_events.c:trace_module_remove_events() {
    ...
	/* Check for any strings allocade for this module */
	list_for_each_entry_safe(modstr, m, &module_strings, next) {
		if (modstr->module != mod)
			continue;
		list_del(&modstr->next);
		kfree(modstr->str);
		kfree(modstr);
	}
    ...
}

Will this cause kfree(modstr->str) to free the exact same string pointer a
second time?

>  		kmem_cache_free(field_cachep, field);
>  	}
>  }

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