Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Leaked synthetic field variable histogram triggers on error paths in `create_field_var_hist()` and `event_hist_trigger_parse()`. -- commit d9bdb20cb47a42dc8d2becde420531d33d458afb Author: Donggeun Yoo <[email protected]> tracing: Fix NULL dereference when copying keys for a field variable This commit fixes a NULL pointer dereference in create_field_var_hist() by using expr_field_str() to retrieve the field name instead of accessing the name directly. This prevents crashes when handling pseudo field types like common_cpu. Cc: [email protected] Fixes: 02205a6752f2 ("tracing: Add support for 'field variables'") > diff --git a/kernel/trace/trace_events_hist.c > b/kernel/trace/trace_events_hist.c > index 9d7ce01fda360..b43b83d7c6f39 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -3097,7 +3097,7 @@ create_field_var_hist(struct hist_trigger_data > *target_hist_data, > key_field = hist_data->fields[i]; > if (!first) > seq_buf_putc(&s, ','); > - seq_buf_puts(&s, key_field->field->name); > + expr_field_str(key_field, &s); > first = false; > } > [Severity: High] This is a pre-existing issue, but does create_field_var_hist() leak synthetic field variable histogram triggers on error paths? Later in create_field_var_hist(), if the histogram trigger is successfully registered but find_synthetic_field_var() subsequently fails: ret = event_hist_trigger_parse(&trigger_hist_cmd, file, "", "hist", cmd); if (ret) { ... } kfree(cmd); /* If we can't find the variable, something went wrong */ event_var = find_synthetic_field_var(target_hist_data, subsys_name, event_name, field_name); if (IS_ERR_OR_NULL(event_var)) { kfree(var_hist->cmd); kfree(var_hist); hist_err(tr, HIST_ERR_SYNTH_VAR_NOT_FOUND, errpos(field_name)); return ERR_PTR(-EINVAL); } It appears the tracking reference is dropped without unregistering the newly created sub-trigger. Similarly, in event_hist_trigger_parse(), if a subsequent action fails: ret = create_actions(hist_data); if (ret) goto out_free; The out_free path destroys the tracking structures via destroy_hist_data() but doesn't appear to call unregister_field_var_hists() to unregister them: out_free: remove_hist_vars(hist_data); trigger_data_free(trigger_data); destroy_hist_data(hist_data); goto out; Could this allow userspace to trigger a memory leak or memory exhaustion by repeatedly writing malformed histogram triggers with invalid actions? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
