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 <[email protected]> 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. > 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 [ ... ] > @@ -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/[email protected]?part=1
