Re: [PATCH v2 2/2] tracing: Fix NULL dereference when copying keys for a field variable

[sashiko-bot@kernel.org]

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 <donggeunyoo.kernel@gmail.com>

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: stable@vger.kernel.org
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/20260913203129.941270-1-donggeunyoo.kernel@gmail.com?part=2