[1]: <[email protected]> Thanks,
-- Henry Steven Rostedt <[email protected]> 于2026年8月25日周二 22:21写道： > > On Tue, 25 Aug 2026 11:37:13 +0000 > [email protected] wrote: > > > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c > > > index 36dff277de464..3057d31c57376 100644 > > > --- a/kernel/trace/trace_probe.c > > > +++ b/kernel/trace/trace_probe.c > > > @@ -965,19 +965,48 @@ int traceprobe_set_print_fmt(struct trace_probe > > > *tp, enum probe_print_type ptype > > > int traceprobe_define_arg_fields(struct trace_event_call *event_call, > > > size_t offset, struct trace_probe *tp) > > > { > > > + struct trace_probe_event *tpe = > > > trace_probe_event_from_call(event_call); > > > int ret, i; > > > > > > + /* > > > + * A field created by trace_define_field() only stores the name and > > > + * type pointers, it does not copy the strings. Here they point into > > > + * the probe_arg of @tp, which is freed when @tp is removed. For a > > > + * multi-probe event the field list is defined once by the first probe > > > + * but kept alive by the surviving siblings, so removing that first > > > + * probe would leave the fields referencing freed memory. Make the > > > + * event own duplicates that live as long as the event call itself. > > > + */ > > > + if (tp->nr_args) { > > > + tpe->field_strings = kcalloc(tp->nr_args * 2, sizeof(char *), > > > + GFP_KERNEL); > > > + if (!tpe->field_strings) > > > + return -ENOMEM; > > > + } > > > + > > > /* Set argument names as fields */ > > > for (i = 0; i < tp->nr_args; i++) { > > > struct probe_arg *parg = &tp->args[i]; > > > const char *fmt = parg->type->fmttype; > > > int size = parg->type->size; > > > + char *name, *type; > > > > > > if (parg->fmt) > > > fmt = parg->fmt; > > > if (parg->count) > > > size *= parg->count; > > > - ret = trace_define_field(event_call, fmt, parg->name, > > > + > > > + name = kstrdup(parg->name, GFP_KERNEL); > > > + type = kstrdup(fmt, GFP_KERNEL); > > > + if (!name || !type) { > > > + kfree(name); > > > + kfree(type); > > > + return -ENOMEM; > > > + } > > > + tpe->field_strings[tpe->nr_field_strings++] = name; > > > + tpe->field_strings[tpe->nr_field_strings++] = type; > > > + > > > + ret = trace_define_field(event_call, type, name, > > > offset + parg->offset, size, > > > parg->type->is_signed, > > > FILTER_OTHER); > > > + if (ret) > > > + return ret; > > > > [Severity: High] > > If trace_define_field() fails, does this error path leak the newly allocated > > strings and the tpe->field_strings array? > > > > Looking at event_define_fields() in kernel/trace/trace_events.c, the return > > value from this function is ignored: > > > > if (field->type == TRACE_FUNCTION_TYPE) { > > field->define_fields(call); > > break; > > } > > > > Since the return value is ignored and the event's field list remains empty > > if the first argument fails, couldn't the system invoke this function again > > on retry if there are multiple trace array instances? > > > > If it is retried, the initial kcalloc() unconditionally overwrites > > tpe->field_strings, leaking the old array. > > > > Additionally, since tpe->nr_field_strings is not reset on failure, wouldn't > > a retry start appending string pointers out of bounds of the new array? > > This appears like it could eventually cause trace_probe_event_free() to read > > out-of-bounds pointers and pass them to kfree(). > > > > The above looks legit too. Please address it. > > -- Steve > > > > > } > > > return 0; > > > } > > >
