Re: [PATCH] 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:

New issues:
- [Medium] The commit message claims that fields of static trace events reference their kernel or module rodata string literals directly without allocation, but module strings are unconditionally duplicated.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but if a user_event's destruction fails because the trace event is busy, its fields are irreversibly destroyed, leaving the active event in a corrupted state.
--

--- Patch [1]: [PATCH] tracing: Fix use-after-free on field name/type of dynamic probe events ---
commit cf6ec74d4b06c238fc3f0cc0f2463ea2e1435590
Author: Henry Martin <bsdhenrymartin@gmail.com>

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

This commit modifies dynamic event trace field registration to duplicate
field name and type strings using kstrdup_const(). This ensures the field
owns its strings and prevents use-after-free bugs when primary probes are
deleted but event calls remain active.

> 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]
The commit message states that fields of static trace events reference their
kernel or module rodata string literals directly, as kstrdup_const() only
touches allocated memory.

Does kstrdup_const() actually handle module rodata this way? Looking at
kstrdup_const(), it delegates to is_kernel_rodata(), which only checks core
kernel boundaries. 

Won't this cause module trace events to unconditionally duplicate their name
and type strings instead of referencing the module rodata?

> +	if (!field->type) {
> +		kfree_const(field->name);
> +		kmem_cache_free(field_cachep, field);
> +		return -ENOMEM;
> +	}
>  
>  	if (filter_type == FILTER_OTHER)

[ ... ]

While reviewing the string lifecycle in trace events, a related preexisting
issue in user events was identified:

kernel/trace/trace_events_user.c:destroy_user_event() {
        ...
        /* Must destroy fields before call removal */
        user_event_destroy_fields(user);

        ret = user_event_set_call_visible(user, false);

        if (ret)
                return ret;
        ...
}

[Severity: High]
This isn't a bug introduced by this patch, but if a user_event's destruction
fails because the trace event is busy (user_event_set_call_visible() returns
-EBUSY), the event's fields are already irreversibly destroyed.

Will this leave the active event in a corrupted state, where any subsequent
interactions with this event operate on an empty field list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824071011.3507735-1-bsdhenrymartin@gmail.com?part=1