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