Re: [PATCH v2] tracing/user_events: Don't destroy fields when event removal fails
From: sashiko-bot@kernel.org
Date: 2026-09-04 12:12:15
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Data race leading to out-of-bounds read and BPF context size check bypass when event removal races with BPF attach. -- commit 67af0e963857d01d3ef2cc1bd224b3d373fe0483 Author: Henry Martin [off-list ref] tracing/user_events: Don't destroy fields when event removal fails destroy_user_event() previously destroyed fields before attempting to remove the trace event call. This patch moves the field destruction after the call removal and temporarily moves the fields to a local list to prevent them from being irreversibly destroyed if the removal fails.
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c index 2bbc89d4a2666..dbd8e9f95a4a7 100644 --- a/kernel/trace/trace_events_user.c +++ b/kernel/trace/trace_events_user.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1499,17 +1498,32 @@ static int user_event_set_call_visible(struct user_event *user, bool visible) static int destroy_user_event(struct user_event *user) { + LIST_HEAD(fields); int ret = 0; lockdep_assert_held(&event_mutex); - /* Must destroy fields before call removal */ - user_event_destroy_fields(user); + /* + * Detach the fields before removing the call. Removing the event + * frees the field list memory (trace_destroy_fields() is run on + * successful removal and kmem_cache_free()s the fields), but the + * fields here are allocated and owned by user_events. Destroy + * them separately once removal has succeeded. + */ + list_splice_init(&user->fields, &fields);
[Severity: High]
This is a pre-existing issue, but both the old behavior and this new
list_splice_init() leave the published field list temporarily empty while
the event is still active. Does this create a data race with concurrent
BPF attaches?
A concurrent PERF_EVENT_IOC_SET_BPF can call trace_event_get_offsets(),
which locklessly accesses this list without holding event_mutex or
trace_event_sem:
tail = list_first_entry(head, struct ftrace_event_field, link);
return tail->offset + tail->size;
Because the list is empty at this point, list_first_entry() resolves to the
list head itself (&user->fields). This causes tail->offset and tail->size
to read garbage values from adjacent struct members (such as
user->put_work).
Could this out-of-bounds read result in the BPF verifier bypassing context
size checks, potentially allowing a kernel memory information leak?
ret = user_event_set_call_visible(user, false);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260904115223.2976446-1-bsdhenrymartin@gmail.com?part=1