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

[sashiko-bot@kernel.org]

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