Re: [PATCH v3] tracing/probes: Fix use-after-free on field name/type of multi-probe events

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

quoted

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

quoted

 	}
 	return 0;
 }