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

[sashiko-bot@kernel.org]

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