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

Henry Martin posted 1 patch 1 month ago
There is a newer version of this series
kernel/trace/trace_events.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
[PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events
Posted by Henry Martin 1 month ago
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