Re: [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tracing: incomplete fix leaves BPF OOB read exploitable for zero-field user_events
--

commit 4bc63c3e7a59307c8dcbd3d4bfc283a9c62599d9
Author: Haoran Wang <haoranwangsec@gmail.com>

tracing: reject trace events with an empty TP_STRUCT__entry

This patch modifies event_init() to reject trace events with an empty
TP_STRUCT__entry by checking if the fields_array is empty. The fix attempts
to prevent an out-of-bounds read in trace_event_get_offsets() when BPF
programs attach to zero-field tracepoints, but it specifically exempts
events that implement a get_fields callback.

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 30c0ddf90887f..4bdd31dd24234 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3406,6 +3406,11 @@ static int event_init(struct trace_event_call *call)
>  	if (WARN_ON(!name))
>  		return -EINVAL;
>  
> +	if (call->class->fields_array &&
> +	    !call->class->fields_array[0].type &&
> +	    !call->class->get_fields)
> +		return -EINVAL;
[Severity: High]
Does this leave the out-of-bounds read exploitable for zero-field user_events?

The added check explicitly exempts events that define a get_fields callback.
The user_events subsystem defines user_event_get_fields and therefore bypasses
this new validation.

Since unprivileged users can dynamically create user_events with zero fields
(for example, via echo 'my_event' > user_events_data), user_event_parse_fields()
returns early:

kernel/trace/trace_events_user.c:user_event_parse_fields() {
    ...
    if (args == NULL)
        return 0;
    ...
}

This results in an empty user->fields list. Because user_events have the
TRACE_EVENT_FL_TRACEPOINT flag, BPF programs can attach to them.
__perf_event_set_bpf_prog() then calls the unpatched
trace_event_get_offsets(), which dereferences the empty list via
list_first_entry():

kernel/trace/trace_events.c:trace_event_get_offsets() {
    ...
    tail = list_first_entry(head, struct ftrace_event_field, link);
    return tail->offset + tail->size;
}

This reads out-of-bounds memory, bypassing BPF context size validation and
potentially allowing an unprivileged attacker to load a BPF program that
reads adjacent kernel memory during tracepoint execution.

Should there be an additional check in trace_event_get_offsets() or should
zero-field user_events be rejected entirely?

> +
>  	if (call->class->raw_init) {
>  		ret = call->class->raw_init(call);
>  		if (ret < 0 && ret != -ENOSYS)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6ad1d48d.a461c41f.282b8.7a97@mx.google.com?part=1