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

[Steven Rostedt <rostedt@goodmis.org>]

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;
> >  }  
>