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; > > } >
